diff --git a/src/web/public/constants.js b/src/web/public/constants.js index 4cb2f099..3308dfd2 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2178,27 +2178,93 @@ function findFilePathLinks(text) { while ((match = urlPattern.exec(value)) !== null) urls.push({ index: match.index, path: match[0] }); const inUrl = (index, length) => urls.some((u) => index < u.index + u.path.length && u.index < index + length); + // A path with spaces is taken whole, or the span it covers links nothing + // (spacedFilePathOnLine). Either way no detector below may cut a fragment out of it. + const reserved = []; + let lineStart = 0; + for (const line of value.split('\n')) { + const spaced = spacedFilePathOnLine(line); + if (spaced && !inUrl(lineStart + spaced.index, spaced.path.length)) { + const index = lineStart + spaced.index; + reserved.push({ index, path: spaced.path }); + if (spaced.link) found.push({ path: spaced.path, index }); + } + lineStart += line.length + 1; + } + const isReserved = (index, length) => + reserved.some((r) => index < r.index + r.path.length && r.index < index + length); + const absolute = absoluteFilePathPattern(); while ((match = absolute.exec(value)) !== null) { - if (!inUrl(match.index, match[1].length)) found.push({ path: match[1], index: match.index }); + if (!inUrl(match.index, match[1].length) && !isReserved(match.index, match[1].length)) { + found.push({ path: match[1], index: match.index }); + } } const relative = new RegExp(RELATIVE_FILE_PATH_LINK_PATTERN.source, 'g'); while ((match = relative.exec(value)) !== null) { const path = match[2]; const index = match.index + match[1].length; - if (overlaps(index, path.length) || inUrl(index, path.length)) continue; + if (overlaps(index, path.length) || inUrl(index, path.length) || isReserved(index, path.length)) continue; found.push({ path, index }); } const folders = new RegExp(FOLDER_PATH_LINK_PATTERN.source, 'g'); while ((match = folders.exec(value)) !== null) { const path = folderLinkPath(match[2]); const index = match.index + match[1].length; - if (!path || overlaps(index, path.length) || inUrl(index, path.length)) continue; + if (!path || overlaps(index, path.length) || inUrl(index, path.length) || isReserved(index, path.length)) continue; found.push({ path, index, folder: true }); } return found.sort((a, b) => a.index - b.index); } +/** + * A file path WITH SPACES on one line, or null. + * + * Every path pattern ends a path at whitespace, so `/mnt/c/OneDrive - Contoso/a.docx` + * linked `/mnt/c/OneDrive` plus a relative `Contoso/a.docx`, two links to the + * wrong targets. A space is only safe inside a path where the line itself bounds + * the path, so a spaced path is recognised in exactly these shapes: + * + * /mnt/c/My Docs/report.docx the whole line is the path + * - /mnt/c/My Docs/report.docx after a list marker (`-`, `*`, `•`, `1.`, `2)`) + * Saved to: 03 Attachments/plan.pptx after a short label (one to four words + `:`) + * + * An absolute path may stand alone; a relative or `~/` one needs a marker or a + * label, since a bare spaced relative candidate is ordinary prose. One trailing + * sentence mark is left out. The candidate must read as ONE path: it ends on a + * known extension, and no earlier word ends on one, starts a new `/` path, ends + * in `:` or is an arrow, so two paths on a line are never merged. + * + * A candidate that is one path but not a linkable one (an unknown root such as + * `/etc`, a `.`/`..`/empty segment) comes back with `link: false`: its span links + * nothing, rather than the fragment after its last space. + * + * Linear: one bounded prefix match, then string scans. + * + * @param {string} line One line, no newline. + * @returns {{path: string, index: number, link: boolean} | null} + */ +function spacedFilePathOnLine(line) { + const text = String(line || ''); + const prefix = /^[ \t]*((?:\d{1,3}[.)]|[-*•])[ \t]+)?((?:[A-Za-z][\w-]{0,30}(?: [\w-]{1,30}){0,3}):[ \t]+)?/.exec(text); + const index = prefix[0].length; + const path = text.slice(index).replace(/\s+$/, '').replace(/[.,;:!?]$/, ''); + if (!path.includes(' ') || !path.includes('/') || path.length > 1000) return null; + if (!path.startsWith('/') && !prefix[1] && !prefix[2]) return null; + if (!PATH_TOKEN_ENDS_WITH_EXTENSION.test(path)) return null; + if (/[\t"'`<>|;&\x00-\x1f\\]/.test(path) || path.includes('://')) return null; + const words = path.split(' '); + for (const word of words.slice(0, -1)) { + if (word.endsWith(':') || /^(?:—|–|→|=>|->|<-)$/.test(word)) return null; + if (PATH_TOKEN_ENDS_WITH_EXTENSION.test(word.replace(/[.,;:!?)\]}]+$/, ''))) return null; + } + if (words.slice(1).some((word) => word.startsWith('/'))) return null; + const rooted = SPACED_PATH_ROOT.test(path) || path.startsWith('~/'); + const segments = path.replace(/^~?\//, '').split('/'); + const link = (rooted || !path.startsWith('/')) && segments.every((seg) => seg && seg !== '.' && seg !== '..'); + return { path, index, link }; +} + /** * Folder paths, for the folder grid: rooted like {@link FILE_PATH_LINK_PATTERN} * (`/mnt/raid1/neon_getaway/styleframes`), `~/renders`, or relative with a @@ -2385,6 +2451,8 @@ const FILE_LINK_ROOT_GROUP = /^\(\\\/\(\?:([^()]*)\)/.exec(FILE_PATH_LINK_PATTER const PATH_TOKEN_ENDS_WITH_EXTENSION = new RegExp(`\\.${FILE_LINK_EXTENSION_GROUP}$`); const PATH_TOKEN_HAS_EXTENSION = new RegExp(`\\.${FILE_LINK_EXTENSION_GROUP}\\b`); const PATH_TOKEN_STARTS_NEW_PATH = new RegExp(`^(?:~\\/|\\/(?:${FILE_LINK_ROOT_GROUP})\\/)`); +/** A rooted path's start (`/home/`, `/mnt/`, …), for {@link spacedFilePathOnLine}. */ +const SPACED_PATH_ROOT = new RegExp(`^\\/(?:${FILE_LINK_ROOT_GROUP})\\/`); /** A relative head that names its own location (`./a`, `../a`) is an entry of its own. */ const PATH_TOKEN_STARTS_EXPLICIT_RELATIVE = /^\.{1,2}\//; /** First directory of a path, a leading `/` or `./` ignored: `src` for `src/a.ts` and `/src/a.ts`. */ diff --git a/test/spaced-file-path-links.test.ts b/test/spaced-file-path-links.test.ts new file mode 100644 index 00000000..b799a888 --- /dev/null +++ b/test/spaced-file-path-links.test.ts @@ -0,0 +1,165 @@ +// Port: none (a pure function in a vm context — no browser, no server). +// +// File paths with spaces in them (`OneDrive - Contoso/…`, `90 Sensitive/…`) were +// cut at the first space by every path pattern, so the terminal and the response +// viewer linked a fragment such as `/mnt/c/proj/My` plus a bogus relative tail +// `Docs/report.docx`, each opening the wrong target. findFilePathLinks() now takes +// a spaced path WHOLE where the line makes its extent unambiguous: the path is the +// whole line, or follows a list marker and/or a short `Label:`. A spaced line that +// is one path but not a linkable one (unknown root, `..`) links nothing at all, +// rather than a fragment. +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it } from 'vitest'; + +const SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); + +function load(): (text: string) => Array<{ path: string; index: number; folder?: true }> { + const context = vm.createContext({ console, window: undefined }); + vm.runInContext(`${SOURCE}\nglobalThis.__fn = findFilePathLinks;`, context, { filename: 'constants.js' }); + return (context as { __fn: never })['__fn'] as never; +} + +const findFilePathLinks = load(); +const paths = (text: string) => findFilePathLinks(text).map((f) => f.path); + +const LONG = + '/mnt/c/Users/someone/OneDrive - Contoso/01_Projects/Other/kb/out/deliverables/fy27-messaging-market-story.docx'; + +describe('spaced file paths', () => { + it('links an own-line absolute path with spaces whole', () => { + expect(paths(LONG)).toEqual([LONG]); + expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); + }); + + it('reports where the whole path starts', () => { + const line = ' - Path: /mnt/c/proj/My Big Docs/report.docx'; + expect(findFilePathLinks(line)).toEqual([ + { path: '/mnt/c/proj/My Big Docs/report.docx', index: line.indexOf('/mnt') }, + ]); + }); + + it('leaves a single trailing sentence period out', () => { + expect(paths('/mnt/c/proj/My Big Docs/report.docx.')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']); + }); + + it.each(['Path:', 'File:', 'Saved to:'])('links an absolute spaced path after %s', (label) => { + const p = '/mnt/sync/brain/90 Sensitive/team/training/attendees.xlsx'; + expect(paths(`${label} ${p}`)).toEqual([p]); + }); + + it.each(['Path:', 'File:', 'Saved to:'])('links a relative spaced path after %s', (label) => { + const p = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + expect(paths(`${label} ${p}`)).toEqual([p]); + }); + + it.each(['- ', '* ', '• ', '1. ', '2) '])('links a spaced path after the list marker %j', (marker) => { + const rel = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + expect(paths(`${marker}${rel}`)).toEqual([rel]); + expect(paths(` ${marker}${LONG}`)).toEqual([LONG]); + }); + + it('works line by line inside multi-line text (the response viewer hands over text nodes)', () => { + const text = `Wrote two files:\n- ${LONG}\n- notes/a b/c.md\n`; + expect(paths(text)).toEqual([LONG, 'notes/a b/c.md']); + }); + + it('links nothing for a spaced path outside the known roots (no partial tail)', () => { + expect(paths('/etc/secret files/passwords.txt')).toEqual([]); + expect(paths('/srvx/team share/notes/passwords.txt')).toEqual([]); + }); + + it('links nothing for a spaced path with a `..` or `.` segment', () => { + expect(paths('/mnt/c/proj/../other place/docs/report.docx')).toEqual([]); + expect(paths('File: 03 Attachments/../private/report.xlsx')).toEqual([]); + expect(paths('- 03 Attachments/./private/report.xlsx')).toEqual([]); + }); + + it('never merges two space-separated paths into one link', () => { + expect(paths('/mnt/c/proj/a.md /mnt/c/proj/b.pdf')).toEqual(['/mnt/c/proj/a.md', '/mnt/c/proj/b.pdf']); + expect(paths('- docs/a.md docs/b.md')).toEqual(['docs/a.md', 'docs/b.md']); + expect(paths('/mnt/c/proj/a b.md -> /mnt/c/proj/c.md')).not.toContain('/mnt/c/proj/a b.md -> /mnt/c/proj/c.md'); + }); + + it('does not take prose into a path', () => { + // No marker or label: a relative spaced candidate is prose, not a path. + expect(paths('See docs/report.docx for details')).toEqual(['docs/report.docx']); + expect(paths('see the docs/a b.md')).not.toContain('see the docs/a b.md'); + // A long description before a colon is a sentence, not a label: only the path links. + const rel = '.worktrees/feature-x/docs/specs/2026-09-05-design.md'; + expect(paths(`• Design committed in the isolated worktree: ${rel}`)).toEqual([rel]); + }); + + it('leaves unspaced paths to the ordinary detectors', () => { + expect(paths('- /mnt/c/proj/docs/report.docx')).toEqual(['/mnt/c/proj/docs/report.docx']); + expect(paths('• committed as /tmp/x/docs/specs/design.md:1 (e7ae1b4).')).toEqual(['/tmp/x/docs/specs/design.md']); + }); + + it('stays linear on a long spaced line', () => { + const line = '- ' + 'a '.repeat(20000) + '/x.md'; + const t0 = performance.now(); + findFilePathLinks(line); + expect(performance.now() - t0).toBeLessThan(500); + }); +}); + +describe('terminal link provider with spaced paths', () => { + type Link = { text: string; activate: (event: unknown, text: string) => void }; + const TERMINAL_UI = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + + function linksFor(line: string) { + const noop = () => {}; + const context = vm.createContext({ + console: { ...console, debug: noop, log: noop }, + CodemanApp: class CodemanApp {}, + document: { addEventListener: noop, getElementById: noop, documentElement: { dataset: {} } }, + window: { addEventListener: noop, removeEventListener: noop, document: { documentElement: { dataset: {} } } }, + }); + vm.runInContext(`${SOURCE}\n${TERMINAL_UI}\nglobalThis.__proto = CodemanApp.prototype;`, context); + const proto = (context as unknown as { __proto: Record unknown> }).__proto; + let provider: { provideLinks: (row: number, cb: (links?: Link[]) => void) => void } | undefined; + const opened: string[] = []; + const app = { + terminal: { + cols: 200, + registerLinkProvider: (p: typeof provider) => (provider = p), + buffer: { + active: { + length: 1, + getLine: (r: number) => + r === 0 + ? { + isWrapped: false, + translateToString: (trim?: boolean) => (trim === false ? line.padEnd(200) : line), + } + : undefined, + }, + }, + }, + activeSessionId: 's1', + sessions: new Map([['s1', { workingDir: '/workspace/project' }]]), + openFilePreview: (p: string) => opened.push(p), + openLogViewerWindow: (p: string) => opened.push(`log:${p}`), + _isExternalPreviewPath: () => false, + _resolveLinkedFilePath: proto._resolveLinkedFilePath, + registerFilePathLinkProvider: proto.registerFilePathLinkProvider, + }; + app.registerFilePathLinkProvider.call(app); + let links: Link[] = []; + provider!.provideLinks(1, (value) => (links = value ?? [])); + return { links, opened }; + } + + it('links the whole spaced path and opens it, resolved against the workspace', () => { + const rel = '03 Attachments/Build SIPOC Design/Build-SIPOC.pptx'; + const { links, opened } = linksFor(`Saved to: ${rel}`); + expect(links.map((l) => l.text)).toEqual([rel]); + links[0].activate(null, links[0].text); + expect(opened).toEqual([`/workspace/project/${rel}`]); + }); + + it('links nothing for an unlinkable spaced path', () => { + expect(linksFor('/etc/secret files/passwords.txt').links).toEqual([]); + }); +});