From 6f785e116f422e791b4fe1f255a9c066fdb59b66 Mon Sep 17 00:00:00 2001 From: JD Date: Sun, 18 Oct 2026 13:36:10 -0400 Subject: [PATCH 1/2] fix(response-viewer): open markdown links to file paths in the preview, not as a same-origin URL Codex cites files as [label](/abs/path). The chat view gave those links target=_blank, so a click opened /abs/path and landed on a Fastify 404. Path-shaped hrefs now take the linkified-path shape the file preview already opens; scheme, protocol-relative and fragment links are unchanged. --- docs/wiki/Mobile-Guide.md | 5 +- docs/wiki/Working-With-Files.md | 3 +- src/web/public/app.js | 28 ++++++++++ test/response-viewer-file-links.test.ts | 71 +++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 3 deletions(-) diff --git a/docs/wiki/Mobile-Guide.md b/docs/wiki/Mobile-Guide.md index 11a78292..820710ad 100644 --- a/docs/wiki/Mobile-Guide.md +++ b/docs/wiki/Mobile-Guide.md @@ -134,8 +134,9 @@ already writing in the editor. - **Tap a link** in terminal output and it opens in a new tab. Same for a 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 ef7c5697..df94e8d6 100644 --- a/docs/wiki/Working-With-Files.md +++ b/docs/wiki/Working-With-Files.md @@ -84,7 +84,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 62716497..6d53cb4d 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2966,6 +2966,34 @@ 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; + a.className = 'rv-path'; + 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/test/response-viewer-file-links.test.ts b/test/response-viewer-file-links.test.ts index a771ad7c..c30d2fd3 100644 --- a/test/response-viewer-file-links.test.ts +++ b/test/response-viewer-file-links.test.ts @@ -110,6 +110,77 @@ 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

` + ); + + 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', + ]); + 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 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). + expect(links.map((a) => a.hasAttribute('data-folder'))).toEqual([false, false, true, true, false]); + }); + + 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

'); From fde23e3a076957da3599473f6c68ecd3ec94922d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Thu, 22 Oct 2026 19:20:01 +0200 Subject: [PATCH 2/2] fix(response-viewer): keep a path link's label in the prose font, pin its edges Merge-time fix for #665. - A markdown link converted to a preview link became `a.rv-path`, so its label ("Our incident record") took the path style: monospace, 0.92em and `word-break: break-all`, which split prose mid-word on a phone. The anchor now also carries `rv-path-link`, and `.rv-text a.rv-path.rv-path-link` resets font and word-break, as the File Viewer does for its labelled document links. - The first new test adds `/tmp/a.json?raw=true` (pins the `?query` strip) and `/tmp/` (pins the trailing-slash folder check, since findFilePathLinks finds no folder in a bare `/tmp/`), and checks the new class. - The `_linkifyFilePaths` JSDoc and docs/architecture-invariants.md say the function also rewrites anchors whose href is a scheme-less, non-`//` path, on purpose without FILE_PATH_LINK_PATTERN's root and extension filter. - Mobile Guide: "a web link" in the response viewer opens a tab. Co-Authored-By: Claude Opus 5.5 (1M context) --- docs/architecture-invariants.md | 2 +- docs/wiki/Mobile-Guide.md | 2 +- src/web/public/app.js | 10 +++++++++- src/web/public/styles.css | 8 ++++++++ test/response-viewer-file-links.test.ts | 11 +++++++++-- 5 files changed, 28 insertions(+), 5 deletions(-) diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index a50539c8..8c42278a 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -386,7 +386,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 820710ad..b21b896e 100644 --- a/docs/wiki/Mobile-Guide.md +++ b/docs/wiki/Mobile-Guide.md @@ -131,7 +131,7 @@ 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, or a link to one in its answer (Codex writes diff --git a/src/web/public/app.js b/src/web/public/app.js index 6d53cb4d..5ac677de 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2959,6 +2959,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; @@ -2984,7 +2990,9 @@ class CodemanApp { } // A bare `#fragment` (and every `.rv-path`, whose href is `#`) ends up empty. if (!path) continue; - a.className = 'rv-path'; + // 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'; diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 624364d9..8fcc9cb5 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -14926,6 +14926,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 c30d2fd3..85947186 100644 --- a/test/response-viewer-file-links.test.ts +++ b/test/response-viewer-file-links.test.ts @@ -117,7 +117,7 @@ describe('response viewer file-path linkifier', () => { const root = linkify( `

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

` + `release, raw, tmp

` ); const links = paths(root); @@ -128,6 +128,9 @@ describe('response viewer file-path linkifier', () => { '/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('#'); @@ -136,9 +139,13 @@ describe('response viewer file-path linkifier', () => { 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). - expect(links.map((a) => a.hasAttribute('data-folder'))).toEqual([false, false, true, true, false]); + // 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', () => {