diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index f67c06bc..471e41de 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -390,7 +390,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli **One pattern, two consumers.** `FILE_PATH_LINK_PATTERN` / `absoluteFilePathPattern()` live in `constants.js`; the xterm link provider (`registerFilePathLinkProvider`, terminal-ui.js) and the response viewer's `_linkifyFilePaths()` (app.js) both reach it through `findFilePathLinks()` (below), which builds a fresh instance per call. ⚠️ Fresh per call, never one shared object: `lastIndex` is per-object state on a `/g` regex. The pattern is anchored on a known absolute root and terminated by a known extension, so a fraction (`3/4`) or a date can't match and trailing punctuation stays out. Roots include `Users` and `mnt`, without which nothing was clickable on macOS or WSL. The linear-time guard and the "terminal-ui builds from the factory" structural check are in `test/link-provider-regex.test.ts`. -**The chat linkifier walks text nodes.** `_linkifyFilePaths()` builds anchors with `createElement`/`textContent` on the rendered subtree, never by rebuilding sanitized markup as a string — the source is model output. Subtrees already inside an `` are skipped (marked autolinks URLs; a nested anchor would swallow the click), and the anchor's text is the path verbatim so "copy code" still yields what the agent printed. `test/response-viewer-file-links.test.ts` pins both properties. +**The chat linkifier walks text nodes.** `_linkifyFilePaths()` builds anchors with `createElement`/`textContent` on the rendered subtree, never by rebuilding sanitized markup as a string — the source is model output. Subtrees already inside an `` are skipped (marked autolinks URLs; a nested anchor would swallow the click), and the anchor's text is the path verbatim so "copy code" still yields what the agent printed. Before the walk it also rewrites existing anchors whose href is a scheme-less, non-`//` path (Codex cites files as `[label](/abs/path.json)`, which otherwise opens as a same-origin URL) into the same `a.rv-path` shape, plus `rv-path-link` so the label keeps the prose font, on purpose without `FILE_PATH_LINK_PATTERN`'s root and extension filter: the href is a link target the agent chose. `test/response-viewer-file-links.test.ts` pins both properties and the anchor rewrite. **Out-of-workspace paths go through the attachment routes, not the file routes.** `file-content`/`file-raw` resolve against `workingDir` and 404 anything that escapes it, which is correct and unchanged — but the paths agents most often print (a `/tmp` capture, Claude's own scratchpad, another checkout) are exactly that, so clicking one used to report "File not found" for a file sitting on disk. `openFilePreview()` now detects the case (`_isExternalPreviewPath`, a string compare for ROUTING only; the real decision stays server-side) and registers the path via `POST /api/sessions/:id/attachments` first, rendering by id. ⚠️ That registration passes `notify: false`, which suppresses ONLY the `attachment:detected` broadcast — the guard, the registry entry and the by-id routes are identical either way. Without it every click also popped an attachment card announcing the file already filling the screen. ⚠️ The click is an explicit user action on the **explicit, Origin-guarded** registration route, which is why it may cross the workspace boundary at all; the passive magic-link scanner stays force-confined. A type outside `SUPPORTED_ATTACHMENT_EXTENSIONS` (`.svg`, `.bmp`) is refused with a message naming what IS previewable, rather than the registry's own policy term. diff --git a/docs/wiki/Mobile-Guide.md b/docs/wiki/Mobile-Guide.md index 11a78292..b21b896e 100644 --- a/docs/wiki/Mobile-Guide.md +++ b/docs/wiki/Mobile-Guide.md @@ -131,11 +131,12 @@ already writing in the editor. ## Tapping, links and copying -- **Tap a link** in terminal output and it opens in a new tab. Same for a link in an agent's +- **Tap a link** in terminal output and it opens in a new tab. Same for a web link in an agent's answer in the response viewer — it opens a tab rather than navigating the dashboard away, which on a phone would unload the whole session view. -- **Tap a file path** an agent printed and the file-preview overlay opens (Markdown rendered, - HTML as a page); a log or other plain-text path opens the log viewer. Works in scrolled-up +- **Tap a file path** an agent printed, or a link to one in its answer (Codex writes + `[report](/tmp/report.json)`), and the file-preview overlay opens (Markdown rendered, HTML + as a page); a log or other plain-text path opens the log viewer. Works in scrolled-up transcript too. - A tap on the prose *beside* a link still places the cursor as usual, and a tap on a dialog's numbered choice still answers the dialog even when the row contains a path — the dialog wins, diff --git a/docs/wiki/Working-With-Files.md b/docs/wiki/Working-With-Files.md index 4135e9d9..0be064d7 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -89,7 +89,8 @@ File paths in a session are links. That works in two places: - **In the terminal**, on any absolute path an agent prints. - **In the response viewer**, where paths are usually written as prose or in backticks. They - render as underlined monospace links. + render as underlined monospace links. A Markdown link whose target is a path + (`[report](/tmp/run/report.json)`, the way Codex cites files) opens the same way. Clicking one opens it in the preview: images and PDFs render, video and audio play with a working scrub bar, documents convert, text shows inline, Markdown renders and HTML opens as a diff --git a/src/web/public/app.js b/src/web/public/app.js index 78ae8e8b..88e148ae 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2967,6 +2967,12 @@ class CodemanApp { * output. Subtrees already inside an `` are skipped so an autolinked URL * is never re-cut, and the anchor's textContent is the path verbatim, so * "copy code" still yields exactly what the agent printed. + * + * Before that walk it also rewrites an existing anchor whose href is a path + * (no scheme, not `//host`, not a bare `#fragment`) into the same `a.rv-path` + * shape, on purpose WITHOUT `FILE_PATH_LINK_PATTERN`'s root and extension + * filter: the agent chose that href as a link target, and left alone it opens + * as a same-origin URL (a 404 page in a new tab). */ _linkifyFilePaths(root) { if (!root || typeof document === 'undefined') return; @@ -2974,6 +2980,36 @@ class CodemanApp { // out of the middle of rendering a message. if (typeof findFilePathLinks !== 'function') return; + // A markdown link whose destination is a path (`[report](/home/me/run/x.json)`, + // Codex's usual way of citing a file) resolved against Codeman's own origin + // and opened a 404 in a new tab. Give it the linkified-path shape instead; + // links with a scheme or `//host` keep their own behaviour. + for (const a of root.querySelectorAll('a[href]')) { + // The URL parser drops tabs and newlines, so `http ://` is still http. + const href = (a.getAttribute('href') || '').replace(/[\t\n\r]/g, ''); + if (/^[a-z][a-z0-9+.-]*:/i.test(href) || href.startsWith('//')) continue; + // `#L42`, `?plain=1` and a `:42:7` suffix name a spot in the file, not the + // file. The suffix is cut before decoding, so an encoded `%3A` stays a name. + let path = href.split('#')[0].split('?')[0].replace(/:\d+(?::\d+)?$/, ''); + try { + path = decodeURIComponent(path); + } catch { + /* malformed escape: keep the path as written */ + } + // A bare `#fragment` (and every `.rv-path`, whose href is `#`) ends up empty. + if (!path) continue; + // rv-path-link keeps the label in the prose font (styles.css): it is a label, + // not a path, and break-all would split it mid-word on a phone. + a.className = 'rv-path rv-path-link'; + a.dataset.path = path; + a.dataset.linkified = '1'; + if (path.endsWith('/') || findFilePathLinks(path).some((f) => f.folder && f.path === path)) a.dataset.folder = '1'; + a.title = path; + a.setAttribute('href', '#'); + a.removeAttribute('target'); + a.removeAttribute('rel'); + } + // Collect first: replacing a node while the walker is positioned on it // invalidates the traversal. const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT); diff --git a/src/web/public/styles.css b/src/web/public/styles.css index aca188de..412c4399 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -14961,6 +14961,14 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat cursor: pointer; } +/* A markdown link whose target is a path (`[Our incident record](/tmp/x.json)`) + takes the rv-path shape for the click delegate, but its text is a label, not a + path: keep it in the prose font and never break it mid-word. */ +.rv-text a.rv-path.rv-path-link { + font: inherit; + word-break: normal; +} + /* Tables — scroll wrapper keeps table proper while allowing horizontal overflow */ .rv-table-wrap { margin: 1em 0; diff --git a/test/response-viewer-file-links.test.ts b/test/response-viewer-file-links.test.ts index a771ad7c..85947186 100644 --- a/test/response-viewer-file-links.test.ts +++ b/test/response-viewer-file-links.test.ts @@ -110,6 +110,84 @@ describe('response viewer file-path linkifier', () => { expect(root.querySelector('a')!.getAttribute('href')).toBe('https://example.com/tmp/shot.png'); }); + it('turns a markdown link to a path into a preview link, not a same-origin navigation', () => { + // Codex cites files as `[label](/abs/path)`: left alone, the href resolved + // against Codeman's origin and opened a 404 JSON page in a new tab. + const path = '/home/me/llm-bench/runs/crash-20261017/incident-summary.json'; + const root = linkify( + `

Our incident record and ` + + `a b, docs, renders, ` + + `release, raw, tmp

` + ); + + const links = paths(root); + expect(links.map((a) => a.getAttribute('data-path'))).toEqual([ + path, + 'src/a b.ts', + 'docs/', + '/tmp/renders', + // An encoded colon is part of the name, never a line suffix. + '/tmp/release:12', + // A query names a view of the file, not the file. + '/tmp/a.json', + '/tmp/', + ]); + for (const a of links) { + expect(a.getAttribute('href')).toBe('#'); + expect(a.hasAttribute('target')).toBe(false); + expect(a.hasAttribute('rel')).toBe(false); + expect(a.getAttribute('data-linkified')).toBe('1'); + } + expect(links[0].textContent).toBe('Our incident record'); + // A label, not a path: the second class keeps it in the prose font (styles.css). + expect(links.every((a) => a.classList.contains('rv-path-link'))).toBe(true); + // A trailing slash and a rooted folder path without one both open the folder grid + // (`:` ends a folder match in findFilePathLinks, so `release:12` previews instead). + // findFilePathLinks finds no folder in a bare `/tmp/`, so only the trailing-slash + // check marks that one. + expect(links.map((a) => a.hasAttribute('data-folder'))).toEqual([false, false, true, true, false, false, true]); + }); + + it('opens a clicked markdown path link in the file preview through the click delegate', () => { + const app = Object.create(CodemanApp.prototype) as { + activeSessionId: string; + openFilePreview: ReturnType; + openFolderGrid: ReturnType; + _linkifyFilePaths(root: unknown): void; + _bindResponseViewerInteractions(body: unknown): void; + }; + app.activeSessionId = 'sess-1'; + app.openFilePreview = vi.fn(); + app.openFolderGrid = vi.fn(); + const body = document.createElement('div'); + body.innerHTML = '

Our incident record

'; + app._linkifyFilePaths(body); + app._bindResponseViewerInteractions(body); + + const click = new dom.window.MouseEvent('click', { bubbles: true, cancelable: true }); + body.querySelector('a')!.dispatchEvent(click); + + expect(click.defaultPrevented).toBe(true); + expect(app.openFilePreview).toHaveBeenCalledWith('/home/me/run/summary.json', 'sess-1'); + expect(app.openFolderGrid).not.toHaveBeenCalled(); + }); + + it('keeps web, mail, protocol-relative and fragment links as they are', () => { + const hrefs = ['https://example.com/a.json', 'mailto:me@example.com', '//cdn.example.com/x.js', '#section']; + const root = linkify(`

${hrefs.map((h) => `x`).join(' ')}

`); + + expect(paths(root)).toHaveLength(0); + expect(Array.from(root.querySelectorAll('a')).map((a) => a.getAttribute('href'))).toEqual(hrefs); + }); + + it('reads a scheme split by a tab as the URL the browser would open', () => { + // The URL parser drops tabs, so this is http://localhost:5173/a, never a relative file. + const root = linkify('

local

'); + + expect(paths(root)).toHaveLength(0); + expect(root.querySelector('a')!.getAttribute('href')).toBe('http\t://localhost:5173/a'); + }); + it('leaves text with no path untouched', () => { const root = linkify('

Ratio 3/4 on 2026/08/16, see the app

');