From 5ce8213726d748020b03a637e54a7a287fb74fb4 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 28 Oct 2026 20:30:10 +0800 Subject: [PATCH 1/5] fix(links): preserve wrapped configuration filenames --- .changeset/Repair-FilePathLinks.md | 5 ++ BUGS.md | 1 + config/test-suites.ts | 1 + src/web/public/constants.js | 53 +++++++++++----- test/file-path-link-forms.test.ts | 42 +++++++++++-- test/response-viewer-file-links.test.ts | 9 +++ test/spaced-file-path-links.test.ts | 14 +++++ test/wrapped-file-paths.browser.test.ts | 82 +++++++++++++++++++++++++ 8 files changed, 189 insertions(+), 18 deletions(-) create mode 100644 .changeset/Repair-FilePathLinks.md create mode 100644 BUGS.md create mode 100644 test/wrapped-file-paths.browser.test.ts diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md new file mode 100644 index 000000000..d187e66b5 --- /dev/null +++ b/.changeset/Repair-FilePathLinks.md @@ -0,0 +1,5 @@ +--- +'aicodeman': patch +--- + +Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. Preserve the filename's space and clickable row positions when a CLI wraps the path onto another line, instead of opening a truncated folder name. The same detector serves the terminal and Response Viewer, with the existing path guards retained. diff --git a/BUGS.md b/BUGS.md new file mode 100644 index 000000000..9264185e9 --- /dev/null +++ b/BUGS.md @@ -0,0 +1 @@ +Bug tracker: [Codeman issues](https://github.com/Ark0N/Codeman/issues). diff --git a/config/test-suites.ts b/config/test-suites.ts index ae9745422..4760e091e 100644 --- a/config/test-suites.ts +++ b/config/test-suites.ts @@ -19,6 +19,7 @@ * why these are a runnable suite (`npm run test:browser`) rather than skipped. */ export const BROWSER_TEST_GLOBS = [ + 'test/wrapped-file-paths.browser.test.ts', 'test/command-palette-agent-arrows.browser.test.ts', 'test/tmux-capture-sgr.browser.test.ts', 'test/tab-rail-resize.browser.test.ts', diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 742ac363a..62c3dade3 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2106,7 +2106,7 @@ function computeRewriteScrollLine(input) { * a `/g` regex, so {@link absoluteFilePathPattern} mints a fresh one per call. */ const FILE_PATH_LINK_PATTERN = - /(\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; + /(\/(?:home|Users|tmp|var|private|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|conf|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; /** A fresh, zero-state instance of {@link FILE_PATH_LINK_PATTERN}. */ function absoluteFilePathPattern() { @@ -2159,7 +2159,7 @@ function previewsInFileViewer(filePath) { * {@link findFilePathLinks}. */ const RELATIVE_FILE_PATH_LINK_PATTERN = - /(^|[\s"'`(\[{<>=,])((?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)(?:~\/|[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; + /(^|[\s"'`(\[{<>=,])((?![^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*:\/\/)(?:~\/|[^\s"'`<>|;&()[\]{}=,\/~\n\x00-\x1f])[^\s"'`<>|;&()[\]{}=,\n\x00-\x1f]*\.(?:log|txt|json|markdown|md|ya?ml|csv|tsv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html?|toml|ini|conf|sql|diff|patch|png|jpe?g|gif|webp|avif|bmp|ico|svg|pdf|docx|pptx|xlsx|zip|mp4|webm|mov|m4v|ogv|mp3|wav))\b/g; /** The `(?:log|txt|…)` extension group, read off the rooted pattern so it cannot drift. */ const FILE_LINK_EXTENSION_GROUP = /\\\.(\(\?:[^()]*\))\)\\b$/.exec(FILE_PATH_LINK_PATTERN.source)[1]; @@ -2195,19 +2195,25 @@ function findFilePathLinks(text) { // A path with spaces is taken whole, or the span it covers links nothing // (spacedFilePathOnLine). Either way no detector below may cut a fragment out of it. const reserved = []; + const isReserved = (index, length) => + reserved.some((r) => index < r.index + r.path.length && r.index < index + length); + const reserve = (spaced, index) => { + if (!spaced) return; + const start = index + spaced.index; + if (inUrl(start, spaced.path.length) || isReserved(start, spaced.path.length)) return; + reserved.push({ index: start, path: spaced.path }); + if (spaced.link) found.push({ path: spaced.path, index: start }); + }; let lineStart = 0; for (const line of value.split('\n')) { - const spaced = spacedFilePathOnLine(line); - if (spaced && !inUrl(lineStart + spaced.index, spaced.path.length)) { - const index = lineStart + spaced.index; - reserved.push({ index, path: spaced.path }); - if (spaced.link) found.push({ path: spaced.path, index }); + reserve(spacedFilePathOnLine(line), lineStart); + // Parentheses bound an inline path just as a whole line bounds a standalone path. + const parenthesised = /\(([^()\r\n]{1,1000})\)/g; + while ((match = parenthesised.exec(line)) !== null) { + reserve(spacedFilePathOnLine(match[1]), lineStart + match.index + 1); } lineStart += line.length + 1; } - const isReserved = (index, length) => - reserved.some((r) => index < r.index + r.path.length && r.index < index + length); - const absolute = absoluteFilePathPattern(); while ((match = absolute.exec(value)) !== null) { if (!inUrl(match.index, match[1].length) && !isReserved(match.index, match[1].length)) { @@ -2548,7 +2554,8 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] * Whether the line `nextText` carries on a file path `prevText` broke off. * * Answers `'join'` when the path ends (reaches a known extension) on the next - * line, `'chain'` when the next line is nothing but more of the path (a path + * line, `'join-spaced'` for a filename word wrapped inside parentheses, and + * `'chain'` when the next line is nothing but more of the path (a path * spanning three or more lines; only worth keeping once a later line ends it), * and `null` otherwise. The broken-off token must already look like a path in * progress: it contains a `/`, ends on a path character and does not yet end @@ -2557,9 +2564,25 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] * top directory the broken token did (the next entry of a listing). */ function pathContinuation(prevText, nextText) { - const tail = /[^\s"'<>|;&]+$/.exec(String(prevText || '').replace(/\s+$/, ''))?.[0] || ''; - if (!tail.includes('/') || !/[\w\-.~+@%/]$/.test(tail) || PATH_TOKEN_ENDS_WITH_EXTENSION.test(tail)) return null; + const previous = String(prevText || '').trimEnd(); const next = String(nextText || '').replace(/^\s+/, ''); + const opening = previous.lastIndexOf('('); + const closing = next.indexOf(')'); + if (opening >= 0 && closing > 0) { + const before = previous.slice(opening + 1); + const after = next.slice(0, closing); + // A word wrap in a parenthesised filename replaces its separating space. + if ( + !before.includes(')') && + !/[/-]$/.test(before) && + !after.includes('/') && + spacedFilePathOnLine(`${before} ${after}`)?.link + ) { + return 'join-spaced'; + } + } + const tail = /[^\s"'<>|;&]+$/.exec(previous)?.[0] || ''; + if (!tail.includes('/') || !/[\w\-.~+@%/]$/.test(tail) || PATH_TOKEN_ENDS_WITH_EXTENSION.test(tail)) return null; const head = /^[^\s"'<>|;&]+/.exec(next)?.[0] || ''; if (!head || PATH_TOKEN_STARTS_NEW_PATH.test(head) || PATH_TOKEN_STARTS_EXPLICIT_RELATIVE.test(head)) return null; // A listing (`git ls-files`, `find`, `ls -R`) puts one path per row, so `src/components` @@ -2616,9 +2639,10 @@ function terminalPathLine(buffer, row, cols, maxRows) { const kind = continues(parts[parts.length - 1], next, text); if (!kind) break; parts.push(next); + if (kind === 'join-spaced') text += ' '; text += next.text.replace(/^\s+/, ''); rows += rowsOf(next); - if (kind === 'join') confirmed = parts.length; + if (kind !== 'chain') confirmed = parts.length; } return parts.slice(0, confirmed); }; @@ -2640,6 +2664,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { const segments = []; let text = ''; for (const [i, part] of parts.entries()) { + if (i > 0 && continues(parts[i - 1], part, text) === 'join-spaced') text += ' '; const skip = i === 0 ? 0 : part.text.length - part.text.replace(/^\s+/, '').length; segments.push({ part, textStart: text.length, skip }); text += part.text.slice(skip); diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index e601bc9ec..0383459c1 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -52,13 +52,13 @@ const { findFilePathLinks, resolveLinkedFilePath, terminalPathLine } = loadHelpe const paths = (text: string) => findFilePathLinks(text).map((f) => f.path); /** An xterm buffer stand-in: hard-broken rows (no `isWrapped`), as Claude Code emits them. */ -function buffer(lines: string[], cols = 80) { +function buffer(lines: string[], cols = 80, wrappedRows = new Set()) { return { length: lines.length, getLine: (row: number) => row >= 0 && row < lines.length ? { - isWrapped: false, + isWrapped: wrappedRows.has(row), translateToString: (trim?: boolean) => (trim ? lines[row].replace(/\s+$/, '') : lines[row].padEnd(cols)), } : undefined, @@ -157,6 +157,10 @@ describe('terminalPathLine', () => { // A path that already ended, and a next line opening a new rooted path. expect(terminalPathLine(buffer(['see /tmp/a/b.png', 'c/d.png']), 0, 80)).toMatchObject({ endRow: 0 }); expect(terminalPathLine(buffer(['/home/joe/dir/', '/home/x.png']), 0, 80)).toMatchObject({ endRow: 0 }); + expect(terminalPathLine(buffer(['see (/mnt/raid1/folder)', ' file.conf)']), 0, 80)).toMatchObject({ endRow: 0 }); + expect(terminalPathLine(buffer(['see (/mnt/raid1/folder', ' /mnt/other/file.conf)']), 0, 80)).toMatchObject({ + endRow: 0, + }); }); // `git ls-files`, `find` and `ls -R` print one path per row. An extensionless @@ -241,9 +245,9 @@ function loadApp() { return app; } -function linksOn(lines: string[], row: number) { +function linksOn(lines: string[], row: number, cols = 80, wrappedRows = new Set()) { const app = loadApp(); - app.terminal = { cols: 80, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines) } }; + app.terminal = { cols, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines, cols, wrappedRows) } }; const provider = app.registerFilePathLinkProvider() as Provider; let out: Link[] = []; provider.provideLinks(row, (links) => { @@ -253,6 +257,36 @@ function linksOn(lines: string[], row: number) { } describe('terminal link provider', () => { + it('retains the filename space when the CLI word-wraps a parenthesised path', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const rows = [ + 'Found it: I prepared a corrected configuration (/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo', + ' eReader.conf).', + ]; + for (const row of [1, 2]) { + const { app, links } = linksOn(rows, row, 120); + expect(links.map((link) => link.text)).toEqual([path]); + expect(links[0].range.end).toEqual({ x: ' eReader.conf'.length + 1, y: 2 }); + links[0].activate({}, links[0].text); + expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1'); + } + }); + + it('opens the complete parenthesised configuration path from either wrapped row', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Found it: I prepared a corrected configuration (${path}).`; + const split = text.indexOf(' eReader') + 1; + const rows = [text.slice(0, split), text.slice(split)]; + for (const row of [1, 2]) { + const { app, links } = linksOn(rows, row, split, new Set([1])); + expect(links.map((link) => link.text)).toEqual([path]); + expect(links[0].range.start).toEqual({ x: text.indexOf(path) + 1, y: 1 }); + expect(links[0].range.end).toEqual({ x: 'eReader.conf'.length + 1, y: 2 }); + links[0].activate({}, links[0].text); + expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1'); + } + }); + it('links a broken path once, across both rows, and opens the whole path', () => { const { app, links } = linksOn(BROKEN, 2); expect(links.map((l) => l.text)).toEqual([BROKEN_PATH]); diff --git a/test/response-viewer-file-links.test.ts b/test/response-viewer-file-links.test.ts index 0eff0b357..723d233be 100644 --- a/test/response-viewer-file-links.test.ts +++ b/test/response-viewer-file-links.test.ts @@ -78,6 +78,15 @@ function linkify(html: string): HTMLElement { const paths = (root: HTMLElement) => Array.from(root.querySelectorAll('a.rv-path')); describe('response viewer file-path linkifier', () => { + it('links a parenthesised configuration filename with spaces without changing the prose', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Prepared a corrected configuration (${path}).`; + const root = linkify(`

${text}

`); + expect(paths(root).map((a) => a.dataset.path)).toEqual([path]); + expect(paths(root)[0].dataset.folder).toBeUndefined(); + expect(root.textContent).toBe(text); + }); + it.each([ ['- 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx', ['Design/Build-SIPOC.pptx']], ['- notes/a b/c.md', ['b/c.md']], diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts index ead0aaf94..5b08e1859 100644 --- a/test/spaced-file-path-links.test.ts +++ b/test/spaced-file-path-links.test.ts @@ -28,6 +28,20 @@ const LONG = '/mnt/c/Users/someone/OneDrive - Contoso/01_Projects/Other/kb/out/deliverables/fy27-messaging-market-story.docx'; describe('spaced file paths', () => { + it('keeps a parenthesised configuration filename whole inside a sentence', () => { + const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + const text = `Prepared a corrected configuration (${path}).`; + expect(findFilePathLinks(text)).toEqual([{ path, index: text.indexOf(path) }]); + }); + + it('keeps separate parenthesised paths and the surrounding prose separate', () => { + const first = '/mnt/data/Kobo/Kobo eReader.conf'; + const second = '/home/me/My Docs/notes.md'; + expect(paths(`Updated (${first}) and (${second}).`)).toEqual([first, second]); + expect(paths('Do not link (/etc/private files/settings.conf).')).toEqual([]); + expect(paths('Do not link (/mnt/data/../private files/settings.conf).')).toEqual([]); + }); + it('links an own-line absolute path with spaces whole', () => { expect(paths(LONG)).toEqual([LONG]); expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts new file mode 100644 index 000000000..1a0e9f198 --- /dev/null +++ b/test/wrapped-file-paths.browser.test.ts @@ -0,0 +1,82 @@ +/** Real xterm clicks must open the complete spaced filename from either wrapped row. */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { chromium, type Browser } from 'playwright'; + +const publicDir = resolve(import.meta.dirname, '../src/web/public'); +const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; + +describe('wrapped configuration file links (Chromium)', () => { + let browser: Browser; + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + }); + afterAll(async () => { + await browser.close(); + }); + + it.each(['soft', 'hard'])('opens the complete file from both rows of a %s wrap', async (wrap) => { + const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); + try { + await page.setContent('
'); + await page.addStyleTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/css/xterm.css') }); + await page.addScriptTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/lib/xterm.js') }); + await page.addScriptTag({ content: 'window.CodemanApp = class CodemanApp {};' }); + for (const file of ['constants.js', 'terminal-ui.js']) { + await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); + } + await page.evaluate( + async ({ path, wrap }) => { + const w = window as any; + const first = `Found it: prepared a corrected configuration (${path.slice(0, -' eReader.conf'.length)}`; + const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); + terminal.open(document.getElementById('terminal')); + w.opened = []; + w.app = Object.assign(new w.CodemanApp(), { + terminal, + activeSessionId: 'fixture', + sessions: new Map([['fixture', { workingDir: '/workspace' }]]), + _isExternalPreviewPath: () => true, + openFilePreview: (file: string) => w.opened.push(file), + openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`), + openFolderGrid: (file: string) => w.opened.push(`folder:${file}`), + }); + w.app.registerFilePathLinkProvider(); + const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}eReader.conf).`; + await new Promise((done) => terminal.write(text, done)); + await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); + }, + { path, wrap } + ); + + for (const row of [0, 1]) { + const point = await page.evaluate( + async ({ path, row }) => { + const app = (window as any).app; + const terminal = app.terminal; + const links = await new Promise((done) => app._terminalLinkProvider.provideLinks(row + 1, done)); + const link = links.find((candidate) => candidate.text === path); + if (!link) throw new Error(`The complete filename is not linked on row ${row + 1}`); + const text = terminal.buffer.active.getLine(row).translateToString(true); + const col = row === 0 ? link.range.start.x : text.indexOf('eReader') + 1; + const box = terminal.element.querySelector('.xterm-screen').getBoundingClientRect(); + return { + x: box.left + (col - 0.5) * (box.width / terminal.cols), + y: box.top + (row + 0.5) * (box.height / terminal.rows), + }; + }, + { path, row } + ); + await page.mouse.move(point.x, point.y); + await page.waitForFunction(() => (window as any).app._linkHovered === true); + await page.mouse.click(point.x, point.y); + await page.waitForFunction((count) => (window as any).opened.length === count, row + 1); + expect(await page.evaluate(() => (window as any).opened)).toEqual(Array(row + 1).fill(path)); + await page.mouse.move(0, 0); + } + } finally { + await page.close(); + } + }); +}); From 97d53f6621f4c36ea8b53669bf4a542755506cf2 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 28 Oct 2026 21:45:49 +0800 Subject: [PATCH 2/5] fix(links): resolve ambiguous hard wraps through guarded lookup --- .changeset/Repair-FilePathLinks.md | 2 +- BUGS.md | 1 - docs/architecture-invariants.md | 5 + src/web/public/constants.js | 80 +++++++------ src/web/public/panels-ui.js | 5 +- src/web/public/terminal-ui.js | 24 +++- test/file-path-link-forms.test.ts | 58 ++++++++- test/folder-grid-ui.test.ts | 21 ++++ test/spaced-file-path-links.test.ts | 4 + test/wrapped-file-paths.browser.test.ts | 149 +++++++++++++++--------- 10 files changed, 248 insertions(+), 101 deletions(-) delete mode 100644 BUGS.md diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md index d187e66b5..17c6d62b7 100644 --- a/.changeset/Repair-FilePathLinks.md +++ b/.changeset/Repair-FilePathLinks.md @@ -2,4 +2,4 @@ 'aicodeman': patch --- -Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. Preserve the filename's space and clickable row positions when a CLI wraps the path onto another line, instead of opening a truncated folder name. The same detector serves the terminal and Response Viewer, with the existing path guards retained. +Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Existing filenames take priority, and permission or connection failures stop resolution. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. diff --git a/BUGS.md b/BUGS.md deleted file mode 100644 index 9264185e9..000000000 --- a/BUGS.md +++ /dev/null @@ -1 +0,0 @@ -Bug tracker: [Codeman issues](https://github.com/Ark0N/Codeman/issues). diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index ebba79736..d26a88147 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -413,6 +413,11 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli Spaced paths are considered first by `findFilePathLinks()`: accepted candidates reserve their entire span even when unlinkable, so later detectors cannot link fragments. Relative and `~/` spaced paths require an explicit path label (`Path:`, `File:`, `Files:`, `Saved to:`, `Saved as:`, `Wrote:`, `Written to:`, `Output:`, `Created:`, `Location:`, `Attachment:`), optionally preceded by a marker. Generic labels and bare bullets defer to ordinary detection, including slash-containing prose such as `CI/CD` or `45/45`. A later `/`, `~/`, `./` or `../` word starts a separate path; an earlier word ending in `/` is a separate folder. Absolute paths keep the own-line and marker forms plus the same path labels; any other label (`Error:`, `Note:`, `Saved the report to:`) defers an absolute path too. A rooted candidate needs a directory before its first space, so `/review the changes in src/a.ts` is prose. An earlier word holding a known extension anywhere (`a.ts#L12 foo.ts`) ends the path at that word, a `:` followed by a digit inside an earlier word is a line suffix (`a.ts:42:See b.md`, grep and compiler output) and ends the path there, a run of spaces is a column gap rather than a name, and a leading run of `./` or `../` is not a segment (`Saved to: ../My Docs/plan.md` links whole; an interior `..` is still refused). An unlinkable candidate reserves its span only while its last word holds a `/`, so `/usr/bin/env node build.js` keeps its ordinary links while `/etc/secret files/passwords.txt` links nothing. A path label vouches that a path follows, not for the words before it: a relative candidate whose only `/` word is its last is prose after the label (`Wrote: 120 lines to src/foo.ts`, `Path: see src/foo.ts`) and defers to the ordinary detectors. The cost is a labelled relative path whose only space is in its top directory, `Saved to: My Docs/plan.md` (and `Saved to: 03 Attachments/plan.pptx`), which splits as on master; `03 Attachments/Build SIPOC Design/Build-SIPOC.pptx`, `notes/a b/c.md`, `~/`, `./` and `../` forms and absolute paths still link whole. An earlier word that is a lowercase connective or verb (`SPACED_PATH_PROSE_WORD`: `and`, `or`, `to`, `in`, `the`, `is`, `has`, `contains`, `then`, `as` and a few more) or a bare `=` makes the candidate a sentence, in every shape: `/home/me/proj contains a.md`, `- /tmp/out has the logs in run.log`, `Path: /home/me/proj/out contains a.md`, `Files: src/components and utils/x.ts`, `Wrote: 3 files to docs/design notes/overview.md`. The cost is a name that uses one of those words (`Pros and Cons.md`, `How to deploy.md`, `Documents and Settings`), which splits as on master; `at` (`Screenshot 2026-10-24 at 10.00.00.png`) and `of` (`Copy of report.docx`) are left out of the list on purpose. What remains ambiguous is an own-line absolute path followed by words that hold none of them (`/home/me/proj/out report.pdf` reads like `/home/me/docs/Read Me.txt`), which links as one path. A command line is refused too: a first word that is an executable under `bin/`, `.bin/`, `scripts/` or `tools/` (`/opt/homebrew/bin/node server.js`, `/home/me/proj/scripts/deploy prod.yml`) or any `-x`/`--x` flag word (`/home/me/proj/node_modules/.bin/tsc -p tsconfig.json`); a bare `-` (`Docs - Copy`, `OneDrive - Contoso`) is not a flag. So is a shell prompt that prints an absolute working directory (WSL's `/mnt/c/...` is outside `$HOME`, so every earlier command in a shell's scrollback has one): a first word ending in `$` (`/home/me/proj$ cat notes.md`) and any earlier word with no letter or digit other than a bare `-` (`/mnt/c/proj ❯ code notes.md`, `/home/me/proj $ cat notes.md`, `/home/me/proj/out ✓ report.pdf`) refuse the candidate, while `Program Files (x86)` and `Docs - Copy` still link whole. The response viewer carries any leading `` label in a paragraph or list item, or one starting a later line of a paragraph (after a `
`, since chat renders with `breaks: true`), into detection without changing the label or displayed text, and `spacedFilePathOnLine()` judges it as it judges the same line in the terminal (`**Note:** /home/me/proj/out report.pdf` splits). A text node that follows any other inline element starts mid-line, so a placeholder word stands in for what precedes it (``Use `--config` /home/me/proj/cfg data.json`` splits as in the terminal), while a path inside inline code still links whole; paths split across inline elements remain detected per text node. Tests feed vendored marked output into the jsdom linkifier, so consumed bullets and bold labels are covered. Trailing whitespace uses linear `trimEnd()` before the length cap. +Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. A comma at the end of an earlier word marks a list, not one filename. + +Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Permission, connection and server errors stop resolution; hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. + + 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. When other columns follow a wrapped URL on its first row (a codex table's path or date column), the URL is stitched without them, and they still link on that row: `terminalLinkLine()` scans them after the stitched rows as a segment of their own, mapped back to their cells. A URL in such a column is the row's last URL, and only the last URL on a row is continued, so that row is never stitched. 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 62c3dade3..9ef42ceb7 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2166,7 +2166,7 @@ const FILE_LINK_EXTENSION_GROUP = /\\\.(\(\?:[^()]*\))\)\\b$/.exec(FILE_PATH_LIN /** * Every linkable file path in `text`, sorted by position. Spaced candidates - * reserve their whole span first, even when unlinkable; rooted, `~/`, relative + * reserve their whole span first, including paths bounded by parentheses, even when unlinkable; rooted, `~/`, relative * and folder detectors then skip reserved spans, earlier matches and URLs. * * The one entry point for both consumers (terminal link provider, response @@ -2247,8 +2247,9 @@ const SPACED_FILE_PATH_LABEL_PATTERN = * Every path pattern ends a path at whitespace, so `/mnt/c/OneDrive - Contoso/a.docx` * linked `/mnt/c/OneDrive` plus a relative `Contoso/a.docx`, two links to the * wrong targets. A space is only safe inside a path where the line itself bounds - * the path, so a spaced path is recognised in exactly these shapes: + * the path or a parenthesised group bounds it, so a spaced path is recognised in these shapes: * + * Updated (/mnt/c/My Docs/report.docx) inside parentheses in prose * /mnt/c/My Docs/report.docx the whole line is the path * - /mnt/c/My Docs/report.docx after a list marker (`-`, `*`, `•`, `1.`, `2)`) * Saved to: 03 Attachments/Build SIPOC Design/plan.pptx after a path label (SPACED_FILE_PATH_LABEL_PATTERN) @@ -2262,7 +2263,7 @@ const SPACED_FILE_PATH_LABEL_PATTERN = * it ends on a known extension, a rooted one has a directory before its first * space, no earlier word holds an extension (`a.md,`, `a.ts#L12`) or ends in `/`, * and none holds a `:` and a digit (`a.ts:42`, a line suffix ends the path). Later words cannot start a new - * `/`, `~/`, `./` or `../` path, no earlier word ends in `:`, is an arrow or has + * `/`, `~/`, `./` or `../` path, no earlier word ends in `:` or `,`, is an arrow or has * no letter or digit (a prompt glyph such as `❯` or `$`; a bare `-` is kept), and * a run of spaces (a column gap) is refused, so two paths on a line are never * merged. A relative candidate whose only `/` word is its last is prose after the @@ -2309,7 +2310,7 @@ function spacedFilePathOnLine(line) { return null; } for (const word of words.slice(0, -1)) { - if (word.endsWith('/') || word.endsWith(':') || /^(?:—|–|→|=>|->|<-|=)$/.test(word)) return null; + if (word.endsWith('/') || /[,:]$/.test(word) || /^(?:—|–|→|=>|->|<-|=)$/.test(word)) return null; // A shell prompt that prints its working directory in full (`/home/me/proj$ cat // notes.md`; WSL's `/mnt/c/...` is outside `$HOME`) is a command line too, and so is // one whose directory has a space in it (`/mnt/c/My Docs$ ls notes.md`): the `$` @@ -2554,8 +2555,7 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] * Whether the line `nextText` carries on a file path `prevText` broke off. * * Answers `'join'` when the path ends (reaches a known extension) on the next - * line, `'join-spaced'` for a filename word wrapped inside parentheses, and - * `'chain'` when the next line is nothing but more of the path (a path + * line, `'chain'` when the next line is nothing but more of the path (a path * spanning three or more lines; only worth keeping once a later line ends it), * and `null` otherwise. The broken-off token must already look like a path in * progress: it contains a `/`, ends on a path character and does not yet end @@ -2566,21 +2566,6 @@ const pathFirstSegment = (path) => path.replace(/^(?:\.?\/)+/, '').split('/')[0] function pathContinuation(prevText, nextText) { const previous = String(prevText || '').trimEnd(); const next = String(nextText || '').replace(/^\s+/, ''); - const opening = previous.lastIndexOf('('); - const closing = next.indexOf(')'); - if (opening >= 0 && closing > 0) { - const before = previous.slice(opening + 1); - const after = next.slice(0, closing); - // A word wrap in a parenthesised filename replaces its separating space. - if ( - !before.includes(')') && - !/[/-]$/.test(before) && - !after.includes('/') && - spacedFilePathOnLine(`${before} ${after}`)?.link - ) { - return 'join-spaced'; - } - } const tail = /[^\s"'<>|;&]+$/.exec(previous)?.[0] || ''; if (!tail.includes('/') || !/[\w\-.~+@%/]$/.test(tail) || PATH_TOKEN_ENDS_WITH_EXTENSION.test(tail)) return null; const head = /^[^\s"'<>|;&]+/.exec(next)?.[0] || ''; @@ -2609,7 +2594,9 @@ function pathContinuation(prevText, nextText) { * A following logical line is glued on (its indent dropped, cell mapping kept * exact) when {@link pathContinuation} says it carries the path on and the row it * broke off is at least as long as the row it continues on. Bounded by the same - * `maxRows` as the logical line. + * `maxRows` as the logical line. Hard breaks concatenate without inserting spaces. + * An ambiguous parenthesised filename also carries `pathAlternatives` for a + * guarded lookup on activation; its text and cell mapping remain unchanged. * * @returns Same shape as {@link terminalLogicalLine}; null when the row does not exist. */ @@ -2630,6 +2617,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { /** `first` and the lines a path carries on into, stopping at the last confirmed join. */ const extend = (first) => { const parts = [first]; + const alternatives = []; let text = first.text; let rows = rowsOf(first); let confirmed = 1; @@ -2639,32 +2627,44 @@ function terminalPathLine(buffer, row, cols, maxRows) { const kind = continues(parts[parts.length - 1], next, text); if (!kind) break; parts.push(next); - if (kind === 'join-spaced') text += ' '; + if (kind === 'join') { + const opening = text.lastIndexOf('('); + const closing = next.text.indexOf(')'); + if (opening >= 0 && closing > 0) { + const before = text.slice(opening + 1); + const after = next.text.slice(0, closing).trimStart(); + const spaced = spacedFilePathOnLine(`${before} ${after}`); + if (!before.includes(')') && !/[/-]$/.test(before) && !after.includes('/') && spaced?.link) { + alternatives.push({ path: before + after, spacedPath: spaced.path }); + } + } + } text += next.text.replace(/^\s+/, ''); rows += rowsOf(next); - if (kind !== 'chain') confirmed = parts.length; + if (kind === 'join') confirmed = parts.length; } - return parts.slice(0, confirmed); + return { parts: parts.slice(0, confirmed), alternatives }; }; - // The hovered row may be the second or third line of a broken path: walk back - // to where it could start, then let the forward pass decide what really joins. + // A middle fragment may have no slash, so only a forward pass from its + // rooted start can confirm a path spanning three or more rows. + let extended = extend(base); let first = base; let rows = rowsOf(base); while (first.startRow > 0) { const prev = lineAt(first.startRow - 1); - if (!prev || rows + rowsOf(prev) > bound || !continues(prev, first, prev.text)) break; + if (!prev || rows + rowsOf(prev) > bound) break; first = prev; rows += rowsOf(prev); + const candidate = extend(first); + if (candidate.parts[candidate.parts.length - 1].endRow >= base.startRow) extended = candidate; } - let parts = extend(first); - if (parts[parts.length - 1].endRow < base.startRow) parts = extend(base); + const { parts, alternatives } = extended; if (parts.length === 1) return parts[0]; const segments = []; let text = ''; for (const [i, part] of parts.entries()) { - if (i > 0 && continues(parts[i - 1], part, text) === 'join-spaced') text += ' '; const skip = i === 0 ? 0 : part.text.length - part.text.replace(/^\s+/, '').length; segments.push({ part, textStart: text.length, skip }); text += part.text.slice(skip); @@ -2684,7 +2684,14 @@ function terminalPathLine(buffer, row, cols, maxRows) { } return -1; }; - return { startRow: parts[0].startRow, endRow: parts[parts.length - 1].endRow, text, offsetToCell, cellToOffset }; + return { + startRow: parts[0].startRow, + endRow: parts[parts.length - 1].endRow, + text, + offsetToCell, + cellToOffset, + ...(alternatives.length ? { pathAlternatives: alternatives } : {}), + }; } /** @@ -2940,7 +2947,14 @@ function terminalLinkLine(buffer, row, cols, maxRows) { for (const s of segments) if (s.row === targetRow && (!seg || s.colStart <= targetCol)) seg = s; return seg ? seg.textStart + Math.max(0, targetCol - seg.colStart) : -1; }; - return { startRow, endRow: lastRow, text, offsetToCell, cellToOffset }; + return { + startRow, + endRow: lastRow, + text, + offsetToCell, + cellToOffset, + ...(base.pathAlternatives ? { pathAlternatives: base.pathAlternatives } : {}), + }; } /** diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index fef69c904..5dc971e66 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -4204,7 +4204,7 @@ Object.assign(CodemanApp.prototype, { * full attachment guard (blocked secret trees, extension allowlist, symlinks * resolved), so a refusal here is a policy answer worth showing verbatim. * - * @returns {Promise<{attachmentId?: string, size?: number, error?: string}>} + * @returns {Promise<{attachmentId?: string, size?: number, error?: string, status?: number}>} */ async _registerExternalPreview(filePath, sessionId) { try { @@ -4224,10 +4224,11 @@ Object.assign(CodemanApp.prototype, { if (/unsupported/i.test(reason)) { const ext = (filePath.split('.').pop() || '').toLowerCase(); return { + status: res.status, error: `Cannot preview .${ext} from outside the session workspace (images, video, audio, PDF, Office documents and text files only).`, }; } - return { error: reason }; + return { error: reason, status: res.status }; } catch (err) { return { error: err.message || 'Cannot open this file' }; } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 0a37bc46e..0f6d50426 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2318,7 +2318,7 @@ 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) => { + const activateFilePath = (text, folder = false, verifiedSessionId = null) => { // 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 @@ -2336,8 +2336,26 @@ Object.assign(CodemanApp.prototype, { // 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 sessionId = verifiedSessionId || getSessionId(); + if (!sessionId) return; const target = self._resolveLinkedFilePath(text, sessionId); + const alternative = !folder && logical.pathAlternatives?.find((candidate) => candidate.path === text); + if (alternative && !verifiedSessionId) { + return (async () => { + let path = target; + let registered = await self._registerExternalPreview(path, sessionId); + if (registered.status === 404) { + path = self._resolveLinkedFilePath(alternative.spacedPath, sessionId); + registered = await self._registerExternalPreview(path, sessionId); + } + if (!registered.attachmentId) { + self.showToast(registered.error || 'Cannot open this file', 'error'); + return; + } + // The normal opener rechecks its guards; attachment registration deduplicates the path. + return activateFilePath(path, false, 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) { @@ -2372,7 +2390,7 @@ Object.assign(CodemanApp.prototype, { underline: true, }, activate(_event, text) { - activateFilePath(text, folder); + return activateFilePath(text, folder); }, hover() { setHovered(true); diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index 0383459c1..ffd430b6a 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -241,6 +241,8 @@ function loadApp() { app.sessions = new Map([['s1', { workingDir: '/mnt/build/neon_getaway-mgr-hebel' }]]); app.openFilePreview = vi.fn(); app.openLogViewerWindow = vi.fn(); + app._registerExternalPreview = vi.fn(async () => ({ attachmentId: 'att-fixture' })); + app.showToast = vi.fn(); app._isExternalPreviewPath = (p: string) => p.startsWith('~/'); return app; } @@ -257,7 +259,30 @@ function linksOn(lines: string[], row: number, cols = 80, wrappedRows = new Set< } describe('terminal link provider', () => { - it('retains the filename space when the CLI word-wraps a parenthesised path', () => { + it.each([ + [ + ['Saved the screenshot (/home/me/proj/out/ghost1b_game', ' play.png).'], + '/home/me/proj/out/ghost1b_gameplay.png', + ], + [ + [' ⎿ Wrote (/mnt/build/neon/builds/captures/ghost1b_gamepl', ' ay.png) ok'], + '/mnt/build/neon/builds/captures/ghost1b_gameplay.png', + ], + [ + ['Saved the screenshot (/home/me/proj/out/ghost1b_', ' very_long_game', ' play.png).'], + '/home/me/proj/out/ghost1b_very_long_gameplay.png', + ], + ])('preserves a parenthesised filename without spaces across hard wraps: %s', async (rows, path) => { + for (let row = 1; row <= rows.length; row++) { + const { app, links } = linksOn(rows, row, 120); + expect(links.map((link) => link.text)).toEqual([path]); + await links[0].activate({}, links[0].text); + expect(app.openFilePreview).toHaveBeenCalledWith(path, 's1'); + expect(app._registerExternalPreview.mock.calls).toEqual([[path, 's1']]); + } + }); + + it('uses the spaced filename only when the unspaced hard-wrap target is missing', async () => { const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; const rows = [ 'Found it: I prepared a corrected configuration (/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo', @@ -265,13 +290,40 @@ describe('terminal link provider', () => { ]; for (const row of [1, 2]) { const { app, links } = linksOn(rows, row, 120); - expect(links.map((link) => link.text)).toEqual([path]); + const unspaced = path.replace('Kobo eReader', 'KoboeReader'); + expect(links.map((link) => link.text)).toEqual([unspaced]); expect(links[0].range.end).toEqual({ x: ' eReader.conf'.length + 1, y: 2 }); - links[0].activate({}, links[0].text); + app._registerExternalPreview.mockImplementation(async (candidate: string) => + candidate === path ? { attachmentId: 'att-fixture' } : { status: 404, error: 'File not found' } + ); + await links[0].activate({}, links[0].text); expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1'); + expect(app._registerExternalPreview.mock.calls).toEqual([ + [unspaced, 's1'], + [path, 's1'], + ]); } }); + it.each([403, 500, 503])('does not try another path after a guarded lookup returns HTTP %s', async (status) => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + app._registerExternalPreview.mockResolvedValue({ status, error: 'Lookup refused' }); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview).toHaveBeenCalledTimes(1); + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(app.openLogViewerWindow).not.toHaveBeenCalled(); + expect(app.showToast).toHaveBeenCalledWith('Lookup refused', 'error'); + }); + + it('keeps the session selected at click time while a path lookup is pending', async () => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + const pending = links[0].activate({}, links[0].text); + app.activeSessionId = 'another-session'; + await pending; + expect(app._registerExternalPreview).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); + }); + it('opens the complete parenthesised configuration path from either wrapped row', () => { const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; const text = `Found it: I prepared a corrected configuration (${path}).`; diff --git a/test/folder-grid-ui.test.ts b/test/folder-grid-ui.test.ts index 53f848590..cbfdc8e01 100644 --- a/test/folder-grid-ui.test.ts +++ b/test/folder-grid-ui.test.ts @@ -212,6 +212,27 @@ function loadApp(fetchImpl: (url: string, init?: { body?: string }) => unknown) return { app, elements, keyHandlers, fetch: context.fetch as ReturnType }; } +describe('guarded attachment registration results', () => { + it.each([404, 403, 500, 503])('preserves HTTP %s for the activation decision', async (status) => { + const { app, fetch } = loadApp(async () => ({ + ok: false, + status, + json: async () => ({ error: 'Cannot read file' }), + })); + const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, '/mnt/data/a.conf', 's1'); + expect(result).toEqual({ error: 'Cannot read file', status }); + expect(JSON.parse(fetch.mock.calls[0][1].body)).toEqual({ path: '/mnt/data/a.conf', notify: false }); + }); + + it('does not turn a connection failure into a missing file', async () => { + const { app } = loadApp(async () => { + throw new Error('Connection lost'); + }); + const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, '/mnt/data/a.conf', 's1'); + expect(result).toEqual({ error: 'Connection lost' }); + }); +}); + const LISTING = { folderId: 'fld_1', path: '/mnt/raid1/styleframes', diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts index 5b08e1859..a768bb587 100644 --- a/test/spaced-file-path-links.test.ts +++ b/test/spaced-file-path-links.test.ts @@ -42,6 +42,10 @@ describe('spaced file paths', () => { expect(paths('Do not link (/mnt/data/../private files/settings.conf).')).toEqual([]); }); + it('does not take a comma-separated list inside parentheses as one spaced filename', () => { + expect(paths('Updated (/home/me/proj, config.json)')).toEqual(['/home/me/proj', 'config.json']); + }); + it('links an own-line absolute path with spaces whole', () => { expect(paths(LONG)).toEqual([LONG]); expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts index 1a0e9f198..983e695cd 100644 --- a/test/wrapped-file-paths.browser.test.ts +++ b/test/wrapped-file-paths.browser.test.ts @@ -16,67 +16,100 @@ describe('wrapped configuration file links (Chromium)', () => { await browser.close(); }); - it.each(['soft', 'hard'])('opens the complete file from both rows of a %s wrap', async (wrap) => { - const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); - try { - await page.setContent('
'); - await page.addStyleTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/css/xterm.css') }); - await page.addScriptTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/lib/xterm.js') }); - await page.addScriptTag({ content: 'window.CodemanApp = class CodemanApp {};' }); - for (const file of ['constants.js', 'terminal-ui.js']) { - await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); - } - await page.evaluate( - async ({ path, wrap }) => { - const w = window as any; - const first = `Found it: prepared a corrected configuration (${path.slice(0, -' eReader.conf'.length)}`; - const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); - terminal.open(document.getElementById('terminal')); - w.opened = []; - w.app = Object.assign(new w.CodemanApp(), { - terminal, - activeSessionId: 'fixture', - sessions: new Map([['fixture', { workingDir: '/workspace' }]]), - _isExternalPreviewPath: () => true, - openFilePreview: (file: string) => w.opened.push(file), - openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`), - openFolderGrid: (file: string) => w.opened.push(`folder:${file}`), + it.each([ + { wrap: 'soft', originalExists: false }, + { wrap: 'hard', originalExists: false }, + { wrap: 'hard', originalExists: true }, + ])( + 'opens the existing file from both rows: $wrap, original exists: $originalExists', + async ({ wrap, originalExists }) => { + const original = path.replace('Kobo eReader', 'KoboeReader'); + const expected = wrap === 'hard' && originalExists ? original : path; + const lookups: string[] = []; + const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); + try { + await page.route('http://fixture.test/', (route) => + route.fulfill({ + contentType: 'text/html', + body: '
', + }) + ); + await page.route('http://fixture.test/api/sessions/fixture/attachments', async (route) => { + const body = route.request().postDataJSON(); + expect(body.notify).toBe(false); + lookups.push(body.path); + const exists = body.path === path || (originalExists && body.path === original); + await route.fulfill({ + status: exists ? 200 : 404, + json: exists + ? { success: true, data: { attachmentId: 'att-fixture' } } + : { success: false, error: 'File not found' }, }); - w.app.registerFilePathLinkProvider(); - const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}eReader.conf).`; - await new Promise((done) => terminal.write(text, done)); - await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); - }, - { path, wrap } - ); - - for (const row of [0, 1]) { - const point = await page.evaluate( - async ({ path, row }) => { - const app = (window as any).app; - const terminal = app.terminal; - const links = await new Promise((done) => app._terminalLinkProvider.provideLinks(row + 1, done)); - const link = links.find((candidate) => candidate.text === path); - if (!link) throw new Error(`The complete filename is not linked on row ${row + 1}`); - const text = terminal.buffer.active.getLine(row).translateToString(true); - const col = row === 0 ? link.range.start.x : text.indexOf('eReader') + 1; - const box = terminal.element.querySelector('.xterm-screen').getBoundingClientRect(); - return { - x: box.left + (col - 0.5) * (box.width / terminal.cols), - y: box.top + (row + 0.5) * (box.height / terminal.rows), - }; + }); + await page.goto('http://fixture.test/'); + await page.addStyleTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/css/xterm.css') }); + await page.addScriptTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/lib/xterm.js') }); + await page.addScriptTag({ content: 'window.CodemanApp = class CodemanApp {};' }); + for (const file of ['constants.js', 'terminal-ui.js', 'panels-ui.js']) { + await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); + } + await page.evaluate( + async ({ path, wrap }) => { + const w = window as any; + const first = `Found it: prepared a corrected configuration (${path.slice(0, -' eReader.conf'.length)}`; + const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); + terminal.open(document.getElementById('terminal')); + w.opened = []; + w.app = Object.assign(new w.CodemanApp(), { + terminal, + activeSessionId: 'fixture', + sessions: new Map([['fixture', { workingDir: '/workspace' }]]), + _isExternalPreviewPath: () => true, + openFilePreview: (file: string) => w.opened.push(file), + openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`), + openFolderGrid: (file: string) => w.opened.push(`folder:${file}`), + showToast: (message: string) => w.opened.push(`error:${message}`), + }); + w.app.registerFilePathLinkProvider(); + const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}eReader.conf).`; + await new Promise((done) => terminal.write(text, done)); + await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); }, - { path, row } + { path, wrap } ); - await page.mouse.move(point.x, point.y); - await page.waitForFunction(() => (window as any).app._linkHovered === true); - await page.mouse.click(point.x, point.y); - await page.waitForFunction((count) => (window as any).opened.length === count, row + 1); - expect(await page.evaluate(() => (window as any).opened)).toEqual(Array(row + 1).fill(path)); - await page.mouse.move(0, 0); + + expect(lookups).toEqual([]); + for (const row of [0, 1]) { + const point = await page.evaluate( + async ({ path, row }) => { + const app = (window as any).app; + const terminal = app.terminal; + const links = await new Promise((done) => app._terminalLinkProvider.provideLinks(row + 1, done)); + const link = links.find((candidate) => candidate.text === path); + if (!link) throw new Error(`The complete filename is not linked on row ${row + 1}`); + const text = terminal.buffer.active.getLine(row).translateToString(true); + const col = row === 0 ? link.range.start.x : text.indexOf('eReader') + 1; + const box = terminal.element.querySelector('.xterm-screen').getBoundingClientRect(); + return { + x: box.left + (col - 0.5) * (box.width / terminal.cols), + y: box.top + (row + 0.5) * (box.height / terminal.rows), + }; + }, + { path: wrap === 'hard' ? original : path, row } + ); + await page.mouse.move(point.x, point.y); + await page.waitForFunction(() => (window as any).app._linkHovered === true); + await page.mouse.click(point.x, point.y); + await page.waitForFunction((count) => (window as any).opened.length === count, row + 1); + expect(await page.evaluate(() => (window as any).opened)).toEqual(Array(row + 1).fill(expected)); + await page.mouse.move(0, 0); + } + expect(lookups).toEqual( + wrap === 'soft' ? [] : originalExists ? [original, original] : [original, path, original, path] + ); + } finally { + await page.close(); } - } finally { - await page.close(); } - }); + ); }); From fd0e52496d7937fe25940b77639db07fd5728cd4 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 28 Oct 2026 23:03:45 +0800 Subject: [PATCH 3/5] fix(links): preserve workspace openers after lookup refusals --- .changeset/Repair-FilePathLinks.md | 2 +- CLAUDE.md | 2 +- docs/architecture-invariants.md | 3 +- src/web/public/constants.js | 2 + src/web/public/terminal-ui.js | 11 +++--- test/file-path-link-forms.test.ts | 52 +++++++++++++++++++++++-- test/wrapped-file-paths.browser.test.ts | 37 ++++++++++-------- 7 files changed, 80 insertions(+), 29 deletions(-) diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md index 17c6d62b7..c9cafa3b3 100644 --- a/.changeset/Repair-FilePathLinks.md +++ b/.changeset/Repair-FilePathLinks.md @@ -2,4 +2,4 @@ 'aicodeman': patch --- -Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Existing filenames take priority, and permission or connection failures stop resolution. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. +Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Every original response other than not-found retains the normal opener, including workspace files whose extensions the attachment registry rejects. A spaced alternative opens only after successful registration. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. diff --git a/CLAUDE.md b/CLAUDE.md index 3926a6b08..d1234f37d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -226,7 +226,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Prompt uploads** (`POST /api/sessions/:id/paste-image`: clipboard paste, drop, the Compose Attach key and the phone toolbar's Attach button; client wiring in `image-input.js` and `keyboard-accessory.js`): a pick is written to `/.codeman-uploads/` and its path lands on the prompt as unsent text. The folder is IN the workspace because that is the only path identical for a local and a container agent, FLAT because `/.codeman/` would BE the data dir when the workspace is the home directory, and self-ignoring through a `.gitignore` of `*` written ONCE with `wx` (a file already there is the user's and is never overwritten; the user's own ignore file is never touched). ⚠️ The directory names live ONLY in `paste-image-gc.ts` (`UPLOAD_DIR_NAMES`): the pre-move `.claude-images/` receives nothing but stays readable for one release, so the GC sweep and the delete cleanup (through `uploadDirs()`) and the image watcher's ignore filter (reading the names) cover BOTH; retire the legacy entry there, never by hand elsewhere. ⚠️ `uploadDirs()` is async and probes the workspace BOUNDED first (`probePathKind()`, the #516 rule: an `unknown` path is skipped, never touched; the sweep keeps the stall cap and the delete passes `pastCap`), then returns only REAL directories (lstat, so a planted symlink is never one), none that is or contains `getDataDir()` (one strictly inside it only holds uploads and is listed), and none for a remote session, since both consumers delete; the check is made at list time, and the same-user race after it is accepted. Images are buffered in memory under `MAX_PASTE_IMAGE_BYTES` (50MB) after a client-side canvas re-encode; a DOCUMENT (pdf, docx, pptx, xlsx, zip and the text family txt/md/csv/tsv/json/log/yaml/yml/xml; the curated `ALLOWED_DOCUMENT_EXTS`, never `SUPPORTED_ATTACHMENT_EXTENSIONS` or `EDITABLE_EXTENSIONS`, which carry sh/py/js/html) is admitted by its name's extension alone, buffered like an image, judged on its bytes by the same fail-closed `imageMagicMatchesExt` (`%PDF-`, `PK\x03\x04` for the OOXML trio and zip, UTF-8 without a NUL byte for text: a SIGNATURE or ENCODING check, never a structure or content check, so generic zip bytes pass as `.docx` and a script renamed `.txt` is text; the sh/py/js/html exclusion is a SCOPE choice by extension, keeping the server from storing what it would serve or run, not a content filter: the agent can already write any of those) and saved as `paste---` with a sanitized stem (`[A-Za-z0-9._-]`, 40 chars, a wholly non-ASCII name leaves no stem; the `paste-` prefix is what the GC matches). A VIDEO is never buffered: `?kind=video` streams it to disk under `MAX_PASTE_VIDEO_BYTES` (1GB, env `CODEMAN_MAX_PASTE_VIDEO_BYTES`), judges the container on the first 12 bytes as they arrive (ISO BMFF / QuickTime atom, EBML for webm, Ogg for ogv; a mislabeled body is drained unwritten from there, never parked on disk first) and unlinks on any failure, a dead transfer included. ⚠️ The flag is the ONLY way onto that path and admits video types alone (the registry's `VIDEO_ATTACHMENT_EXTENSIONS`, single-sourced: mp4/webm/mov/m4v/ogv, so an uploaded video is one the file viewer can also play; 3gp/mkv are out, since a phone's camera roll produces neither and neither previews), so an image can never ride the larger cap through memory; without it the route is byte-identical to the image-only version. Busboy takes its size limit before the part is seen, which is why the choice rides the URL and not the file. ⚠️ A video draws on its OWN per-(IP, session) rate bucket (`consumeVideoToken`, 3/min) beside the image bucket (`consumePasteToken`, 30/min): at 1GB each, 30/min would be 30GB of disk a minute, so the lower rate bounds it to 3GB; the image path keeps the 30/min bucket and is unchanged without the flag. A remote (SSH) session is refused (400) before any disk touch: its `workingDir` is the remote path, so the file would land on THIS host where the agent cannot read it (Docker is fine, the workspace is bind-mounted at the same absolute path). ⚠️ A live session's workspace can still sit on a network mount that stopped answering, so the route probes it BOUNDED (`probePathKind(session.workingDir, { pastCap: true })`, the #516 rule) before the first disk touch and refuses an `unknown` mount through `describeUnknownPath()` (OPERATION_FAILED), the way session create does, so a mount already unreachable cannot freeze a libuv worker on every upload (a mount that dies after the probe is the same residual race create accepts). ⚠️ Dispatch mirrors the client: an `image/*` MIME takes the IMAGE path whatever the name says (a GIF named `animation.txt` stays a GIF), a document extension takes the document branch, and anything else takes the image path exactly as before, where the HEIC sniff still recovers a HEIC under a generic name; a foreign extension with no image MIME gets its `Unsupported file type` 400 only AFTER that sniff fails, never before, so nothing the image path ever accepted by its bytes is refused. A text mismatch is refused as not UTF-8 text (UTF-16 when the file opens with a UTF-16 BOM, as Excel's "Unicode Text" and Notepad's "Unicode" do), telling the user to save it as UTF-8, and its log line names the failure (`utf16-bom`/`nul-byte`/`not-utf8`) and never prints the bytes, since the head of a text file is user content. ⚠️ The client classifies a pick through ONE helper, `_promptAttachKind()` (MIME type first, then the name's extension, because some Android file managers hand over a File with none; returns `image`, `video` or `document`); every pick and paste filter (Attach, drop, the tile grid's drop in `tile-grid.js`, the Compose and paste-dialog pickers, the clipboard handlers), the flag and the re-encode gate read it, never `file.type` directly. Those surfaces ADMIT through `_admitPromptAttachments()` (image-input.js), which calls it and toasts what a pick skipped (how many files of a mixed pick, `Unsupported file type` when none pass, nothing for a text paste); a new surface admits through that helper too, never by filtering on `_promptAttachKind()` itself, or it loses the toasts. ⚠️ A pick that lands while the tile grid owns the terminal goes to the PTY, never the main terminal's overlay: that terminal is parked and hidden then, and a tile's Enter cannot flush it (`_tilesOwnTerminal()` in the `_attachFromToolbar` guard). Tests: `test/toolbar-attach.test.ts`, `test/mobile-prompt-composer.test.ts`, `test/paste-image-gc.test.ts`, `test/paste-image-dir-shared.test.ts`, `test/upload-document-list-parity.test.ts` (the browser's copies of `ALLOWED_DOCUMENT_EXTS`: `_promptAttachKind()`, the three pickers' `accept=` and the two file-path link patterns), the paste-image block of `test/routes/session-routes.test.ts`. → [architecture-invariants#buffers-uploads-and-terminal-history](docs/architecture-invariants.md#buffers-uploads-and-terminal-history) -**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which reserves spaced-path spans first (an unlinkable one only while its last word holds a `/`; any label, before an absolute or relative path, must be an explicit path label, prose after that label stays out, and an earlier connective or verb such as `and` or `has` (`SPACED_PATH_PROSE_WORD`) makes any candidate prose; rules and trade-offs in the architecture-invariants paragraph linked below), then adds rooted, `~/`, relative and folder paths without overlapping a reservation, earlier match or URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. ⚠️ `TEXT_ATTACHMENT_EXTENSIONS` IS `EDITABLE_EXTENSIONS` (never a second list), and widening READ must never widen RUN: `html`/`htm`/`svg` stay download-only, other text is inert `text/plain`+`nosniff`. ⚠️ The ONE place HTML renders is `/html-view//...` (`html-view-routes.ts`, `html-view-capabilities.ts`): `POST /api/sessions/:id/html-view` mints a capability for the file's own directory, every response carries `Content-Security-Policy: sandbox` WITHOUT `allow-same-origin` (opaque origin even as a top-level tab, so the page can never read Codeman or drive its API) and with NO fetch directive (open network, owner decision 2026-10-13: never restrict it or add `allow-same-origin` without the owner), and only non-dot, asset-allowlisted files under that directory are served (GET/HEAD only; capabilities revoked with the web-tab ones on logout). That tree is served recursively, so minting is refused for a hidden directory or a broad root (`isBroadHtmlViewRoot`); the preview renders by default and the **Page** pill (or a refused mint) falls through to the source view and its Edit button. Never serve HTML same-origin anywhere else. Tests: `test/routes/html-view-routes.test.ts`. Media extensions are single-sourced in `attachment-registry.ts`. ⚠️ **XLSX previews parse in the BROWSER**, never on the server; the admission limits that keep a hostile workbook from freezing the worker live in `src/web/CLAUDE.md` (Spreadsheet preview). → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) +**File-path links (terminal + chat)**: a path an agent prints is clickable on BOTH surfaces and opens the file-preview overlay. ⚠️ ONE pattern (`FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` in constants.js) feeds the xterm link provider AND `_linkifyFilePaths()`, a fresh instance per call (`lastIndex`). Both call `findFilePathLinks()`, which reserves spaced-path spans first (an unlinkable one only while its last word holds a `/`; any label, before an absolute or relative path, must be an explicit path label, prose after that label stays out, and an earlier connective or verb such as `and` or `has` (`SPACED_PATH_PROSE_WORD`) makes any candidate prose; rules and trade-offs in the architecture-invariants paragraph linked below), then adds rooted, `~/`, relative and folder paths without overlapping a reservation, earlier match or URL; a relative path resolves against `workingDir` (`resolveLinkedFilePath`), `~/` only server-side on the session's host (attachment route), and `terminalPathLine()` joins a path a program hard-wrapped short of the edge. ⚠️ A folder link opens the **folder grid**: `POST /api/sessions/:id/folders` guards + GRANTS the folder (`folder-listing.ts`, own registry, never the 200-capped attachment registry), files come by bare name via `…/folders/:folderId/files/:name/*` through `resolveServableAttachmentPath` plus the same extension/regular-file gate on the symlink's resolved target, so no browser request carries a path; `~/` is refused in a Docker case (the container's home is not this host's). The chat linkifier walks TEXT NODES with DOM APIs, never rebuilds sanitized markup as a string. ⚠️ An out-of-workspace path goes through the ATTACHMENT routes (`POST /api/sessions/:id/attachments` with `notify: false`), never by widening `file-content`/`file-raw` or `file-stream-manager`'s `tail -f` allowlist. An ambiguous parenthesised hard wrap is checked only on activation, original first: only a 404 permits a spaced-path lookup, and only a registered alternative is opened; every other original result uses the normal opener, never an attachment-policy gate on workspace files, and hover performs no lookup. ⚠️ `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; 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. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index d26a88147..31e792f1a 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -415,8 +415,7 @@ Spaced paths are considered first by `findFilePathLinks()`: accepted candidates Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. A comma at the end of an earlier word marks a list, not one filename. -Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Permission, connection and server errors stop resolution; hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. - +Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. 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. When other columns follow a wrapped URL on its first row (a codex table's path or date column), the URL is stitched without them, and they still link on that row: `terminalLinkLine()` scans them after the stitched rows as a segment of their own, mapped back to their cells. A URL in such a column is the row's last URL, and only the last URL on a row is continued, so that row is never stitched. 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. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 9ef42ceb7..36a1c7d15 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2656,6 +2656,8 @@ function terminalPathLine(buffer, row, cols, maxRows) { if (!prev || rows + rowsOf(prev) > bound) break; first = prev; rows += rowsOf(prev); + const tail = /[^\s"'<>|;&]+$/.exec(first.text.trimEnd())?.[0] || ''; + if (!tail.includes('/')) continue; const candidate = extend(first); if (candidate.parts[candidate.parts.length - 1].endRow >= base.startRow) extended = candidate; } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 0f6d50426..d52e6bba2 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2342,12 +2342,11 @@ Object.assign(CodemanApp.prototype, { const alternative = !folder && logical.pathAlternatives?.find((candidate) => candidate.path === text); if (alternative && !verifiedSessionId) { return (async () => { - let path = target; - let registered = await self._registerExternalPreview(path, sessionId); - if (registered.status === 404) { - path = self._resolveLinkedFilePath(alternative.spacedPath, sessionId); - registered = await self._registerExternalPreview(path, sessionId); - } + const original = await self._registerExternalPreview(target, sessionId); + // Attachment policy must not gate the workspace's normal opener. + if (original.status !== 404) return activateFilePath(target, false, sessionId); + const path = self._resolveLinkedFilePath(alternative.spacedPath, sessionId); + const registered = await self._registerExternalPreview(path, sessionId); if (!registered.attachmentId) { self.showToast(registered.error || 'Cannot open this file', 'error'); return; diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index ffd430b6a..b80615d56 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -259,6 +259,27 @@ function linksOn(lines: string[], row: number, cols = 80, wrappedRows = new Set< } describe('terminal link provider', () => { + it.each(['svg', 'markdown', 'avif', 'bmp', 'ico', 'zip'])( + 'keeps the workspace opener for a hard-wrapped .%s rejected by the attachment registry', + async (extension) => { + for (const row of [1, 2]) { + const path = `/home/me/proj/out/system_architecture.${extension}`; + const { app, links } = linksOn( + ['Saved the diagram (/home/me/proj/out/system_archi', ` tecture.${extension}).`], + row, + 120 + ); + app.sessions = new Map([['s1', { workingDir: '/home/me/proj' }]]); + app._registerExternalPreview.mockResolvedValue({ status: 400, error: 'Unsupported attachment type' }); + await links[0].activate({}, links[0].text); + const opener = extension === 'zip' ? app.openLogViewerWindow : app.openFilePreview; + expect(opener).toHaveBeenCalledWith(path, 's1'); + expect(app._registerExternalPreview.mock.calls).toEqual([[path, 's1']]); + expect(app.showToast).not.toHaveBeenCalled(); + } + } + ); + it.each([ [ ['Saved the screenshot (/home/me/proj/out/ghost1b_game', ' play.png).'], @@ -310,11 +331,36 @@ describe('terminal link provider', () => { app._registerExternalPreview.mockResolvedValue({ status, error: 'Lookup refused' }); await links[0].activate({}, links[0].text); expect(app._registerExternalPreview).toHaveBeenCalledTimes(1); - expect(app.openFilePreview).not.toHaveBeenCalled(); - expect(app.openLogViewerWindow).not.toHaveBeenCalled(); - expect(app.showToast).toHaveBeenCalledWith('Lookup refused', 'error'); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); + expect(app.showToast).not.toHaveBeenCalled(); + }); + + it('keeps the original opener when the lookup cannot reach the server', async () => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + app._registerExternalPreview.mockResolvedValue({ error: 'Connection lost' }); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview).toHaveBeenCalledTimes(1); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); }); + it.each([400, 403, 404, 500, 503])( + 'does not open an unregistered spaced alternative after HTTP %s', + async (status) => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + app._registerExternalPreview + .mockResolvedValueOnce({ status: 404, error: 'File not found' }) + .mockResolvedValueOnce({ status, error: 'Alternative unavailable' }); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview.mock.calls).toEqual([ + ['/mnt/data/KoboeReader.conf', 's1'], + ['/mnt/data/Kobo eReader.conf', 's1'], + ]); + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(app.openLogViewerWindow).not.toHaveBeenCalled(); + expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error'); + } + ); + it('keeps the session selected at click time while a path lookup is pending', async () => { const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); const pending = links[0].activate({}, links[0].text); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts index 983e695cd..3890bae7d 100644 --- a/test/wrapped-file-paths.browser.test.ts +++ b/test/wrapped-file-paths.browser.test.ts @@ -5,7 +5,7 @@ import { afterAll, beforeAll, describe, expect, it } from 'vitest'; import { chromium, type Browser } from 'playwright'; const publicDir = resolve(import.meta.dirname, '../src/web/public'); -const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; +const basePath = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf'; describe('wrapped configuration file links (Chromium)', () => { let browser: Browser; @@ -17,12 +17,15 @@ describe('wrapped configuration file links (Chromium)', () => { }); it.each([ - { wrap: 'soft', originalExists: false }, - { wrap: 'hard', originalExists: false }, - { wrap: 'hard', originalExists: true }, + { wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200 }, + { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200 }, + { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200 }, + { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400 }, + { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400 }, ])( - 'opens the existing file from both rows: $wrap, original exists: $originalExists', - async ({ wrap, originalExists }) => { + 'opens the existing .$extension file from both rows: $wrap, original exists: $originalExists', + async ({ wrap, originalExists, extension, registrationStatus }) => { + const path = basePath.replace('.conf', `.${extension}`); const original = path.replace('Kobo eReader', 'KoboeReader'); const expected = wrap === 'hard' && originalExists ? original : path; const lookups: string[] = []; @@ -39,11 +42,13 @@ describe('wrapped configuration file links (Chromium)', () => { expect(body.notify).toBe(false); lookups.push(body.path); const exists = body.path === path || (originalExists && body.path === original); + const status = exists ? registrationStatus : 404; await route.fulfill({ - status: exists ? 200 : 404, - json: exists - ? { success: true, data: { attachmentId: 'att-fixture' } } - : { success: false, error: 'File not found' }, + status, + json: + status === 200 + ? { success: true, data: { attachmentId: 'att-fixture' } } + : { success: false, error: status === 400 ? 'Unsupported attachment type' : 'File not found' }, }); }); await page.goto('http://fixture.test/'); @@ -54,28 +59,28 @@ describe('wrapped configuration file links (Chromium)', () => { await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); } await page.evaluate( - async ({ path, wrap }) => { + async ({ path, wrap, workingDir }) => { const w = window as any; - const first = `Found it: prepared a corrected configuration (${path.slice(0, -' eReader.conf'.length)}`; + const boundary = path.lastIndexOf(' '); + const first = `Found it: prepared a corrected configuration (${path.slice(0, boundary)}`; const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); terminal.open(document.getElementById('terminal')); w.opened = []; w.app = Object.assign(new w.CodemanApp(), { terminal, activeSessionId: 'fixture', - sessions: new Map([['fixture', { workingDir: '/workspace' }]]), - _isExternalPreviewPath: () => true, + sessions: new Map([['fixture', { workingDir }]]), openFilePreview: (file: string) => w.opened.push(file), openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`), openFolderGrid: (file: string) => w.opened.push(`folder:${file}`), showToast: (message: string) => w.opened.push(`error:${message}`), }); w.app.registerFilePathLinkProvider(); - const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}eReader.conf).`; + const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}${path.slice(boundary + 1)}).`; await new Promise((done) => terminal.write(text, done)); await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); }, - { path, wrap } + { path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace' } ); expect(lookups).toEqual([]); From 168c15ae087d962439067778f802c8c9bdb28fdb Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Thu, 29 Oct 2026 00:17:48 +0800 Subject: [PATCH 4/5] fix(links): scope comma handling to parenthesised paths --- .changeset/Repair-FilePathLinks.md | 2 ++ docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 12 ++++++---- src/web/public/panels-ui.js | 9 ++++--- test/folder-grid-ui.test.ts | 32 +++++++++++++++++++++++++ test/spaced-file-path-links.test.ts | 8 +++++++ test/wrapped-file-paths.browser.test.ts | 17 ++++++++++--- 7 files changed, 70 insertions(+), 12 deletions(-) diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md index c9cafa3b3..3418dccc9 100644 --- a/.changeset/Repair-FilePathLinks.md +++ b/.changeset/Repair-FilePathLinks.md @@ -3,3 +3,5 @@ --- Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Every original response other than not-found retains the normal opener, including workspace files whose extensions the attachment registry rejects. A spaced alternative opens only after successful registration. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. + +Restrict comma-separated-list detection to parenthesised candidates so existing own-line, list-marker and labelled paths with commas remain whole. A rejected workspace path is named in the attachment-lookup error without describing it as outside the workspace. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 31e792f1a..cf823d056 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -413,7 +413,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli Spaced paths are considered first by `findFilePathLinks()`: accepted candidates reserve their entire span even when unlinkable, so later detectors cannot link fragments. Relative and `~/` spaced paths require an explicit path label (`Path:`, `File:`, `Files:`, `Saved to:`, `Saved as:`, `Wrote:`, `Written to:`, `Output:`, `Created:`, `Location:`, `Attachment:`), optionally preceded by a marker. Generic labels and bare bullets defer to ordinary detection, including slash-containing prose such as `CI/CD` or `45/45`. A later `/`, `~/`, `./` or `../` word starts a separate path; an earlier word ending in `/` is a separate folder. Absolute paths keep the own-line and marker forms plus the same path labels; any other label (`Error:`, `Note:`, `Saved the report to:`) defers an absolute path too. A rooted candidate needs a directory before its first space, so `/review the changes in src/a.ts` is prose. An earlier word holding a known extension anywhere (`a.ts#L12 foo.ts`) ends the path at that word, a `:` followed by a digit inside an earlier word is a line suffix (`a.ts:42:See b.md`, grep and compiler output) and ends the path there, a run of spaces is a column gap rather than a name, and a leading run of `./` or `../` is not a segment (`Saved to: ../My Docs/plan.md` links whole; an interior `..` is still refused). An unlinkable candidate reserves its span only while its last word holds a `/`, so `/usr/bin/env node build.js` keeps its ordinary links while `/etc/secret files/passwords.txt` links nothing. A path label vouches that a path follows, not for the words before it: a relative candidate whose only `/` word is its last is prose after the label (`Wrote: 120 lines to src/foo.ts`, `Path: see src/foo.ts`) and defers to the ordinary detectors. The cost is a labelled relative path whose only space is in its top directory, `Saved to: My Docs/plan.md` (and `Saved to: 03 Attachments/plan.pptx`), which splits as on master; `03 Attachments/Build SIPOC Design/Build-SIPOC.pptx`, `notes/a b/c.md`, `~/`, `./` and `../` forms and absolute paths still link whole. An earlier word that is a lowercase connective or verb (`SPACED_PATH_PROSE_WORD`: `and`, `or`, `to`, `in`, `the`, `is`, `has`, `contains`, `then`, `as` and a few more) or a bare `=` makes the candidate a sentence, in every shape: `/home/me/proj contains a.md`, `- /tmp/out has the logs in run.log`, `Path: /home/me/proj/out contains a.md`, `Files: src/components and utils/x.ts`, `Wrote: 3 files to docs/design notes/overview.md`. The cost is a name that uses one of those words (`Pros and Cons.md`, `How to deploy.md`, `Documents and Settings`), which splits as on master; `at` (`Screenshot 2026-10-24 at 10.00.00.png`) and `of` (`Copy of report.docx`) are left out of the list on purpose. What remains ambiguous is an own-line absolute path followed by words that hold none of them (`/home/me/proj/out report.pdf` reads like `/home/me/docs/Read Me.txt`), which links as one path. A command line is refused too: a first word that is an executable under `bin/`, `.bin/`, `scripts/` or `tools/` (`/opt/homebrew/bin/node server.js`, `/home/me/proj/scripts/deploy prod.yml`) or any `-x`/`--x` flag word (`/home/me/proj/node_modules/.bin/tsc -p tsconfig.json`); a bare `-` (`Docs - Copy`, `OneDrive - Contoso`) is not a flag. So is a shell prompt that prints an absolute working directory (WSL's `/mnt/c/...` is outside `$HOME`, so every earlier command in a shell's scrollback has one): a first word ending in `$` (`/home/me/proj$ cat notes.md`) and any earlier word with no letter or digit other than a bare `-` (`/mnt/c/proj ❯ code notes.md`, `/home/me/proj $ cat notes.md`, `/home/me/proj/out ✓ report.pdf`) refuse the candidate, while `Program Files (x86)` and `Docs - Copy` still link whole. The response viewer carries any leading `` label in a paragraph or list item, or one starting a later line of a paragraph (after a `
`, since chat renders with `breaks: true`), into detection without changing the label or displayed text, and `spacedFilePathOnLine()` judges it as it judges the same line in the terminal (`**Note:** /home/me/proj/out report.pdf` splits). A text node that follows any other inline element starts mid-line, so a placeholder word stands in for what precedes it (``Use `--config` /home/me/proj/cfg data.json`` splits as in the terminal), while a path inside inline code still links whole; paths split across inline elements remain detected per text node. Tests feed vendored marked output into the jsdom linkifier, so consumed bullets and bold labels are covered. Trailing whitespace uses linear `trimEnd()` before the length cap. -Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. A comma at the end of an earlier word marks a list, not one filename. +Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. Inside parentheses only, a comma at the end of an earlier word marks a list, not one filename. Own-line, list-marker and labelled paths retain commas in filenames and directory names. Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 36a1c7d15..8a6459411 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2210,7 +2210,7 @@ function findFilePathLinks(text) { // Parentheses bound an inline path just as a whole line bounds a standalone path. const parenthesised = /\(([^()\r\n]{1,1000})\)/g; while ((match = parenthesised.exec(line)) !== null) { - reserve(spacedFilePathOnLine(match[1]), lineStart + match.index + 1); + reserve(spacedFilePathOnLine(match[1], { parenthesised: true }), lineStart + match.index + 1); } lineStart += line.length + 1; } @@ -2263,7 +2263,7 @@ const SPACED_FILE_PATH_LABEL_PATTERN = * it ends on a known extension, a rooted one has a directory before its first * space, no earlier word holds an extension (`a.md,`, `a.ts#L12`) or ends in `/`, * and none holds a `:` and a digit (`a.ts:42`, a line suffix ends the path). Later words cannot start a new - * `/`, `~/`, `./` or `../` path, no earlier word ends in `:` or `,`, is an arrow or has + * `/`, `~/`, `./` or `../` path, no earlier word ends in `:` (or `,` inside parentheses), is an arrow or has * no letter or digit (a prompt glyph such as `❯` or `$`; a bare `-` is kept), and * a run of spaces (a column gap) is refused, so two paths on a line are never * merged. A relative candidate whose only `/` word is its last is prose after the @@ -2285,9 +2285,10 @@ const SPACED_FILE_PATH_LABEL_PATTERN = * Linear: one bounded prefix match, then string scans. * * @param {string} line One line, no newline. + * @param {{parenthesised?: boolean}} [options] Refuse comma-separated lists inside parentheses only. * @returns {{path: string, index: number, link: boolean} | null} */ -function spacedFilePathOnLine(line) { +function spacedFilePathOnLine(line, options = {}) { const text = String(line || ''); const prefix = /^[ \t]*((?:\d{1,3}[.)]|[-*•])[ \t]+)?((?:[A-Za-z][\w-]{0,30}(?: [\w-]{1,30}){0,3}):[ \t]+)?/.exec(text); const index = prefix[0].length; @@ -2310,7 +2311,8 @@ function spacedFilePathOnLine(line) { return null; } for (const word of words.slice(0, -1)) { - if (word.endsWith('/') || /[,:]$/.test(word) || /^(?:—|–|→|=>|->|<-|=)$/.test(word)) return null; + if (word.endsWith('/') || word.endsWith(':') || /^(?:—|–|→|=>|->|<-|=)$/.test(word)) return null; + if (options.parenthesised && word.endsWith(',')) return null; // A shell prompt that prints its working directory in full (`/home/me/proj$ cat // notes.md`; WSL's `/mnt/c/...` is outside `$HOME`) is a command line too, and so is // one whose directory has a space in it (`/mnt/c/My Docs$ ls notes.md`): the `$` @@ -2633,7 +2635,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { if (opening >= 0 && closing > 0) { const before = text.slice(opening + 1); const after = next.text.slice(0, closing).trimStart(); - const spaced = spacedFilePathOnLine(`${before} ${after}`); + const spaced = spacedFilePathOnLine(`${before} ${after}`, { parenthesised: true }); if (!before.includes(')') && !/[/-]$/.test(before) && !after.includes('/') && spaced?.link) { alternatives.push({ path: before + after, spacedPath: spaced.path }); } diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js index 5dc971e66..e46baa4a4 100644 --- a/src/web/public/panels-ui.js +++ b/src/web/public/panels-ui.js @@ -4195,7 +4195,7 @@ Object.assign(CodemanApp.prototype, { }, /** - * Register an out-of-workspace path as a live external attachment and return + * Register a file path as a live external attachment and return * its id, so the preview can render it through the by-id attachment routes. * * `notify: false` keeps this quiet: the caller is already opening the file in @@ -4220,12 +4220,15 @@ Object.assign(CodemanApp.prototype, { const reason = result?.error || `Cannot open this file (HTTP ${res.status})`; // The registry's type answer is a policy term, not an explanation, and the // user just clicked a file they can see on disk. Say what IS previewable - // from outside the workspace instead. + // through this lookup instead, using the path's actual workspace scope. if (/unsupported/i.test(reason)) { const ext = (filePath.split('.').pop() || '').toLowerCase(); + const target = this._isExternalPreviewPath(filePath, sessionId) + ? `.${ext} from outside the session workspace` + : `"${filePath}" through the attachment lookup`; return { status: res.status, - error: `Cannot preview .${ext} from outside the session workspace (images, video, audio, PDF, Office documents and text files only).`, + error: `Cannot preview ${target} (images, video, audio, PDF, Office documents and text files only).`, }; } return { error: reason, status: res.status }; diff --git a/test/folder-grid-ui.test.ts b/test/folder-grid-ui.test.ts index cbfdc8e01..131bcdf16 100644 --- a/test/folder-grid-ui.test.ts +++ b/test/folder-grid-ui.test.ts @@ -213,6 +213,38 @@ function loadApp(fetchImpl: (url: string, init?: { body?: string }) => unknown) } describe('guarded attachment registration results', () => { + it.each(['svg', 'markdown', 'avif', 'bmp', 'ico', 'zip'])( + 'names a rejected workspace .%s path without describing it as external', + async (extension) => { + const { app } = loadApp(async () => ({ + ok: false, + status: 400, + json: async () => ({ error: 'Unsupported attachment type' }), + })); + const path = `/home/joe/wiki/out/system architecture.${extension}`; + const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, path, 's1'); + expect(result.status).toBe(400); + expect(result.error).toContain(path); + expect(result.error).not.toContain('outside the session workspace'); + } + ); + + it('retains the external-file explanation for a rejected external type', async () => { + const { app } = loadApp(async () => ({ + ok: false, + status: 400, + json: async () => ({ error: 'Unsupported attachment type' }), + })); + const result = await Object.getPrototypeOf(app)._registerExternalPreview.call( + app, + '/mnt/data/system architecture.svg', + 's1' + ); + expect(result.error).toBe( + 'Cannot preview .svg from outside the session workspace (images, video, audio, PDF, Office documents and text files only).' + ); + }); + it.each([404, 403, 500, 503])('preserves HTTP %s for the activation decision', async (status) => { const { app, fetch } = loadApp(async () => ({ ok: false, diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts index a768bb587..062c58bce 100644 --- a/test/spaced-file-path-links.test.ts +++ b/test/spaced-file-path-links.test.ts @@ -46,6 +46,14 @@ describe('spaced file paths', () => { expect(paths('Updated (/home/me/proj, config.json)')).toEqual(['/home/me/proj', 'config.json']); }); + it.each([ + ['/mnt/c/Users/me/Smith, John/report.docx', '/mnt/c/Users/me/Smith, John/report.docx'], + ['- /home/me/Clients/Acme, Inc/contract.pdf', '/home/me/Clients/Acme, Inc/contract.pdf'], + ['Saved to: /home/me/proj/Invoice 12, final.pdf', '/home/me/proj/Invoice 12, final.pdf'], + ])('keeps commas in a non-parenthesised spaced path: %s', (line, path) => { + expect(findFilePathLinks(line)).toEqual([{ path, index: line.indexOf(path) }]); + }); + it('links an own-line absolute path with spaces whole', () => { expect(paths(LONG)).toEqual([LONG]); expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts index 3890bae7d..14dddf4f8 100644 --- a/test/wrapped-file-paths.browser.test.ts +++ b/test/wrapped-file-paths.browser.test.ts @@ -1,4 +1,4 @@ -/** Real xterm clicks must open the complete spaced filename from either wrapped row. */ +/** Real xterm clicks resolve the complete filename or report its lookup error from either wrapped row. */ import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import { afterAll, beforeAll, describe, expect, it } from 'vitest'; @@ -22,12 +22,14 @@ describe('wrapped configuration file links (Chromium)', () => { { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200 }, { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400 }, { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400 }, + { wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400 }, ])( - 'opens the existing .$extension file from both rows: $wrap, original exists: $originalExists', + 'handles .$extension links on both rows: $wrap, original exists: $originalExists', async ({ wrap, originalExists, extension, registrationStatus }) => { const path = basePath.replace('.conf', `.${extension}`); const original = path.replace('Kobo eReader', 'KoboeReader'); const expected = wrap === 'hard' && originalExists ? original : path; + const lookupFails = registrationStatus === 400 && !originalExists; const lookups: string[] = []; const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); try { @@ -106,7 +108,16 @@ describe('wrapped configuration file links (Chromium)', () => { await page.waitForFunction(() => (window as any).app._linkHovered === true); await page.mouse.click(point.x, point.y); await page.waitForFunction((count) => (window as any).opened.length === count, row + 1); - expect(await page.evaluate(() => (window as any).opened)).toEqual(Array(row + 1).fill(expected)); + const opened = await page.evaluate(() => (window as any).opened as string[]); + if (lookupFails) { + for (const message of opened) { + expect(message).toMatch(/^error:/); + expect(message).toContain(path); + expect(message).not.toContain('outside the session workspace'); + } + } else { + expect(opened).toEqual(Array(row + 1).fill(expected)); + } await page.mouse.move(0, 0); } expect(lookups).toEqual( From 3847e377d8a4e2f331b363827470852a99b199b9 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Thu, 29 Oct 2026 01:32:17 +0800 Subject: [PATCH 5/5] fix(links): preserve missing-path and labelled-wrap behaviour --- .changeset/Repair-FilePathLinks.md | 2 + docs/architecture-invariants.md | 2 +- src/web/public/constants.js | 2 +- src/web/public/terminal-ui.js | 1 + test/file-path-link-forms.test.ts | 50 ++++++++++++++++++++----- test/wrapped-file-paths.browser.test.ts | 26 +++++++------ 6 files changed, 59 insertions(+), 24 deletions(-) diff --git a/.changeset/Repair-FilePathLinks.md b/.changeset/Repair-FilePathLinks.md index 3418dccc9..70c3e620a 100644 --- a/.changeset/Repair-FilePathLinks.md +++ b/.changeset/Repair-FilePathLinks.md @@ -5,3 +5,5 @@ Keep parenthesised filenames with spaces clickable as one complete file path, including `.conf` configuration files. For an ambiguous hard wrap, check the original concatenated filename through the existing guarded attachment route when clicked, and try the spaced filename only after a not-found response. Every original response other than not-found retains the normal opener, including workspace files whose extensions the attachment registry rejects. A spaced alternative opens only after successful registration. Preserve clickable row positions and share the spaced-path detector between the terminal and Response Viewer. Restrict comma-separated-list detection to parenthesised candidates so existing own-line, list-marker and labelled paths with commas remain whole. A rejected workspace path is named in the attachment-lookup error without describing it as outside the workspace. + +Keep the normal opener for the original filename when both hard-wrap candidates are missing, and exclude path labels and list markers inside parentheses from the lookup key. diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index cf823d056..1fdafa96d 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -415,7 +415,7 @@ Spaced paths are considered first by `findFilePathLinks()`: accepted candidates Parentheses also bound an inline spaced path, such as `Updated (/mnt/c/My Docs/report.conf)`. The whole candidate is reserved before fragment detectors run; existing root, prose and traversal guards still apply. Inside parentheses only, a comma at the end of an earlier word marks a list, not one filename. Own-line, list-marker and labelled paths retain commas in filenames and directory names. -Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. +Hard-wrapped filenames keep the existing concatenation without inserted spaces. A parenthesised final filename broken between words also carries a spaced alternative without changing its cell mapping. On activation only, the terminal checks the original path through the existing guarded attachment registration route (`notify: false`), tries the alternative only after HTTP 404, and retains the clicked session throughout. If both exist, the original wins. Every original result other than HTTP 404 goes to the normal opener, preserving workspace previews and log viewing even for extensions the attachment registry rejects; for an external path that opener retains the same attachment guards and errors. After an original 404, a spaced alternative opens only if it registers successfully; if it also returns 404, the original goes to the normal opener. Path labels, list markers and leading indentation inside parentheses are excluded from the alternative’s lookup key. Hover performs no lookup. Text alone cannot distinguish the two filenames, so this uses the filesystem result rather than a spacing guess. 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. When other columns follow a wrapped URL on its first row (a codex table's path or date column), the URL is stitched without them, and they still link on that row: `terminalLinkLine()` scans them after the stitched rows as a segment of their own, mapped back to their cells. A URL in such a column is the row's last URL, and only the last URL on a row is continued, so that row is never stitched. 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. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 8a6459411..551fb234f 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2637,7 +2637,7 @@ function terminalPathLine(buffer, row, cols, maxRows) { const after = next.text.slice(0, closing).trimStart(); const spaced = spacedFilePathOnLine(`${before} ${after}`, { parenthesised: true }); if (!before.includes(')') && !/[/-]$/.test(before) && !after.includes('/') && spaced?.link) { - alternatives.push({ path: before + after, spacedPath: spaced.path }); + alternatives.push({ path: before.slice(spaced.index) + after, spacedPath: spaced.path }); } } } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index d52e6bba2..993c6b676 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -2347,6 +2347,7 @@ Object.assign(CodemanApp.prototype, { if (original.status !== 404) return activateFilePath(target, false, sessionId); const path = self._resolveLinkedFilePath(alternative.spacedPath, sessionId); const registered = await self._registerExternalPreview(path, sessionId); + if (registered.status === 404) return activateFilePath(target, false, sessionId); if (!registered.attachmentId) { self.showToast(registered.error || 'Cannot open this file', 'error'); return; diff --git a/test/file-path-link-forms.test.ts b/test/file-path-link-forms.test.ts index b80615d56..ea77a3d6b 100644 --- a/test/file-path-link-forms.test.ts +++ b/test/file-path-link-forms.test.ts @@ -343,24 +343,54 @@ describe('terminal link provider', () => { expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); }); - it.each([400, 403, 404, 500, 503])( - 'does not open an unregistered spaced alternative after HTTP %s', - async (status) => { - const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); - app._registerExternalPreview - .mockResolvedValueOnce({ status: 404, error: 'File not found' }) - .mockResolvedValueOnce({ status, error: 'Alternative unavailable' }); + it('uses the original opener when both hard-wrap filenames are missing', async () => { + for (const row of [1, 2]) { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], row, 120); + app._registerExternalPreview.mockResolvedValue({ status: 404, error: 'Attachment file not found' }); await links[0].activate({}, links[0].text); expect(app._registerExternalPreview.mock.calls).toEqual([ ['/mnt/data/KoboeReader.conf', 's1'], ['/mnt/data/Kobo eReader.conf', 's1'], ]); - expect(app.openFilePreview).not.toHaveBeenCalled(); - expect(app.openLogViewerWindow).not.toHaveBeenCalled(); - expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error'); + expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1'); + expect(app.showToast).not.toHaveBeenCalled(); + } + }); + + it.each(['Path: ', '- ', '- Path: ', ' Path: '])( + 'resolves the filename without its parenthesised prefix %j', + async (prefix) => { + for (const row of [1, 2]) { + const { app, links } = linksOn([`Saved the notes (${prefix}/home/me/proj/out/My`, ' Notes.md).'], row, 120); + app._registerExternalPreview + .mockResolvedValueOnce({ status: 404, error: 'Attachment file not found' }) + .mockResolvedValueOnce({ attachmentId: 'att-spaced' }); + expect(links.map((link) => link.text)).toEqual(['/home/me/proj/out/MyNotes.md']); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview.mock.calls).toEqual([ + ['/home/me/proj/out/MyNotes.md', 's1'], + ['/home/me/proj/out/My Notes.md', 's1'], + ]); + expect(app.openFilePreview).toHaveBeenCalledWith('/home/me/proj/out/My Notes.md', 's1'); + } } ); + it.each([400, 403, 500, 503])('does not open an unregistered spaced alternative after HTTP %s', async (status) => { + const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); + app._registerExternalPreview + .mockResolvedValueOnce({ status: 404, error: 'File not found' }) + .mockResolvedValueOnce({ status, error: 'Alternative unavailable' }); + await links[0].activate({}, links[0].text); + expect(app._registerExternalPreview.mock.calls).toEqual([ + ['/mnt/data/KoboeReader.conf', 's1'], + ['/mnt/data/Kobo eReader.conf', 's1'], + ]); + expect(app.openFilePreview).not.toHaveBeenCalled(); + expect(app.openLogViewerWindow).not.toHaveBeenCalled(); + expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error'); + }); + it('keeps the session selected at click time while a path lookup is pending', async () => { const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120); const pending = links[0].activate({}, links[0].text); diff --git a/test/wrapped-file-paths.browser.test.ts b/test/wrapped-file-paths.browser.test.ts index 14dddf4f8..10b5d4c71 100644 --- a/test/wrapped-file-paths.browser.test.ts +++ b/test/wrapped-file-paths.browser.test.ts @@ -17,18 +17,20 @@ describe('wrapped configuration file links (Chromium)', () => { }); it.each([ - { wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200 }, - { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400 }, - { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400 }, - { wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400 }, + { wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 404, prefix: '' }, + { wrap: 'hard', originalExists: false, extension: 'markdown', registrationStatus: 200, prefix: 'Path: ' }, ])( - 'handles .$extension links on both rows: $wrap, original exists: $originalExists', - async ({ wrap, originalExists, extension, registrationStatus }) => { + 'handles .$extension links on both rows: $wrap, original exists: $originalExists, prefix: $prefix', + async ({ wrap, originalExists, extension, registrationStatus, prefix }) => { const path = basePath.replace('.conf', `.${extension}`); const original = path.replace('Kobo eReader', 'KoboeReader'); - const expected = wrap === 'hard' && originalExists ? original : path; + const expected = wrap === 'hard' && (originalExists || registrationStatus === 404) ? original : path; const lookupFails = registrationStatus === 400 && !originalExists; const lookups: string[] = []; const page = await browser.newPage({ viewport: { width: 1920, height: 600 } }); @@ -61,10 +63,10 @@ describe('wrapped configuration file links (Chromium)', () => { await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') }); } await page.evaluate( - async ({ path, wrap, workingDir }) => { + async ({ path, wrap, workingDir, prefix }) => { const w = window as any; const boundary = path.lastIndexOf(' '); - const first = `Found it: prepared a corrected configuration (${path.slice(0, boundary)}`; + const first = `Found it: prepared a corrected configuration (${prefix}${path.slice(0, boundary)}`; const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 }); terminal.open(document.getElementById('terminal')); w.opened = []; @@ -82,7 +84,7 @@ describe('wrapped configuration file links (Chromium)', () => { await new Promise((done) => terminal.write(text, done)); await new Promise((done) => requestAnimationFrame(() => requestAnimationFrame(() => done()))); }, - { path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace' } + { path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace', prefix } ); expect(lookups).toEqual([]);