From 9f62da9ac311b593e1b3b135704d419c159b5664 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Thu, 22 Oct 2026 17:22:21 -0400 Subject: [PATCH 1/9] 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 40a9528be11b294b22e076d89624d8e9cda0ac38 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Fri, 23 Oct 2026 07:26:15 -0400 Subject: [PATCH 2/9] 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 27c93b20fb7451ee644882c78f5f150d4f3aa8fb Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Fri, 23 Oct 2026 10:00:38 -0400 Subject: [PATCH 3/9] fix(terminal): stop URL continuations from swallowing prose --- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 7 +++- test/terminal-url-continuation.test.ts | 44 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 6dc61ed7..2f68fad8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index b89bfdb7..b0bf0f2b 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2569,7 +2569,8 @@ function findTerminalUrls(text) { * started in (a table cell or list item continuation, not unrelated text); * - that row's first token looks like URL material (contains one of * `% & = / ? . - _`), is not a bare number/version or list marker, - * and is not itself a new `scheme://`. + * and is not itself a new `scheme://`; without `% & = / ?`, the token + * must finish its cell (row end or a gap of at least two spaces). * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. * @@ -2623,6 +2624,10 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; + // Hyphens, underscores and dots also occur in prose. Without stronger + // URL punctuation, the token must finish its cell, not introduce a sentence. + const rest = nextText.slice(indent + token.length); + if (!/[%&=/?]/.test(token) && rest.trim() !== '' && !/^ {2,}/.test(rest)) break; const nextLogical = terminalLogicalLine(buffer, endRow + 1, cols, bound); if (!nextLogical) break; if (trailingColumns) firstRowUrlEnd = last.rawEnd; diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 6d97ac2f..64b4868a 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -89,6 +89,19 @@ const TABLE_WITH_TRAILING_COLUMNS: Row[] = [ TABLE[4], ]; const TABLE_COLS = 220; +const PROSE_ROWS = [ + [ + '⏺ The guide lives at https://example.com/guide/', + ' self-hosted runners need a token first.', + 'https://example.com/guide/', + ], + ['Docs: https://example.com/guide/', 'Self-hosting is covered in chapter 3.', 'https://example.com/guide/'], + ['Open http://localhost:3000/', 'e.g. to check the dashboard', 'http://localhost:3000/'], + ['Endpoint https://api.example.com/v1/', 'user_id is required', 'https://api.example.com/v1/'], + ['Repo: https://github.com/org/repo/', 'well-known issues are tracked', 'https://github.com/org/repo/'], + ['Callback https://example.com/cb?code=', 'Re-run the login', 'https://example.com/cb?code='], + ['Branch https://github.com/o/r/tree/feat-', 'x.y happened', 'https://github.com/o/r/tree/feat-'], +]; describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); @@ -178,6 +191,28 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); }); + it.each(PROSE_ROWS)('does not join prose 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.each(['my-post-title', 'user_id', 'x.html', 'my-post-title ', 'my-post-title Other column'])( + 'keeps a weak-punctuation continuation that ends its cell: %s', + (next) => { + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: next }], 100); + for (const row of [0, 1]) + expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + } + ); + + it.each(['part%20two', 'x=1&y=2', 'path/slug', 'query?x=1'])( + 'keeps stronger URL punctuation followed by prose: %s', + (token) => { + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: token + ' more text' }], 100); + expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + } + ); + 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' }], @@ -295,6 +330,15 @@ describe('terminal link provider with wrapped table URLs', () => { return { provide, openSpy }; } + it.each(PROSE_ROWS)('preserves the original clickable URL before prose: %s', (first, next, url) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + const links = provide(1).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toEqual([url]); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(url, '_blank', 'noopener,noreferrer'); + expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); + }); + it('opens the whole URL from any of its rows', () => { const { provide, openSpy } = harness(TABLE, COLS); for (const line of [2, 3, 4]) { From 3a4e2c996ee28e622afd5d505190c313a8f067b3 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 24 Oct 2026 19:54:40 -0400 Subject: [PATCH 4/9] fix(terminal): keep shell prompts and status lines out of wrapped URLs A URL ending a row on `/` took the next row's first word whenever it carried URL punctuation, so a shell prompt, a home path or an HTTP status line could be appended to it. Refuse those tokens, and refuse a column-0 row as a hard continuation (real continuations are indented), which also keeps `status=200`, `24/7` and `really?` out. A divider row no longer continues the row above it, so a URL reaching the right edge does not absorb a full-width rule below it. Pin the new negatives at detector and provider level, re-indent the positive continuation fixtures, and pin the `self-hosted.` and `and/or` trade-offs. --- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 16 +++- test/terminal-url-continuation.test.ts | 106 +++++++++++++++++++++++-- 3 files changed, 115 insertions(+), 9 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 2f68fad8..68559b28 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted.` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`) or an HTTP status line never continues a URL. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index b0bf0f2b..3f8f5d23 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2333,6 +2333,9 @@ function terminalLogicalLine(buffer, row, cols, maxRows) { const continuesPrevious = (r) => { if (r <= 0) return false; if (lineAt(r)?.isWrapped) return true; + // A divider row never continues the row above, even when that row reaches + // the right edge: a URL ending at the edge must not absorb `────` below it. + if (/^[\s─━═╌╍┄┅┈┉-]+$/u.test(lineAt(r)?.translateToString(true) || '')) return false; const prev = lineAt(r - 1); const prevText = prev?.translateToString(true) || ''; // A full-width table divider is a row boundary, not a hard-wrapped line. @@ -2565,11 +2568,12 @@ function findTerminalUrls(text) { * 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 + * - the next row is indented, but 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 * `% & = / ? . - _`), is not a bare number/version or list marker, - * and is not itself a new `scheme://`; without `% & = / ?`, the token + * and is not itself a new `scheme://`, a shell prompt (`user@host:`), + * a home path (`~/`) or an HTTP status line; without `% & = / ?`, the token * must finish its cell (row end or a gap of at least two spaces). * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. @@ -2611,7 +2615,10 @@ 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 || /^(?:\d+[.)]|[-*•])(?:\s|$)/.test(nextText.slice(indent))) break; + // A hard continuation is indented (a table cell, a list item, Claude's + // two-space paragraph indent); a column-0 row is a new line of output. + if (!token || !indent || 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 ( @@ -2623,6 +2630,9 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { ) break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; + // A shell prompt (`user@host:~/dir$`), a home path and an HTTP status line + // are never the rest of a URL, however much URL punctuation they carry. + if (/^[\w.-]+@[\w.-]+:|^~\/|^HTTP\/\d/i.test(token)) break; if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; // Hyphens, underscores and dots also occur in prose. Without stronger // URL punctuation, the token must finish its cell, not introduce a sentence. diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 64b4868a..b03a5ccf 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -30,15 +30,20 @@ type Link = { function loadPure() { const context = vm.createContext({ console, window: undefined }); vm.runInContext( - `${CONSTANTS}\nglobalThis.__urls = findTerminalUrls; globalThis.__extent = terminalUrlContinuationExtent;`, + `${CONSTANTS}\nglobalThis.__urls = findTerminalUrls; globalThis.__extent = terminalUrlContinuationExtent; globalThis.__logical = terminalLogicalLine;`, context, { filename: 'constants.js' } ); const ctx = context as unknown as { __urls: (text: string) => UrlRef[]; __extent: (buffer: unknown, row: number, cols: number, maxRows?: number) => Extent; + __logical: (buffer: unknown, row: number, cols: number, maxRows?: number) => Extent; + }; + return { + findTerminalUrls: ctx.__urls, + terminalUrlContinuationExtent: ctx.__extent, + terminalLogicalLine: ctx.__logical, }; - return { findTerminalUrls: ctx.__urls, terminalUrlContinuationExtent: ctx.__extent }; } function fakeBuffer(rows: Row[], cols: number) { @@ -102,6 +107,29 @@ const PROSE_ROWS = [ ['Callback https://example.com/cb?code=', 'Re-run the login', 'https://example.com/cb?code='], ['Branch https://github.com/o/r/tree/feat-', 'x.y happened', 'https://github.com/o/r/tree/feat-'], ]; +// Rows that carry URL punctuation but are never the rest of the URL above them: +// a shell prompt, an HTTP status line, a home path, and column-0 prose. Each +// third element is the URL, the fourth the path link expected on the second row. +const NOT_CONTINUATION_ROWS: [string, string, string, string | null][] = [ + ['https://registry.npmjs.org/', 'arkon@box:~/proj$', 'https://registry.npmjs.org/', '~/proj'], + ['https://pypi.org/simple/', 'dev@laptop:/srv/app$', 'https://pypi.org/simple/', '/srv/app'], + ['$ curl -sI https://example.com/', 'HTTP/2 200', 'https://example.com/', null], + ['$ curl -i http://localhost:3000/api/', 'HTTP/1.1 200 OK', 'http://localhost:3000/api/', null], + ['Docs: https://example.com/guide/', '~/notes.md has more', 'https://example.com/guide/', '~/notes.md'], + ['fetch https://api.example.com/items/', 'status=200 took=12ms', 'https://api.example.com/items/', null], + ['Status page: https://status.example.com/', '24/7 monitoring is on.', 'https://status.example.com/', null], + ['Did you mean https://example.com/search?q=', 'really? no.', 'https://example.com/search?q=', null], +]; +// Known trade-offs: a row end counts as a cell end, so a short indented word +// with URL punctuation still continues the URL. Pinned so a change is deliberate. +const TRADE_OFF_ROWS: [string, string, string][] = [ + ['⏺ Docs live at https://example.com/guide/', ' self-hosted.', 'https://example.com/guide/self-hosted'], + [ + '⏺ The docs are at https://example.com/guide/', + ' and/or the API reference covers it.', + 'https://example.com/guide/and/or', + ], +]; describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); @@ -144,7 +172,7 @@ describe('findTerminalUrls (grammar)', () => { }); describe('terminalUrlContinuationExtent (wrap joining)', () => { - const { terminalUrlContinuationExtent } = loadPure(); + const { terminalUrlContinuationExtent, terminalLogicalLine } = loadPure(); it('spans all three rows of a URL wrapped inside a table cell, from any of them', () => { const buffer = fakeBuffer(TABLE, COLS); @@ -196,10 +224,42 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); }); + it.each(NOT_CONTINUATION_ROWS)('does not join a prompt, status line, home path or prose 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(); + }); + + // Indented, so only the prompt / home path / status-line rule can refuse them. + it.each(['arkon@box:~/proj$', 'dev@laptop:/srv/app$', '~/notes.md', 'HTTP/2 200', 'HTTP/1.1 200 OK'])( + 'does not join an indented %s either', + (next) => { + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + next }], 100); + for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); + } + ); + + it.each(TRADE_OFF_ROWS)('still joins the known trade-off after %s', (first, next) => { + const buffer = fakeBuffer([{ text: first }, { text: next }], 100); + for (const row of [0, 1]) + expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + }); + + it('does not let a URL at the right edge absorb a full-width divider row below it', () => { + const cols = 40; + const url = 'https://example.com/' + 'a'.repeat(cols - 'https://example.com/'.length); + for (const divider of ['─'.repeat(cols), '-'.repeat(cols)]) { + const buffer = fakeBuffer([{ text: url }, { text: divider }, { text: 'next' }], cols); + expect(terminalLogicalLine(buffer, 0, cols, 12)).toMatchObject({ startRow: 0, endRow: 0 }); + expect(terminalLogicalLine(buffer, 1, cols, 12)).toMatchObject({ startRow: 1 }); + } + }); + + // Real continuations are indented (codex cells, Claude's two-space paragraph + // indent); a column-0 row is refused, so these fixtures are indented. it.each(['my-post-title', 'user_id', 'x.html', 'my-post-title ', 'my-post-title Other column'])( 'keeps a weak-punctuation continuation that ends its cell: %s', (next) => { - const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: next }], 100); + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + next }], 100); for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); } @@ -208,7 +268,10 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { it.each(['part%20two', 'x=1&y=2', 'path/slug', 'query?x=1'])( 'keeps stronger URL punctuation followed by prose: %s', (token) => { - const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: token + ' more text' }], 100); + const buffer = fakeBuffer( + [{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + token + ' more text' }], + 100 + ); expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); } ); @@ -339,6 +402,39 @@ describe('terminal link provider with wrapped table URLs', () => { expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); }); + it.each(NOT_CONTINUATION_ROWS)('keeps the URL and the second row separate: %s', (first, next, url, pathOnNext) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + const links = provide(1).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toEqual([url]); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(url, '_blank', 'noopener,noreferrer'); + const second = provide(2); + expect(second.filter((link) => /^https?:/.test(link.text))).toEqual([]); + if (pathOnNext) expect(second.map((link) => link.text)).toContain(pathOnNext); + }); + + it.each(TRADE_OFF_ROWS)('pins the known trade-off link after %s', (first, next, joined) => { + const { provide } = harness([{ text: first }, { text: next }], 100); + for (const line of [1, 2]) { + const links = provide(line).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toContain(joined); + } + }); + + it('keeps a full-width divider row out of a URL that reaches the right edge', () => { + const cols = 40; + const url = 'https://example.com/' + 'a'.repeat(cols - 'https://example.com/'.length); + for (const divider of ['─'.repeat(cols), '-'.repeat(cols)]) { + const { provide } = harness([{ text: url }, { text: divider }], cols); + expect( + provide(1) + .filter((link) => /^https?:/.test(link.text)) + .map((link) => link.text) + ).toEqual([url]); + expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); + } + }); + it('opens the whole URL from any of its rows', () => { const { provide, openSpy } = harness(TABLE, COLS); for (const line of [2, 3, 4]) { From 2a90fb8118a8f603c52c8d8a77c4077bedbc342e Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 24 Oct 2026 22:28:29 -0400 Subject: [PATCH 5/9] fix(terminal): keep the next table record out of a last-column URL A URL in the last column of a space-aligned table ended on a slash, so the first cell of the next record (billing-api, CHANGELOG.md, ...) passed every continuation test and was joined onto it. The alignment check that refused this only ran when other columns followed the URL. A continuation row must now start inside the URL's own cell: a gap of two or more spaces between its indent and where the URL starts on the first row means it opens an earlier column. An indented token that is a whole email address is also refused. Adds the six table shapes and the email case as negative tests at detector and provider level, and documents the rule. --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 15 ++++- test/terminal-url-continuation.test.ts | 82 ++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f528681b..de020024 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ 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. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers, version tokens and whole email addresses never continue a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 68559b28..1d0e124e 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted.` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`) or an HTTP status line never continues a URL. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted.` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 3f8f5d23..deb5cf2d 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2569,12 +2569,15 @@ function findTerminalUrls(text) { * - the URL fragment ends on a character a wrapper breaks after * (`/ ? & = - _ % # + ~`); a following table column may be excluded; * - the next row is indented, but no further right than the column the URL - * started in (a table cell or list item continuation, not unrelated text); + * started in (a table cell or list item continuation, not unrelated text), + * and no column gap lies between its indent and the URL on the first row + * (that would be the next record of a table); * - that row's first token looks like URL material (contains one of * `% & = / ? . - _`), is not a bare number/version or list marker, * and is not itself a new `scheme://`, a shell prompt (`user@host:`), - * a home path (`~/`) or an HTTP status line; without `% & = / ?`, the token - * must finish its cell (row end or a gap of at least two spaces). + * a home path (`~/`), an email address or an HTTP status line; without + * `% & = / ?`, the token must finish its cell (row end or a gap of at + * least two spaces). * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. * @@ -2629,10 +2632,16 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { / {2,}/.test(first.text.slice(indent, last.startIndex))) ) break; + // The next row must start inside the URL's own cell: a column gap between + // its indent and the URL on the first row means it opens an earlier + // column (the next record of a table), not the rest of the URL. + if (/ {2,}/.test(first.text.slice(indent, last.startIndex).trimStart())) break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; // A shell prompt (`user@host:~/dir$`), a home path and an HTTP status line // are never the rest of a URL, however much URL punctuation they carry. if (/^[\w.-]+@[\w.-]+:|^~\/|^HTTP\/\d/i.test(token)) break; + // Nor is a token that is a whole email address. + if (/^[\w.+-]+@[\w-]+(?:\.[\w-]+)+$/.test(token)) break; if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; // Hyphens, underscores and dots also occur in prose. Without stronger // URL punctuation, the token must finish its cell, not introduce a sentence. diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index b03a5ccf..ad6fc98f 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -120,6 +120,54 @@ const NOT_CONTINUATION_ROWS: [string, string, string, string | null][] = [ ['Status page: https://status.example.com/', '24/7 monitoring is on.', 'https://status.example.com/', null], ['Did you mean https://example.com/search?q=', 'really? no.', 'https://example.com/search?q=', null], ]; +// The last column of a space-aligned table: the next row opens the next record +// in an earlier column, so its first cell is never the rest of the URL. Each +// row is indented two spaces. The URL on each row, then any path link expected +// on the second row. +const TABLE_LAST_COLUMN_ROWS: [string, string, string, string, string | null][] = [ + [ + ' auth-api https://auth.example.com/', + ' billing-api https://billing.example.com/', + 'https://auth.example.com/', + 'https://billing.example.com/', + null, + ], + [ + ' docs.example.com https://docs.example.com/', + ' api.example.com https://api.example.com/', + 'https://docs.example.com/', + 'https://api.example.com/', + null, + ], + [ + ' Homepage: https://example.com/', + ' Bug-reports: https://github.com/org/repo/issues/', + 'https://example.com/', + 'https://github.com/org/repo/issues/', + null, + ], + [ + ' user_service https://users.example.com/v1/', + ' order_service https://orders.example.com/v1/', + 'https://users.example.com/v1/', + 'https://orders.example.com/v1/', + null, + ], + [ + ' README.md https://example.com/docs/', + ' CHANGELOG.md https://example.com/changes/', + 'https://example.com/docs/', + 'https://example.com/changes/', + 'CHANGELOG.md', + ], + [ + ' my-service- https://svc.example.com/api/', + ' long-name https://svc2.example.com/', + 'https://svc.example.com/api/', + 'https://svc2.example.com/', + null, + ], +]; // Known trade-offs: a row end counts as a cell end, so a short indented word // with URL punctuation still continues the URL. Pinned so a change is deliberate. const TRADE_OFF_ROWS: [string, string, string][] = [ @@ -238,6 +286,16 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { } ); + it.each(TABLE_LAST_COLUMN_ROWS)('does not join the next table record 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 join an indented email address', () => { + const buffer = fakeBuffer([{ text: ' https://example.com/' }, { text: ' user@example.com' }], 100); + for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); + }); + it.each(TRADE_OFF_ROWS)('still joins the known trade-off after %s', (first, next) => { const buffer = fakeBuffer([{ text: first }, { text: next }], 100); for (const row of [0, 1]) @@ -413,6 +471,30 @@ describe('terminal link provider with wrapped table URLs', () => { if (pathOnNext) expect(second.map((link) => link.text)).toContain(pathOnNext); }); + it.each(TABLE_LAST_COLUMN_ROWS)( + 'keeps each last-column URL on its own row: %s', + (first, next, url, nextUrl, pathOnNext) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + const links = provide(1).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toEqual([url]); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(url, '_blank', 'noopener,noreferrer'); + const second = provide(2); + expect(second.filter((link) => /^https?:/.test(link.text)).map((link) => link.text)).toEqual([nextUrl]); + if (pathOnNext) expect(second.map((link) => link.text)).toContain(pathOnNext); + } + ); + + it('keeps an indented email address out of the URL above it', () => { + const { provide } = harness([{ text: ' https://example.com/' }, { text: ' user@example.com' }], 100); + expect( + provide(1) + .filter((link) => /^https?:/.test(link.text)) + .map((link) => link.text) + ).toEqual(['https://example.com/']); + expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); + }); + it.each(TRADE_OFF_ROWS)('pins the known trade-off link after %s', (first, next, joined) => { const { provide } = harness([{ text: first }, { text: next }], 100); for (const line of [1, 2]) { From 1bdc341784bd7253ea36fabbf744981d3f0f1db4 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 24 Oct 2026 22:50:23 -0400 Subject: [PATCH 6/9] fix(terminal): keep code, fields and measurements out of wrapped URLs A URL ending in `/` still took the next indented row when that row was code, a label, a log or config field, a measurement or the end of a sentence. A side-by-side sweep against master turned up shapes such as `ready.` under a codex `Listening on` line, `expect(x).toBe(y)` under a jest title, `Failure/Error:` under an rspec title, `API_KEY=...` under `BASE_URL=...`, `level=info` log fields, `45%|####` tqdm bars, `12.3MB/45.0MB`, `12.3ms` and `10:00:00.123`. A continuation is now refused when it ends with a colon, when it is a measurement or clock time, or when it is a `name=value` token under a URL ending in `/` (a query needs `?` first). A token with only `-`, `_` or `.` is also refused when it carries a call paren or ends a sentence. Wiki titles such as `Mercury_(planet)`, a file name before its period, query parameters after `?`, `&` or `_`, and percent-encoding that starts with digits still join. `self-hosted.` moves from the pinned trade-offs to the negatives; the trade-off is now a lone word without sentence punctuation. --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 28 +++++- test/terminal-url-continuation.test.ts | 120 ++++++++++++++++++++++--- 4 files changed, 135 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index de020024..68de089e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ 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, version tokens and whole email addresses never continue a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `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. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 1d0e124e..e06bb19c 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted.` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index deb5cf2d..78e04721 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2575,9 +2575,12 @@ function findTerminalUrls(text) { * - that row's first token looks like URL material (contains one of * `% & = / ? . - _`), is not a bare number/version or list marker, * and is not itself a new `scheme://`, a shell prompt (`user@host:`), - * a home path (`~/`), an email address or an HTTP status line; without - * `% & = / ?`, the token must finish its cell (row end or a gap of at - * least two spaces). + * a home path (`~/`), an email address, an HTTP status line, a label + * ending in `:`, a measurement or clock time (`45%`, `12.3ms`, + * `10:00:00`), or a `name=value` field under a URL ending in `/`; + * without `% & = / ?`, the token must finish its cell (row end or a + * gap of at least two spaces) and must not be code with a call paren + * or a word ending a sentence (`Done.`). * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. * @@ -2643,10 +2646,27 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { // Nor is a token that is a whole email address. if (/^[\w.+-]+@[\w-]+(?:\.[\w-]+)+$/.test(token)) break; if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; + // A label ends with a colon (`Failure/Error:`, `prometheus.io/scrape:`); + // the rest of a URL does not. + if (token.endsWith(':')) break; + // After a `/`, a `name=value` token is the next assignment or log field + // (`API_KEY=...`, `level=info`), not the URL: a query starts with `?`. + if (urlText.endsWith('/') && /^[A-Za-z_][\w.-]*=/.test(token)) break; + // Nor is a measurement or a clock time: `45%` and `45%|####` progress, + // `12.3MB/45.0MB`, `12.3ms`, `10:00:00` (`%7B` stays percent-encoding). + if (/^\d+(?:\.\d+)?(?:%(?![0-9A-Fa-f]{2})|(?:ms|s|[kKMGT]i?B)(?![\w-]))|^\d{1,2}:\d{2}/.test(token)) break; // Hyphens, underscores and dots also occur in prose. Without stronger // URL punctuation, the token must finish its cell, not introduce a sentence. const rest = nextText.slice(indent + token.length); - if (!/[%&=/?]/.test(token) && rest.trim() !== '' && !/^ {2,}/.test(rest)) break; + if (!/[%&=/?]/.test(token)) { + // Code (`client.get()`, `expect(x).toBe(y)`) is not a URL piece, and a + // word that ends a sentence (`ready.`, `Done.`) is prose. A wiki title + // (`Mercury_(planet)`) and a file name before its period + // (`install.html.`) still continue the URL. + if (/[^_]\(/.test(token)) break; + if (/[.,;!]$/.test(token) && !/\.[A-Za-z][A-Za-z0-9]{1,4}[.,;!]+$/.test(token)) break; + if (rest.trim() !== '' && !/^ {2,}/.test(rest)) break; + } const nextLogical = terminalLogicalLine(buffer, endRow + 1, cols, bound); if (!nextLogical) break; if (trailingColumns) firstRowUrlEnd = last.rawEnd; diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index ad6fc98f..9f8cd847 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -171,7 +171,7 @@ const TABLE_LAST_COLUMN_ROWS: [string, string, string, string, string | null][] // Known trade-offs: a row end counts as a cell end, so a short indented word // with URL punctuation still continues the URL. Pinned so a change is deliberate. const TRADE_OFF_ROWS: [string, string, string][] = [ - ['⏺ Docs live at https://example.com/guide/', ' self-hosted.', 'https://example.com/guide/self-hosted'], + ['⏺ Docs live at https://example.com/guide/', ' self-hosted', 'https://example.com/guide/self-hosted'], [ '⏺ The docs are at https://example.com/guide/', ' and/or the API reference covers it.', @@ -179,6 +179,69 @@ const TRADE_OFF_ROWS: [string, string, string][] = [ ], ]; +// Indented rows under a URL ending in `/` that carry URL punctuation but are +// code, a label, a `name=value` field, a measurement or a clock time, or a word +// ending a sentence. Each third element is the URL on the first row. +const FIELD_AND_CODE_ROWS: [string, string, string][] = [ + [' └ Listening on http://127.0.0.1:8080/', ' ready.', 'http://127.0.0.1:8080/'], + ['⏺ Deployed to https://app.example.com/', ' Done.', 'https://app.example.com/'], + ['⏺ Docs live at https://example.com/guide/', ' self-hosted.', 'https://example.com/guide/'], + [' Fetching https://example.com/files/', ' done...', 'https://example.com/files/'], + [ + ' ● fetch › handles https://api.example.com/v1/', + ' expect(received).toBe(expected)', + 'https://api.example.com/v1/', + ], + [' See https://example.com/api/', ' client.get()', 'https://example.com/api/'], + [ + '/home/u/.venv/lib/urllib3/__init__.py:35: Warning: see https://github.com/urllib3/urllib3/issues/', + ' warnings.warn(', + 'https://github.com/urllib3/urllib3/issues/', + ], + [ + ' 1) Client fetches https://api.example.com/v1/', + ' Failure/Error: expect(res.code).to eq(200)', + 'https://api.example.com/v1/', + ], + [ + 'Annotations: link.argocd.argoproj.io/external-link: https://grafana.example.com/d/', + ' prometheus.io/scrape: true', + 'https://grafana.example.com/d/', + ], + [' BASE_URL=https://api.example.com/', ' API_KEY=your-key-here', 'https://api.example.com/'], + [' AUTH_URL=https://auth.example.com/', ' LOG_LEVEL=debug', 'https://auth.example.com/'], + [ + ' level=info msg="request" url=https://api.example.com/v1/', + ' level=info msg="done" status=200', + 'https://api.example.com/v1/', + ], + [' url=https://api.example.com/v1/', ' request_id=abc-123', 'https://api.example.com/v1/'], + [ + ' Callback: https://example.com/oauth/callback/', + ' state=abc123 must match', + 'https://example.com/oauth/callback/', + ], + [ + ' Downloading https://huggingface.co/org/model/resolve/main/', + ' 45%|████▌ | 450M/1.00G [00:10<00:12, 45.0MB/s]', + 'https://huggingface.co/org/model/resolve/main/', + ], + [' Downloading https://example.com/files/', ' 12.3MB/45.0MB', 'https://example.com/files/'], + [' GET https://example.com/api/', ' 12.3ms', 'https://example.com/api/'], + [' https://example.com/blog/', ' 10:00:00.123', 'https://example.com/blog/'], +]; +// Continuations at the edge of those rules that are still the rest of the URL: +// a wiki title with parens, a file name before a sentence period, a query +// parameter after `?`, `&` or `_`, and percent-encoding that starts with digits. +const STILL_CONTINUES_ROWS: [string, string, string][] = [ + [' https://en.wikipedia.org/wiki/', ' Mercury_(planet)', 'https://en.wikipedia.org/wiki/Mercury_(planet)'], + ['⏺ The guide is at https://example.com/docs/', ' install.html.', 'https://example.com/docs/install.html'], + [' https://example.com/oauth?client_', ' id=abc&scope=read', 'https://example.com/oauth?client_id=abc&scope=read'], + [' https://example.com/cb?code=abc&', ' state=xyz', 'https://example.com/cb?code=abc&state=xyz'], + [' Docs: https://example.com/guide?', ' x=1&y=2 more text', 'https://example.com/guide?x=1&y=2'], + [' https://example.com/files/', ' 2026%20Plan.xlsx', 'https://example.com/files/2026%20Plan.xlsx'], +]; + describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); const urls = (text: string) => findTerminalUrls(text).map((u) => u.url); @@ -291,6 +354,20 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); }); + it.each(FIELD_AND_CODE_ROWS)( + 'does not join code, a field, a measurement or a sentence end 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.each(STILL_CONTINUES_ROWS)('still joins the continuation after %s', (first, next) => { + const buffer = fakeBuffer([{ text: first }, { text: next }], 100); + for (const row of [0, 1]) + expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + }); + it('does not join an indented email address', () => { const buffer = fakeBuffer([{ text: ' https://example.com/' }, { text: ' user@example.com' }], 100); for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); @@ -323,16 +400,13 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { } ); - it.each(['part%20two', 'x=1&y=2', 'path/slug', 'query?x=1'])( - 'keeps stronger URL punctuation followed by prose: %s', - (token) => { - const buffer = fakeBuffer( - [{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + token + ' more text' }], - 100 - ); - expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); - } - ); + it.each(['part%20two', 'path/slug', 'query?x=1'])('keeps stronger URL punctuation followed by prose: %s', (token) => { + const buffer = fakeBuffer( + [{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + token + ' more text' }], + 100 + ); + expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + }); it('does not glue a following prose line onto a URL that ends a line', () => { const buffer = fakeBuffer( @@ -485,6 +559,30 @@ describe('terminal link provider with wrapped table URLs', () => { } ); + it.each(FIELD_AND_CODE_ROWS)( + 'keeps code, a field, a measurement or a sentence end separate: %s', + (first, next, url) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + const links = provide(1).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toEqual([url]); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(url, '_blank', 'noopener,noreferrer'); + expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); + } + ); + + it.each(STILL_CONTINUES_ROWS)('links the whole URL from both rows after %s', (first, next, joined) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + for (const line of [1, 2]) { + // One link per row the URL covers, each opening the whole URL. + const links = provide(line).filter((link) => /^https?:/.test(link.text)); + expect(new Set(links.map((link) => link.text))).toEqual(new Set([joined])); + } + const [link] = provide(2).filter((l) => /^https?:/.test(l.text)); + link.activate(null, link.text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(joined, '_blank', 'noopener,noreferrer'); + }); + it('keeps an indented email address out of the URL above it', () => { const { provide } = harness([{ text: ' https://example.com/' }, { text: ' user@example.com' }], 100); expect( From 8ad60facf3e3b7988556b56424f6618366d4608d Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 25 Oct 2026 09:03:29 -0400 Subject: [PATCH 7/9] fix(terminal): keep a directory cell from taking the next row's file name A table cell holding a whole directory (`/home/u/proj` over `config.yaml`) was joined onto the file name in the same column on the next row, which is the next record, and the joined link then replaced the correct links on both cells. A wrapper breaks a path after `/` or `-` (codex wraps cells on Unicode line-break opportunities plus a hyphen splitter, which never break at `_`), so a cell fragment must end on one of those before the next row can continue it. A directory written with its trailing slash over a relative path in the same column still joins, since it reads exactly like a wrap at `/`; pinned as a known trade-off. --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 17 +++++-- test/terminal-cell-wrapped-paths.test.ts | 64 ++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 68de089e..9fa84f57 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ 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, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `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. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`) keeps its folder link and the next row keeps its own file link. A directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index e06bb19c..8d1713a2 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`) keeps its folder link and the next row keeps its own file link. A directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 78e04721..5e22d152 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2761,9 +2761,12 @@ 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). 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. + * (row end or a ≥2-space gap follows) and ends on `/` or `-`, the only places + * a wrapper breaks a path; a cell that ends inside a name (`/home/u/proj` over + * `config.yaml`) is a whole value, never the first piece of the next row's. 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). @@ -2825,6 +2828,14 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { 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(); + // A wrapper breaks a path after `/` or `-`; a fragment that ends inside a + // name (`/home/u/proj` over `config.yaml`) is a whole value, not a piece. + // codex wraps table cells on Unicode line-break opportunities plus a + // hyphen splitter, which break after `/` and `-` but never at `_` (UAX #14 + // class AL). Known trade-off: a directory written with its trailing slash + // over a relative path in the same column (`/home/u/proj/src/` over + // `lib/util.ts`) still joins, since it reads exactly like a wrap at `/`. + if (!/[/-]$/.test(joined)) break; if (cellText.length > previousCellLength || !pathContinuation(joined, token)) break; const candidate = joined + token; const match = absoluteFilePathPattern().exec(candidate); diff --git a/test/terminal-cell-wrapped-paths.test.ts b/test/terminal-cell-wrapped-paths.test.ts index 7b25672d..873aaf14 100644 --- a/test/terminal-cell-wrapped-paths.test.ts +++ b/test/terminal-cell-wrapped-paths.test.ts @@ -65,6 +65,32 @@ const LISTINGS = [ ['Created /tmp/out', 'report.md written next to it'], ]; +// A directory that is the whole value of its cell, over the next record's file +// name in the same column. A wrapper breaks a path after `/` or `-`, never inside +// a name, so none of these is one path: each cell keeps its own link. +const WHOLE_VALUE_SHAPES: Array<{ rows: string[]; dir: string; file: string }> = [ + { rows: [' Repo /home/u/proj', ' Config config.yaml'], dir: '/home/u/proj', file: 'config.yaml' }, + { + rows: [' Workspace /home/u/proj/app', ' Entry main.py'], + dir: '/home/u/proj/app', + file: 'main.py', + }, + { + rows: ['│ Workspace │ /home/u/proj/app │', '│ Entry │ main.py │'], + dir: '/home/u/proj/app', + file: 'main.py', + }, + { rows: [' Output dir /tmp/build/out', ' Log build.log'], dir: '/tmp/build/out', file: 'build.log' }, + { + rows: [' Path Purpose', ' /home/u/proj/src sources', ' index.ts entry'], + dir: '/home/u/proj/src', + file: 'index.ts', + }, +]; +// Known trade-off: a directory written WITH its trailing slash over a relative +// path in the same column reads exactly like a wrap at `/`, so it still joins. +const TRAILING_SLASH_TRADE_OFF = [' Source /home/u/proj/src/', ' Helper lib/util.ts']; + const BEFORE = '/tmp/demo-check/docs/review/BLDMESH-243/before-overview.png'; const AFTER = '/tmp/demo-check/docs/review/BLDMESH-243/after-overview.png'; @@ -83,6 +109,25 @@ describe('terminalCellWrappedPaths', () => { for (let row = 0; row < rows.length; row++) expect(find(fakeBuffer(rows, COLS), row)).toEqual([]); }); + it.each(WHOLE_VALUE_SHAPES.map((shape) => [shape.rows, shape]))( + 'does not join a directory cell onto the next record: %j', + (_rows, shape) => { + const rows = shape.rows.map((text) => ({ text })); + for (let row = 0; row < rows.length; row++) expect(find(fakeBuffer(rows, COLS), row)).toEqual([]); + } + ); + + it('does not join a fragment that ends on `_` (codex wraps cells only after `/` or `-`)', () => { + const rows = [{ text: ' shot (/tmp/demo/docs/review_' }, { text: ' before.png)' }]; + expect(find(fakeBuffer(rows, COLS), 0)).toEqual([]); + expect(find(fakeBuffer(rows, COLS), 1)).toEqual([]); + }); + + it('still joins a trailing-slash directory over a relative path in its column (known trade-off)', () => { + const rows = TRAILING_SLASH_TRADE_OFF.map((text) => ({ text })); + expect(find(fakeBuffer(rows, COLS), 1).map((h) => h.path)).toEqual(['/home/u/proj/src/lib/util.ts']); + }); + it('places the segments on the path cells only', () => { const hits = find(fakeBuffer(TABLE, COLS), 1); const before = hits.find((h) => h.path === BEFORE)!; @@ -215,6 +260,25 @@ describe('terminal link provider with cell-wrapped paths', () => { } }); + it.each(WHOLE_VALUE_SHAPES.map((shape) => [shape.rows, shape]))( + 'keeps the folder link and the next record’s file link: %j', + (_rows, shape) => { + const { provide, app } = harness(shape.rows.map((text) => ({ text }))); + const dirLine = shape.rows.findIndex((text) => text.includes(shape.dir)) + 1; + const fileLine = shape.rows.findIndex((text) => text.includes(shape.file)) + 1; + const all = [...provide(dirLine), ...provide(fileLine)].map((link) => link.text); + expect(all.some((text) => text.includes(shape.dir + shape.file))).toBe(false); + const dirLink = provide(dirLine).find((link) => link.text === shape.dir); + expect(dirLink, `${shape.dir} on row ${dirLine}`).toBeDefined(); + expect(dirLink!.range.start.y).toBe(dirLine); + const fileLink = provide(fileLine).find((link) => link.text === shape.file); + expect(fileLink, `${shape.file} on row ${fileLine}`).toBeDefined(); + expect(fileLink!.range.start.y).toBe(fileLine); + fileLink!.activate(null, fileLink!.text); + expect(app.openFilePreview).not.toHaveBeenCalledWith(shape.dir + shape.file, 's1'); + } + ); + 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]) { From ae3d31407bbfeb1d35cecdf1d712d5eb6f626bbc Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 25 Oct 2026 09:11:46 -0400 Subject: [PATCH 8/9] fix(terminal): keep a labelled table record out of a cell-wrapped path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A wrapper wraps every column of one table row together, so the row that carries a cell's continuation is blank to the left of that cell unless a path in a left column continued on it too. A label there (`Entry`, `│ Helper │`) opens the next record, so a directory ending in `/` or `-` (`Workspace /home/u/proj/app/` over `Entry main.py`) no longer takes the next record's file name. What still joins is a trailing-slash directory over a relative path in the same column with nothing to its left, which reads exactly like a wrap at `/`; that trade-off stays pinned. --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 23 ++++++++++++++--- test/terminal-cell-wrapped-paths.test.ts | 32 ++++++++++++++++++------ 4 files changed, 46 insertions(+), 13 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9fa84f57..e8609a10 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ 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, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`) keeps its folder link and the next row keeps its own file link. A directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 8d1713a2..0060343c 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`) keeps its folder link and the next row keeps its own file link. A directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 5e22d152..e5668a25 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2763,7 +2763,10 @@ function terminalLinkLine(buffer, row, cols, maxRows) { * 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 ends on `/` or `-`, the only places * a wrapper breaks a path; a cell that ends inside a name (`/home/u/proj` over - * `config.yaml`) is a whole value, never the first piece of the next row's. Every + * `config.yaml`) is a whole value, never the first piece of the next row's. A + * continuation row must be blank to the left of the cell (a wrapper wraps every + * column of one table row together), unless a path in a left column continued + * on it too: a label there (`Entry`, `│ Log │`) is the next record. 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. @@ -2832,10 +2835,22 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { // name (`/home/u/proj` over `config.yaml`) is a whole value, not a piece. // codex wraps table cells on Unicode line-break opportunities plus a // hyphen splitter, which break after `/` and `-` but never at `_` (UAX #14 - // class AL). Known trade-off: a directory written with its trailing slash - // over a relative path in the same column (`/home/u/proj/src/` over - // `lib/util.ts`) still joins, since it reads exactly like a wrap at `/`. + // class AL). if (!/[/-]$/.test(joined)) break; + // A wrapper wraps every column of one table row together, so a row that + // carries a cell's continuation is blank to its left unless a left column + // wrapped on that row too. A label there (`Entry`, `│ Log │`) opens the + // next record, even under a directory ending in `/` (`Workspace + // /home/u/proj/app/` over `Entry main.py`). + // Known trade-off: with nothing to the left, a directory written with its + // trailing slash over a relative path in the same column (`/home/u/proj/` + // over `index.ts`) still joins, since it reads exactly like a wrap at `/`. + if ( + /[^\s│┃║|]/.test(next.slice(0, cellCol)) && + !found.some((hit) => hit.segments.some((seg) => seg.row === r && seg.endCol <= cellCol)) + ) { + break; + } if (cellText.length > previousCellLength || !pathContinuation(joined, token)) break; const candidate = joined + token; const match = absoluteFilePathPattern().exec(candidate); diff --git a/test/terminal-cell-wrapped-paths.test.ts b/test/terminal-cell-wrapped-paths.test.ts index 873aaf14..765ec3df 100644 --- a/test/terminal-cell-wrapped-paths.test.ts +++ b/test/terminal-cell-wrapped-paths.test.ts @@ -87,9 +87,27 @@ const WHOLE_VALUE_SHAPES: Array<{ rows: string[]; dir: string; file: string }> = file: 'index.ts', }, ]; -// Known trade-off: a directory written WITH its trailing slash over a relative -// path in the same column reads exactly like a wrap at `/`, so it still joins. -const TRAILING_SLASH_TRADE_OFF = [' Source /home/u/proj/src/', ' Helper lib/util.ts']; +// The same records where the directory DOES end on a break character: the label +// on the next row (`Entry`, `│ Helper │`) says it is a new record, since a wrapper +// wraps every column of one table row together and leaves the label cell blank. +const LABELLED_RECORD_SHAPES: Array<{ rows: string[]; dir: string; file: string }> = [ + { + rows: [' Workspace /home/u/proj/app/', ' Entry main.py'], + dir: '/home/u/proj/app/', + file: 'main.py', + }, + { + rows: ['│ Source │ /home/u/proj/src/ │', '│ Helper │ lib/util.ts │'], + dir: '/home/u/proj/src/', + file: 'lib/util.ts', + }, + { rows: [' Prefix /tmp/run-', ' Suffix log.txt'], dir: '/tmp/run-', file: 'log.txt' }, +]; +const SEPARATE_RECORD_SHAPES = [...WHOLE_VALUE_SHAPES, ...LABELLED_RECORD_SHAPES]; +// Known trade-off: with no label to tell them apart, a directory written WITH its +// trailing slash over a relative path in the same column reads exactly like a +// wrap at `/`, so it still joins. +const TRAILING_SLASH_TRADE_OFF = [' /home/u/proj/src/ sources', ' lib/util.ts helpers']; const BEFORE = '/tmp/demo-check/docs/review/BLDMESH-243/before-overview.png'; const AFTER = '/tmp/demo-check/docs/review/BLDMESH-243/after-overview.png'; @@ -109,7 +127,7 @@ describe('terminalCellWrappedPaths', () => { for (let row = 0; row < rows.length; row++) expect(find(fakeBuffer(rows, COLS), row)).toEqual([]); }); - it.each(WHOLE_VALUE_SHAPES.map((shape) => [shape.rows, shape]))( + it.each(SEPARATE_RECORD_SHAPES.map((shape) => [shape.rows, shape]))( 'does not join a directory cell onto the next record: %j', (_rows, shape) => { const rows = shape.rows.map((text) => ({ text })); @@ -123,7 +141,7 @@ describe('terminalCellWrappedPaths', () => { expect(find(fakeBuffer(rows, COLS), 1)).toEqual([]); }); - it('still joins a trailing-slash directory over a relative path in its column (known trade-off)', () => { + it('still joins an unlabelled trailing-slash directory over a relative path in its column (known trade-off)', () => { const rows = TRAILING_SLASH_TRADE_OFF.map((text) => ({ text })); expect(find(fakeBuffer(rows, COLS), 1).map((h) => h.path)).toEqual(['/home/u/proj/src/lib/util.ts']); }); @@ -260,8 +278,8 @@ describe('terminal link provider with cell-wrapped paths', () => { } }); - it.each(WHOLE_VALUE_SHAPES.map((shape) => [shape.rows, shape]))( - 'keeps the folder link and the next record’s file link: %j', + it.each(SEPARATE_RECORD_SHAPES.map((shape) => [shape.rows, shape]))( + 'keeps the folder link and the file link of the next record: %j', (_rows, shape) => { const { provide, app } = harness(shape.rows.map((text) => ({ text }))); const dirLine = shape.rows.findIndex((text) => text.includes(shape.dir)) + 1; From e93382f2f180705eca266e27f0c37841cf74eaca Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 27 Oct 2026 01:23:56 +0100 Subject: [PATCH 9/9] fix: apply the review items for #686 at merge - constants.js:2618 terminalUrlContinuationExtent: trailingColumns now needs a one-row first line (first.startRow === first.endRow), so a URL row that soft-wraps with a column on its wrapped part no longer glues the next row onto that column (`/tmp/report.txtccc/x.html` at 41 columns); master's links come back. Negative tests for the extent and the link provider. - constants.js:2847 terminalCellWrappedPaths: comment reworded to match the JSDoc (the exception is a left-column PATH), plus the wrapped-prose case. - docs/architecture-invariants.md, CLAUDE.md: one clause that a left column of wrapped prose (a Description | Path table) also fills the continuation row, so the path beside it keeps its per-row fragment links, the safe direction. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 24 +++++++++++++++++------- test/terminal-url-continuation.test.ts | 24 ++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e8609a10..8281923a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,7 @@ 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, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers, version tokens, whole email addresses, labels ending in `:`, measurements and clock times (`45%`, `12.3ms`, `10:00:00`) and a `name=value` field under a URL ending in `/` never continue a URL; a token with only `-`, `_` or `.` is also refused when it carries a call paren (`client.get()`) or ends a sentence (`Done.`). A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too; a left column of wrapped prose, as in a `Description | Path` table, fills that row as well, so the path beside it keeps its per-row fragment links, the safe direction), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 0060343c..707ce138 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ 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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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. +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. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. Such a token is still refused when it is code with a call paren (`client.get()`, `expect(x).toBe(y)`; a wiki title like `Mercury_(planet)` keeps its paren) or a word ending a sentence (`ready.`, `Done.`, `self-hosted.`; a file name keeps its period, so `install.html.` still joins). Whatever its punctuation, a token never continues a URL when it ends with a colon (a label such as `Failure/Error:` or `prometheus.io/scrape:`), when it is a measurement or clock time (`45%`, `45%|####`, `12.3MB/45.0MB`, `12.3ms`, `10:00:00`; percent-encoding such as `2026%20Plan` still joins), or when it is a `name=value` field (`API_KEY=...`, `level=info`) under a URL ending in `/`, since a query needs a `?` first (after `?`, `&` or `_` it is a query parameter and joins). A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`), a whole email address or an HTTP status line never continues a URL. A URL in the last column of a space-aligned table never takes the next row's first cell: a continuation row must start inside the URL's own cell, so a gap of two or more spaces between its indent and where the URL starts on the row above means it is the next record. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently: a fragment carries on into the next row only when it ends its cell on `/` or `-` (where a wrapper breaks a path; codex's cell wrap never breaks at `_`), the continuation row is blank to the left of the cell (a wrapper wraps every column of a table row together, so a label there such as `Entry` or `│ Log │` is the next record, unless a left column's path continued on that row too; a left column of wrapped prose, as in a `Description | Path` table, fills that row as well, so the path beside it keeps its per-row fragment links, the safe direction), the next row's token passes `pathContinuation()`, and the cell is no wider than the one above. So a directory that is a whole cell value (`/home/u/proj` over `config.yaml`, or `/home/u/proj/app/` over a labelled `main.py`) keeps its folder link and the next row keeps its own file link. With no label to tell them apart, a directory written with its trailing slash over a relative path in the same column (`/home/u/proj/src/` over `lib/util.ts`) still joins, a known trade-off: it reads exactly like a wrap at `/`. 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`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index e5668a25..8e2b4fcc 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2592,8 +2592,8 @@ function findTerminalUrls(text) { * @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. + * other table columns follow it and that row does not soft-wrap; 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); @@ -2611,8 +2611,15 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { while (endRow - start + 1 < bound) { const urls = findTerminalUrls(text); const last = urls[urls.length - 1]; + // Only on a one-row first line: firstRowUrlEnd cuts the first PHYSICAL + // row (terminalLinkLine), so a column sitting on the soft-wrapped part of + // the URL's row would stay in the text and take the next row with it. const trailingColumns = - endRow === first.endRow && last && last.rawEnd < text.length && /^ {2,}\S/.test(text.slice(last.rawEnd)); + first.startRow === first.endRow && + 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; @@ -2838,10 +2845,13 @@ function terminalCellWrappedPaths(buffer, row, maxRows) { // class AL). if (!/[/-]$/.test(joined)) break; // A wrapper wraps every column of one table row together, so a row that - // carries a cell's continuation is blank to its left unless a left column - // wrapped on that row too. A label there (`Entry`, `│ Log │`) opens the - // next record, even under a directory ending in `/` (`Workspace - // /home/u/proj/app/` over `Entry main.py`). + // carries a cell's continuation is blank to its left unless a path in a + // left column continued on that row too. A label there (`Entry`, + // `│ Log │`) opens the next record, even under a directory ending in `/` + // (`Workspace /home/u/proj/app/` over `Entry main.py`). A left column of + // wrapped prose (a `Description | Path` table) fills that row as well, so + // the path beside it is not joined and keeps its per-row fragment links: + // the safe direction. // Known trade-off: with nothing to the left, a directory written with its // trailing slash over a relative path in the same column (`/home/u/proj/` // over `index.ts`) still joins, since it reads exactly like a wrap at `/`. diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 9f8cd847..e91c5860 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -241,6 +241,15 @@ const STILL_CONTINUES_ROWS: [string, string, string][] = [ [' Docs: https://example.com/guide?', ' x=1&y=2 more text', 'https://example.com/guide?x=1&y=2'], [' https://example.com/files/', ' 2026%20Plan.xlsx', 'https://example.com/files/2026%20Plan.xlsx'], ]; +// A URL row that soft-wraps at 41 columns with a table column on its wrapped +// part, over an indented row: the column and that row are separate links, never +// one glued `/tmp/report.txtccc/x.html`. +const SOFT_WRAP_COLS = 41; +const SOFT_WRAPPED_URL_ROWS: Row[] = [ + { text: ' Doc https://example.com/aaaaaaaaaaaaaa' }, + { text: 'bbb/ /tmp/report.txt', isWrapped: true }, + { text: ' ccc/x.html' }, +]; describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); @@ -439,6 +448,13 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { expect(extent).not.toBeNull(); expect(extent!.endRow - extent!.startRow + 1).toBeLessThanOrEqual(5); }); + + it('does not continue a URL whose row soft-wraps with a column on the wrapped part', () => { + const buffer = fakeBuffer(SOFT_WRAPPED_URL_ROWS, SOFT_WRAP_COLS); + for (const row of [0, 1, 2]) { + expect(terminalUrlContinuationExtent(buffer, row, SOFT_WRAP_COLS, 12)).toBeNull(); + } + }); }); describe('native OSC 8 terminal URLs', () => { @@ -665,6 +681,14 @@ describe('terminal link provider with wrapped table URLs', () => { expect(segments[2].range.end.x).toBe(TABLE[3].text.length - 1); }); + it('links the column on a soft-wrapped URL row and the row below it separately', () => { + const { provide } = harness(SOFT_WRAPPED_URL_ROWS, SOFT_WRAP_COLS); + const texts = [1, 2, 3].flatMap((line) => provide(line).map((link) => link.text)); + expect(texts.filter((text) => text.includes('report.txtccc'))).toEqual([]); + expect(provide(2).map((link) => link.text)).toContain('/tmp/report.txt'); + expect(provide(3).map((link) => link.text)).toContain('ccc/x.html'); + }); + 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);