From 4e2c1b99899ca167f578f1f11b3266db6f16259d Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 16 Aug 2026 17:03:26 +0200 Subject: [PATCH 1/4] fix(files): open file paths agents print, from the terminal and the chat A path an agent prints was already underlined in the terminal, but clicking one opened the preview overlay on "File not found": file-content/file-raw resolve against the session workingDir and refuse anything outside it, and the paths agents print most (a /tmp capture, Claude's own scratchpad, another checkout) are outside it by definition. In the response viewer those paths were not links at all. - openFilePreview() detects an out-of-workspace path and registers it through POST /api/sessions/:id/attachments first, rendering by attachment id. That is the surface built for live external files, so the server-side guard is unchanged: secret trees blocked, symlinks resolved, extension allowlist. The workspace routes keep refusing escapes exactly as before. - New optional `notify` field on that route. `notify: false` suppresses only the attachment:detected broadcast, so a click does not also pop a card announcing the file already filling the screen. Default stays true for the CLI and publish callers. - _linkifyFilePaths() links paths in rendered response-viewer markdown. It walks text nodes and builds anchors with DOM APIs (the source is model output; never a string rebuild of sanitized markup), skips subtrees already inside an , and keeps the message text byte-identical so copy-code is unaffected. - One path pattern in constants.js now feeds both the xterm link provider and the chat linkifier, a fresh instance per call since lastIndex is per-object state. It picks up /Users and /mnt roots (nothing was clickable on macOS or WSL), plus docx/pptx and video/audio extensions. - .file-preview-overlay moves to z-index 5100, above the response viewer at 5000. At its old 2000 a path clicked in the chat opened the overlay behind the panel it was launched from. Verified end to end on an isolated instance, desktop and phone viewport: real clicks in the terminal and the chat both render the image, external md and pdf render, /etc/hosts is still refused, workspace previews unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 4 +- docs/architecture-invariants.md | 12 ++ src/web/public/app.js | 67 +++++++++ src/web/public/constants.js | 39 +++++ src/web/public/panels-ui.js | 82 ++++++++++- src/web/public/styles.css | 17 ++- src/web/public/terminal-ui.js | 25 ++-- src/web/routes/file-routes.ts | 12 +- test/link-provider-regex.test.ts | 37 ++++- test/response-viewer-file-links.test.ts | 137 ++++++++++++++++++ .../file-routes-attachment-path-guard.test.ts | 48 ++++++ 11 files changed, 456 insertions(+), 24 deletions(-) create mode 100644 test/response-viewer-file-links.test.ts diff --git a/CLAUDE.md b/CLAUDE.md index a65f2017e..43bad6bda 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -230,6 +230,8 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Attachments** (live external document references; all wiring in `file-routes.ts`): a **registry** maps a stable `attachmentId` to a realpath-resolved, extension-allowlisted absolute path, so browser requests never carry arbitrary absolute paths. ⚠️ The **magic-link scanner** (`codeman://attach?...` in terminal output) is **prompt-injectable**, so its scan path is force-confined to the session workspace; a hostile prompt could otherwise exfiltrate arbitrary host files over SSE. The security gate is an extension **allowlist**, not a blocklist. `document-conversion-limiter.ts` caps converter spawns globally: without it, N large docs detected at once fork N multi-minute processes, which is a resource-exhaustion vector. → [architecture-invariants#attachments](docs/architecture-invariants.md#attachments) +**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 the response viewer's `_linkifyFilePaths()`; a fresh instance per call, since `lastIndex` is per-object state. The chat linkifier walks TEXT NODES with DOM APIs (the source is model output; never rebuild sanitized markup as a string) and skips subtrees already inside an ``. ⚠️ **An out-of-workspace path is served through the ATTACHMENT routes, not the file routes** — `file-content`/`file-raw` are workspace-confined and 404 exactly the paths agents print most (a `/tmp` capture, Claude's scratchpad), so `openFilePreview()` registers such a path via `POST /api/sessions/:id/attachments` with **`notify: false`** (suppresses only the `attachment:detected` broadcast — same guard, same routes; without it every click also popped a card announcing the file already on screen) and renders by id. The click is an explicit action on the explicit, Origin-guarded route, which is what distinguishes it from the force-confined magic-link scanner; the extension allowlist is unchanged. → [architecture-invariants#file-path-links-terminal--response-viewer](docs/architecture-invariants.md#file-path-links-terminal--response-viewer) + **Filesystem path picker** (Link Existing "Browse" + the mobile keyboard's `📁 Path` key): lazy one-directory browsing via `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` for the tapped file. Inserts the path **without** Enter, so the prompt is never submitted; the sibling `⌫ All` key clears only the unsent prompt and must never send the agent's `/clear`. ⚠️ This is a **second file-serving surface and inherits neither the attachment confinement nor its ownership scoping** — it allowlists Home, `CASES_DIR`, `/mnt/d` and `CODEMAN_FILE_PICKER_ROOTS`, blocks sensitive trees, and rejects symlink escapes **after** `realpath`. ⚠️ The optional `sessionId` is an ownership boundary that must be `canAccessOwned`-checked by hand (it does not go through `findSessionOrFail`), and in multi-user mode a non-admin gets only their own `userSpacePath` as a root: per-user spaces live INSIDE `homedir()`, so a `Home` root exposes every other user's workspace. Previews go through the same global conversion limiter, and Markdown/TXT/JSON are served as inert `text/plain`. → [architecture-invariants#filesystem-path-picker](docs/architecture-invariants.md#filesystem-path-picker) **File Viewer edit mode** (issue #212): the file-preview overlay edits workspace text files in place — `GET .../file-content?edit=1` + `PUT /api/sessions/:id/file-content`, policy in `src/config/file-editing.ts`. This is a **third file surface and the only one that WRITES**: read-path confinement (realpath + workspace + ownership) plus sensitive/blocked/`.git` denies and an extension **allowlist**; writes are `wx`-temp + rename (no `O_CREAT` anywhere = edit-in-place is structural); optimistic concurrency via sha256 `baseHash` → 409. ⚠️ `edit=1` never truncates and the client must never save a plain-preview buffer (the 500-line truncation would silently delete the rest). ⚠️ CRLF/UTF-8 guards: EOL re-applied server-side, non-UTF-8 refused via round-trip compare. → [architecture-invariants#file-viewer-edit-mode](docs/architecture-invariants.md#file-viewer-edit-mode), `docs/file-viewer-edit-plan.md` @@ -296,7 +298,7 @@ Frontend JS modules have `@fileoverview` with `@dependency`/`@loadorder` tags. L **SSE staleness watchdog** (`computeSseStale()` in constants.js, `_checkSseStale()` + a 5s interval in app.js): an `EventSource` that stops delivering does not always error, so `onerror` never fires, the header dot stays green, and every SSE-driven surface (tab status dots, sessions created on another device, renames) freezes until the user reloads. ⚠️ The 15s server keepalive was an SSE **comment** (`:keepalive`), and comments are **invisible to `EventSource` by spec**, so there was nothing a client could observe: it is now the named `sse:heartbeat` event (`cleanupDeadClients()`, sse-stream-manager.ts), which is exactly why the frame had to change type. ⚠️ Staleness is judged **only while the status is `connected`** and the device is online; that guard is the loop breaker, since a forced `connectSSE()` leaves `connected` immediately and cannot re-fire while a reconnect is in flight. ⚠️ The liveness stamp is applied inside `addListener` itself, so every registered handler (the `_SSE_HANDLER_MAP` wrappers AND the directly-registered ones) feeds it from one place; the heartbeat's own listener is a no-op that exists **only** to be registered, since `EventSource` drops named events nobody listens for. ⚠️ The watchdog interval is cleared at the top of `connectSSE()` and nowhere else (its only teardown path); clearing it elsewhere stacks intervals. Recovery needs no new sync path: the reconnect re-runs `handleInit` → `_resetAllAppState()`. The forced reconnect logs one diagnostic line, because a middlebox that strips heartbeats presents as "silently reconnects every 45s". -**Z-index layers**: subagent windows (1000), plan agents (1100), mobile/tablet fixed header (1200, `mobile.css`), modals on ≤768px (1300 — must beat the fixed header or the modal close button is buried), log viewers (2000), connection-loss overlay (2500, above the fixed header and modals), image popups (3000), local echo overlay (7). +**Z-index layers**: subagent windows (1000), plan agents (1100), mobile/tablet fixed header (1200, `mobile.css`), modals on ≤768px (1300 — must beat the fixed header or the modal close button is buried), log viewers (2000), connection-loss overlay (2500, above the fixed header and modals), image popups (3000), response viewer (5000, backdrop 4999), file-preview overlay (5100 — must outrank the response viewer, which can launch it; at its old 2000 a path clicked in the chat opened BEHIND the chat), toasts/path picker (10000+, deliberately above the preview), local echo overlay (7). **Respawn presets**: `solo-work` (3s/60min), `subagent-workflow` (45s/240min), `team-lead` (90s/480min), `ralph-todo` (8s/480min), `overnight-autonomous` (10s/480min). diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 4967662a6..a0b83acb0 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -122,6 +122,18 @@ Implementation detail extracted from `CLAUDE.md` so that file stays small enough **Attachments** (live external document references; COD-37/#119 core, COD-38/#120 previews, COD-39/#121 history): all wiring in `file-routes.ts`. **Registry** (`attachment-registry.ts`): an **in-memory** map of a stable `attachmentId` → an absolute, `realpath`-resolved, extension-allowlisted file path, so browser requests (`GET /api/sessions/:id/attachments/:attachmentId/raw`) never carry arbitrary absolute paths; `POST /api/sessions/:id/attachments` registers one. **Magic links** (`attachment-magic.ts`): parses `codeman://attach?...` out of terminal output — ⚠️ this scanner is prompt-injectable, so the scan path is **force-confined to the session workspace** (a hostile prompt could otherwise make it read arbitrary host files over SSE); emits the `attachment:detected` SSE event. Security gate is an extension **allowlist** (`isSupportedAttachmentExtension`, in the registry/magic modules), not a blocklist; a separate path layer (`config/attachment-guard.ts`) confines reads to the workspace (`attachmentConfineToWorkspace`) and blocks sensitive trees (`/root`, `/etc`). **Previews + thumbnails** (COD-38): `:attachmentId/preview` + `:attachmentId/thumbnail` (and the workspace-file equivalents `file-preview`/`file-thumbnail`) render Office docs/PDFs via external converters (`pdftoppm` / LibreOffice `soffice` / Word-COM `powershell`); `document-preview-cache.ts` is a shared disk cache (de-dups _identical_ in-flight inputs), `document-thumbnailer.ts` does best-effort first-page images, and `document-conversion-limiter.ts` is a **global converter-spawn concurrency cap** (`runWithConversionLimit`) — without it, N distinct large docs detected at once fork N multi-minute converter processes = a localhost fork-bomb-shaped resource-exhaustion vector. **History drawer** (COD-39): `session-attachment-history.ts` tracks the last `ATTACHMENT_HISTORY_LIMIT` (100) attachments per session (`Session._attachmentHistory`, persisted via `SessionState.attachmentHistory`, replayed so externals re-register on reconnect); `GET /api/sessions/:id/attachments` is the list endpoint. ⚠️ The history drawer's launcher button is desktop-only — hidden on phones (regression-guarded; see `mobile-header-buttons-policy` test). Session-local files keep using the existing workspace-scoped `file-routes` paths; the registry is only for explicit live externals. **Codex generated artifacts** (COD-166/#150, `generated-artifact-attachments.ts`): codex-mode sessions ALSO scan (ANSI-stripped) output for `Saved to: file:///…` lines and surface those files as attachment cards with a relaxed trust policy — the allow decision runs on the **realpath-resolved** path against `os.homedir()`-anchored `~/.codex` marker dirs (symlink escapes fall back to force-confinement); gated to `mode === 'codex'` only (`source` is a REQUIRED param through the listener-deps chain — a dropped arg here silently kills the feature). Image thumbnails pass through jpg/jpeg/gif/webp. +### File-path links (terminal + response viewer) + +A file path an agent prints is a link on both surfaces it can appear on, and clicking it opens the file-preview overlay. Three things make that work and each has bitten: + +**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 build a fresh instance from it. ⚠️ 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. + +**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, and nothing here widens `SUPPORTED_ATTACHMENT_EXTENSIONS` (an out-of-workspace `.svg`/`.bmp` is refused with a message naming what IS previewable). + +⚠️ **The preview overlay must outrank the panel that launched it.** `.file-preview-overlay` sits at `z-index: 5100`, above the response viewer (5000) and its backdrop (4999); at its historical 2000 a path clicked in the chat opened the overlay *behind* the chat, which reads as a dead link. It stays below the toast/picker band (10000+) so a "Saved" toast still lands on top. + ### Filesystem path picker **Filesystem path picker** (Link Existing "Browse" button + the extended mobile keyboard's `📁 Path` key): a lazy one-directory-at-a-time browser over `GET /api/filesystem/browse`, with `GET /api/filesystem/preview` serving the tapped file. It starts at the active session's working directory (falling back to `/mnt/d`), hides dot entries, and inserts the chosen path **without** Enter so the prompt is not submitted. The companion `⌫ All` key clears only the current unsent prompt buffer and must never emit the agent's `/clear` command. diff --git a/src/web/public/app.js b/src/web/public/app.js index a611a3daa..56d004178 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -2004,6 +2004,17 @@ class CodemanApp { if (!body || body.dataset.rvBound === '1') return; body.dataset.rvBound = '1'; body.addEventListener('click', async (ev) => { + // File path (_linkifyFilePaths): open it in the preview overlay, which + // resolves workspace and out-of-workspace paths alike. + const pathLink = ev.target.closest('a.rv-path'); + if (pathLink) { + ev.preventDefault(); + ev.stopPropagation(); + const filePath = pathLink.dataset.path; + if (filePath) this.openFilePreview(filePath, this.activeSessionId); + return; + } + // One-click copy: lift the raw source from the sibling
.
       const copyBtn = ev.target.closest('.rv-copy-btn');
       if (copyBtn) {
@@ -2074,10 +2085,66 @@ class CodemanApp {
     const renderedText = document.createElement('div');
     renderedText.className = 'rv-text';
     renderedText.innerHTML = this._renderMarkdown(text);
+    this._linkifyFilePaths(renderedText);
     div.appendChild(renderedText);
     return div;
   }
 
+  /**
+   * Make absolute file paths in a rendered message clickable.
+   *
+   * The terminal's link provider never sees these: the response viewer is
+   * markdown, and a path the agent wrote as prose or inline code renders as
+   * inert text — so the file it just produced (a screenshot, a report) was one
+   * copy-paste away from being viewable instead of one click. Same pattern the
+   * terminal uses (constants.js), same destination (the file-preview overlay).
+   *
+   * Walks TEXT NODES and builds anchors with DOM APIs — never innerHTML, and
+   * never a string rebuild of already-sanitized markup: the source is model
+   * 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.
+   */
+  _linkifyFilePaths(root) {
+    if (!root || typeof document === 'undefined') return;
+    // Guarded: a stale cached constants.js must degrade to plain text, not throw
+    // out of the middle of rendering a message.
+    if (typeof absoluteFilePathPattern !== 'function') return;
+    const pattern = absoluteFilePathPattern();
+
+    // Collect first: replacing a node while the walker is positioned on it
+    // invalidates the traversal.
+    const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT);
+    const targets = [];
+    for (let node = walker.nextNode(); node; node = walker.nextNode()) {
+      if (node.parentElement?.closest('a')) continue;
+      pattern.lastIndex = 0;
+      if (pattern.test(node.nodeValue || '')) targets.push(node);
+    }
+
+    for (const node of targets) {
+      const value = node.nodeValue;
+      const frag = document.createDocumentFragment();
+      let cursor = 0;
+      let match;
+      pattern.lastIndex = 0;
+      while ((match = pattern.exec(value)) !== null) {
+        const path = match[1];
+        if (match.index > cursor) frag.appendChild(document.createTextNode(value.slice(cursor, match.index)));
+        const link = document.createElement('a');
+        link.className = 'rv-path';
+        link.href = '#';
+        link.dataset.path = path;
+        link.title = path;
+        link.textContent = path;
+        frag.appendChild(link);
+        cursor = match.index + path.length;
+      }
+      if (cursor < value.length) frag.appendChild(document.createTextNode(value.slice(cursor)));
+      node.parentNode?.replaceChild(frag, node);
+    }
+  }
+
   _getResponseViewerAgentLabel() {
     const mode = this.sessions.get(this.activeSessionId)?.mode;
     return mode === 'codex'
diff --git a/src/web/public/constants.js b/src/web/public/constants.js
index 56f3852a5..0e5e0b6ed 100644
--- a/src/web/public/constants.js
+++ b/src/web/public/constants.js
@@ -893,6 +893,45 @@ function computeRewriteScrollLine(input) {
   return Math.max(0, (input?.baseY || 0) - linesFromBottom);
 }
 
+/**
+ * Absolute file paths in agent output, as ONE pattern with two consumers: the
+ * xterm link provider (terminal-ui.js) and the response viewer's markdown
+ * linkifier (app.js). They used to be able to drift, and a path that is
+ * clickable in the terminal but inert in the chat reads as a bug, not a policy.
+ *
+ * Anchored on a known absolute root (so an ordinary fraction or a date can
+ * never match) and terminated by a known extension (so the end of the path is
+ * unambiguous — a trailing `)` or `.` after the extension stays out). Longer
+ * extensions come first in each family (`tsx|ts`), so the trailing `\b` cannot
+ * be satisfied by the shorter branch mid-word.
+ *
+ * ⚠ Consumers must never share one instance: `lastIndex` is per-object state on
+ * a `/g` regex, so {@link absoluteFilePathPattern} mints a fresh one per call.
+ */
+const FILE_PATH_LINK_PATTERN =
+  /(\/(?:home|Users|tmp|var|private|etc|opt|mnt|srv|media|data|workspace)\/[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|ya?ml|csv|xml|sh|py|tsx|ts|jsx|js|mjs|cjs|css|html|toml|ini|sql|png|jpe?g|gif|webp|bmp|svg|pdf|docx|pptx|mp4|webm|mov|mp3|wav))\b/g;
+
+/** A fresh, zero-state instance of {@link FILE_PATH_LINK_PATTERN}. */
+function absoluteFilePathPattern() {
+  return new RegExp(FILE_PATH_LINK_PATTERN.source, 'g');
+}
+
+/**
+ * Extensions the file-preview overlay renders itself. Everything else a link
+ * points at goes to the tail/log viewer, which is the right home for a growing
+ * text file and the wrong one for bytes (tailing a PNG shows binary noise).
+ */
+const FILE_PREVIEW_EXTENSIONS = new Set(
+  ('png jpg jpeg gif webp bmp svg pdf docx pptx mp4 webm mov mp3 wav').split(' ')
+);
+
+/** Whether a path's extension is one {@link FILE_PREVIEW_EXTENSIONS} covers. */
+function previewsInFileViewer(filePath) {
+  const ext = String(filePath || '').split('.').pop().toLowerCase();
+  return FILE_PREVIEW_EXTENSIONS.has(ext);
+}
+
 if (typeof window !== 'undefined') {
   window.CodemanHistoryFormat = { formatHistoryBytes, computeHistoryTruncationNotice, computeRewriteScrollLine };
+  window.CodemanFilePaths = { absoluteFilePathPattern, previewsInFileViewer, FILE_PREVIEW_EXTENSIONS };
 }
diff --git a/src/web/public/panels-ui.js b/src/web/public/panels-ui.js
index fbec74645..7787f0afa 100644
--- a/src/web/public/panels-ui.js
+++ b/src/web/public/panels-ui.js
@@ -3234,6 +3234,65 @@ Object.assign(CodemanApp.prototype, {
     if (headerBtn) headerBtn.setAttribute('aria-expanded', 'false');
   },
 
+  /**
+   * Whether a path is absolute and provably OUTSIDE this session's workspace.
+   *
+   * `file-content` / `file-raw` resolve every path against `workingDir` and
+   * refuse anything that escapes it, so an absolute path elsewhere on the host
+   * (an agent's `/tmp` scratchpad capture, a screenshot, another checkout) can
+   * only ever 404 there — it has to go through the attachment routes instead.
+   *
+   * A string compare is enough for ROUTING; the real containment decision stays
+   * server-side (realpath + guard) on whichever route the request lands on. An
+   * unknown workingDir answers false, leaving the historical path untouched.
+   */
+  _isExternalPreviewPath(filePath, sessionId) {
+    if (typeof filePath !== 'string' || !filePath.startsWith('/')) return false;
+    const workingDir = this.sessions.get(sessionId)?.workingDir;
+    if (!workingDir) return false;
+    const root = workingDir.endsWith('/') ? workingDir : `${workingDir}/`;
+    return filePath !== workingDir && !filePath.startsWith(root);
+  },
+
+  /**
+   * Register an out-of-workspace 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
+   * the overlay, so the usual attachment card + unread badge would be noise on
+   * top of the thing the user just asked to see. The server still enforces the
+   * 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}>}
+   */
+  async _registerExternalPreview(filePath, sessionId) {
+    try {
+      const res = await fetch(`/api/sessions/${sessionId}/attachments`, {
+        method: 'POST',
+        headers: { 'Content-Type': 'application/json' },
+        body: JSON.stringify({ path: filePath, notify: false }),
+      });
+      const result = await res.json().catch(() => null);
+      if (res.ok && result?.success && result.data?.attachmentId) {
+        return { attachmentId: result.data.attachmentId, size: result.data.size || 0 };
+      }
+      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.
+      if (/unsupported/i.test(reason)) {
+        const ext = (filePath.split('.').pop() || '').toLowerCase();
+        return {
+          error: `Cannot preview .${ext} from outside the session workspace (images, PDF, Office documents, Markdown and text only).`,
+        };
+      }
+      return { error: reason };
+    } catch (err) {
+      return { error: err.message || 'Cannot open this file' };
+    }
+  },
+
   async openFilePreview(filePath, sessionId = this.activeSessionId, attachmentId = null) {
     if (!sessionId || !filePath) return;
 
@@ -3258,13 +3317,34 @@ Object.assign(CodemanApp.prototype, {
 
     const ext = (filePath.split('.').pop() || '').toLowerCase();
 
+    // Out-of-workspace path: mint an attachment id up front. Every branch below
+    // talks to a workspace-confined route, so without this the image/PDF ones
+    // render a broken frame and the text one reports a bare "File not found"
+    // for a file that is sitting right there on disk.
+    let externalError = '';
+    let externalSize = 0;
+    if (!attachmentId && this._isExternalPreviewPath(filePath, sessionId)) {
+      const external = await this._registerExternalPreview(filePath, sessionId);
+      attachmentId = external.attachmentId || null;
+      externalError = external.error || '';
+      externalSize = external.size || 0;
+    }
+    if (!attachmentId && externalError) {
+      footerEl.textContent = '';
+      bodyEl.innerHTML = `
${escapeHtml(externalError)}
`; + return; + } + // Registered attachment: render straight from its by-id routes — images and // PDFs inline, Office docs via the server-converted PDF preview, text fetched // raw. (Workspace-path previews fall through to the file-content endpoint.) if (attachmentId) { const base = `/api/sessions/${sessionId}/attachments/${encodeURIComponent(attachmentId)}`; const IMAGE_EXTS = new Set(['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp', 'svg']); - footerEl.textContent = ext.toUpperCase(); + // Size when we just registered the file ourselves, so a path opened from a + // link reads like a workspace preview instead of a bare "PNG". History + // cards arrive with an id and no size and keep the short form. + footerEl.textContent = externalSize ? `${this.formatFileSize(externalSize)} • ${ext}` : ext.toUpperCase(); if (IMAGE_EXTS.has(ext)) { bodyEl.innerHTML = `${escapeHtml(filePath)}`; } else if (ext === 'pdf') { diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 5df82884a..0b9b8395b 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -9855,13 +9855,18 @@ kbd { /* ========== File Preview Overlay ========== */ +/* Above the response viewer (5000) and its backdrop (4999): a file path in the + chat opens this overlay, and at the old 2000 it rendered BEHIND the panel it + was launched from — the click looked dead. Same relationship the path picker + and its preview already have (10020 / 10030). Still below the toast and + picker band (10000+), so a "Saved" toast keeps landing on top. */ .file-preview-overlay { position: fixed; inset: 0; background: var(--modal-backdrop); backdrop-filter: blur(6px); -webkit-backdrop-filter: blur(6px); - z-index: 2000; + z-index: 5100; display: none; align-items: center; justify-content: center; @@ -12413,6 +12418,16 @@ kbd { border-bottom-color: var(--accent); } +/* File paths linkified out of the message text. Monospace so a path still reads + as a path in prose, and break-all because these are long and the viewer is + narrow on a phone. Colour/underline come from the .rv-text a rule above. */ +.rv-text a.rv-path { + font-family: 'Fira Code', 'JetBrains Mono', 'SF Mono', Menlo, Monaco, monospace; + font-size: 0.92em; + word-break: break-all; + cursor: pointer; +} + /* Tables — scroll wrapper keeps table proper while allowing horizontal overflow */ .rv-table-wrap { margin: 1em 0; diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 2c36011a7..a212aa4d9 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1423,19 +1423,19 @@ Object.assign(CodemanApp.prototype, { // the whole tab on hover. Non-empty token + bounded reps is O(n). const cmdPattern = /\b(tail|cat|head|less|grep|watch|vim|nano)\s+(?:[^\s\/]+\s+){0,4}(\/[^\s"'<>|;&\n\x00-\x1f]+)/g; - // Pattern 2: Paths with common extensions. - // Image/PDF extensions are included so pasted-attachment paths - // (`.claude-images/paste-*.png`) are clickable; they open the file preview - // rather than the log viewer (see addLink). - const extPattern = - /(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\n\x00-\x1f]*\.(?:log|txt|json|md|yaml|yml|csv|xml|sh|py|ts|js|png|jpe?g|gif|webp|bmp|svg|pdf))\b/g; + // Pattern 2: Paths with common extensions. Image/PDF/media extensions are + // included so pasted-attachment paths (`.claude-images/paste-*.png`) and + // screenshots an agent just wrote are clickable; those open the file + // preview rather than the log viewer (see addLink). + // + // The literal lives in constants.js because the response viewer linkifies + // the SAME paths out of markdown — one definition, two consumers. A fresh + // instance per call: `lastIndex` is per-object state. + const extPattern = absoluteFilePathPattern(); // Pattern 3: Bash() tool output const bashPattern = /Bash\([^)]*?(\/(?:home|tmp|var|etc|opt)[^\s"'<>|;&\)\n\x00-\x1f]+)/g; - /** Extensions that should open the image/document preview, not the log viewer. */ - const PREVIEW_EXTS = new Set(['png', 'jpg', 'jpeg', 'gif', 'webp', 'bmp', 'svg', 'pdf']); - const addLink = (filePath, matchIndex) => { const startCol = lineText.indexOf(filePath, matchIndex); if (startCol === -1) return; @@ -1454,9 +1454,10 @@ Object.assign(CodemanApp.prototype, { }, activate(event, text) { // Tailing a PNG in the log viewer shows binary noise; the file preview - // already renders images and PDFs inline. - const ext = (text.split('.').pop() || '').toLowerCase(); - if (PREVIEW_EXTS.has(ext)) { + // already renders images, PDFs, documents and media inline — and it + // now reaches files outside the workspace too, which is where an + // agent's screenshots and scratchpad captures actually land. + if (previewsInFileViewer(text)) { self.openFilePreview(text, self.activeSessionId); return; } diff --git a/src/web/routes/file-routes.ts b/src/web/routes/file-routes.ts index ede34a58e..aa20b490e 100644 --- a/src/web/routes/file-routes.ts +++ b/src/web/routes/file-routes.ts @@ -1449,7 +1449,7 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even app.post('/api/sessions/:id/attachments', async (req, reply) => { const { id } = req.params as { id: string }; const session = findSessionOrFail(ctx, id, req); - const body = (req.body || {}) as { path?: string }; + const body = (req.body || {}) as { path?: string; notify?: boolean }; if (!body.path || typeof body.path !== 'string') { reply.code(400).send(createErrorResponse(ApiErrorCode.INVALID_INPUT, 'Missing attachment path')); @@ -1458,7 +1458,15 @@ export function registerFileRoutes(app: FastifyInstance, ctx: SessionPort & Even try { const event = await registerExternalAttachment(id, body.path, { sessionWorkingDir: session.workingDir }); - ctx.broadcast(SseEvent.AttachmentDetected, event); + // `notify: false` registers QUIETLY. The file-preview overlay uses it to + // mint an id for a path the user just clicked (a terminal or response-viewer + // link pointing outside the workspace): it is already opening the file, so + // the attachment card + unread badge would be noise announcing what is + // filling the screen. Default stays true — every other caller (the + // `codeman attach` CLI, codeman-publish) wants the card. + if (body.notify !== false) { + ctx.broadcast(SseEvent.AttachmentDetected, event); + } return { success: true, data: event }; } catch (err) { if (err instanceof AttachmentRegistrationError) { diff --git a/test/link-provider-regex.test.ts b/test/link-provider-regex.test.ts index e51f9f58b..ed556337a 100644 --- a/test/link-provider-regex.test.ts +++ b/test/link-provider-regex.test.ts @@ -18,18 +18,25 @@ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'fs'; import { join } from 'path'; -const SOURCE = readFileSync(join(__dirname, '..', 'src', 'web', 'public', 'terminal-ui.js'), 'utf-8'); +const publicFile = (name: string) => readFileSync(join(__dirname, '..', 'src', 'web', 'public', name), 'utf-8'); -/** Extract `const = /.../g;` from the shipped source and build the RegExp. */ +const SOURCE = publicFile('terminal-ui.js'); +// The file-path pattern lives in constants.js: the response viewer linkifies the +// same paths out of markdown, and one definition is what keeps a path that is +// clickable in the terminal from being inert in the chat. +const CONSTANTS_SOURCE = publicFile('constants.js'); + +/** Extract `const = /.../g;` from the shipped sources and build the RegExp. */ function shippedPattern(name: string): RegExp { - const m = SOURCE.match(new RegExp(`const ${name} =\\s*\\n?\\s*(/(?:[^/\\\\\\n]|\\\\.)+/[a-z]*)`)); - if (!m) throw new Error(`pattern ${name} not found in terminal-ui.js`); + const literal = new RegExp(`const ${name} =\\s*\\n?\\s*(/(?:[^/\\\\\\n]|\\\\.)+/[a-z]*)`); + const m = SOURCE.match(literal) ?? CONSTANTS_SOURCE.match(literal); + if (!m) throw new Error(`pattern ${name} not found in terminal-ui.js or constants.js`); const lit = m[1]; const lastSlash = lit.lastIndexOf('/'); return new RegExp(lit.slice(1, lastSlash), lit.slice(lastSlash + 1)); } -const PATTERN_NAMES = ['urlPattern', 'cmdPattern', 'extPattern', 'bashPattern']; +const PATTERN_NAMES = ['urlPattern', 'cmdPattern', 'FILE_PATH_LINK_PATTERN', 'bashPattern']; /** Lines that made 0.9.10's cmdPattern backtrack exponentially (>2s each). */ const KILLER_LINES = [ @@ -116,15 +123,24 @@ describe('terminal link-provider regexes (shipped source)', () => { } }); - it('extPattern links pasted image/PDF attachment paths', () => { + it('the file-path pattern links pasted image/PDF/media attachment paths', () => { // `.claude-images/paste-*.png` is what Codeman writes for a pasted screenshot; // without image extensions the path rendered as plain, unclickable text. - const ext = shippedPattern('extPattern'); + const ext = shippedPattern('FILE_PATH_LINK_PATTERN'); const cases = [ '/home/arkon/default/claudeman/.claude-images/paste-1785164958410-d11eb7d0.png', '/tmp/shot.jpeg', '/opt/app/report.pdf', '/home/a/diagram.svg', + // An agent's own scratchpad capture — the path shape this whole feature + // exists for, and the one that used to open a "File not found" preview. + '/tmp/claude-1000/-home-arkon-default-claudeman/7b3fefd2/scratchpad/probe-run-native.png', + // macOS and WSL roots: unmatched before, so Mac users had no clickable + // paths at all outside /var and /tmp. + '/Users/arbbot/codeman-cases/report.docx', + '/mnt/d/captures/demo.mp4', + // Longer extension of a family must win over its prefix (tsx over ts). + '/home/a/src/App.tsx', ]; for (const path of cases) { ext.lastIndex = 0; @@ -134,6 +150,13 @@ describe('terminal link-provider regexes (shipped source)', () => { } }); + it('terminal-ui builds its path pattern from the shared factory', () => { + // Structural guard: a local literal here would drift from the response + // viewer's linkifier, which is the divergence the move exists to prevent. + expect(SOURCE).toContain('absoluteFilePathPattern()'); + expect(SOURCE).not.toMatch(/const extPattern =\s*\n?\s*\//); + }); + it('cmdPattern arg group cannot match empty tokens (the exponential trigger)', () => { // structural guard: the dangerous construct is an empty-matchable token // inside a repeated group — `[^\s\/]*\s+` repeated. Check the pattern diff --git a/test/response-viewer-file-links.test.ts b/test/response-viewer-file-links.test.ts new file mode 100644 index 000000000..07835e45b --- /dev/null +++ b/test/response-viewer-file-links.test.ts @@ -0,0 +1,137 @@ +/** + * @fileoverview Response-viewer file-path linkifier (`CodemanApp._linkifyFilePaths`). + * + * The viewer renders markdown, so a path an agent wrote — "wrote the chart to + * /tmp/.../chart.png" — arrived as inert text: the terminal's link provider + * never sees the chat, and the file it just produced was a copy-paste away + * instead of a click. The linkifier wraps those paths in an anchor the click + * delegate hands to the file-preview overlay. + * + * Two properties matter more than the linking itself and are pinned here: + * + * 1. **The text is untouched.** Anchors are built from TEXT NODES with DOM + * APIs, never by rebuilding already-sanitized markup as a string, so the + * message reads identically and "copy code" still yields exactly what the + * agent printed. + * 2. **Model output cannot become markup.** The source is model text; a + * path-shaped string carrying HTML must stay text. + * + * Loaded via `vm` with a jsdom document injected (same technique as + * connection-indicator.test.ts — no per-file jsdom environment, which would + * externalize node:fs under vite). + */ +import { readFileSync } from 'node:fs'; +import { performance } from 'node:perf_hooks'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { JSDOM } from 'jsdom'; +import { describe, expect, it, vi } from 'vitest'; + +const dom = new JSDOM(''); +const { document, NodeFilter } = dom.window; + +function loadCodemanAppClass() { + const constants = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8'); + const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8'); + const context = vm.createContext({ + console, + performance, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + HTMLCanvasElement: class HTMLCanvasElement {}, + fetch: vi.fn(), + document, + NodeFilter, + localStorage: { length: 0, key: vi.fn(), getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + MobileDetection: {}, + }); + vm.runInContext(`${constants}\n${source}\nglobalThis.__CodemanApp = CodemanApp;`, context); + return (context as { __CodemanApp: { prototype: { _linkifyFilePaths(root: unknown): void } } }).__CodemanApp; +} + +const CodemanApp = loadCodemanAppClass(); +const APP_SOURCE = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8'); + +/** Render `html` into a detached .rv-text div and run the linkifier over it. */ +function linkify(html: string): HTMLElement { + const app = Object.create(CodemanApp.prototype) as { _linkifyFilePaths(root: unknown): void }; + const root = document.createElement('div'); + root.className = 'rv-text'; + root.innerHTML = html; + app._linkifyFilePaths(root); + return root as unknown as HTMLElement; +} + +const paths = (root: HTMLElement) => Array.from(root.querySelectorAll('a.rv-path')); + +describe('response viewer file-path linkifier', () => { + it('links an absolute path written as prose', () => { + const path = '/tmp/claude-1000/-home-arkon-default-claudeman/7b3fefd2/scratchpad/probe-run-native.png'; + const root = linkify(`

Saved the capture to ${path} — have a look.

`); + + const links = paths(root); + expect(links).toHaveLength(1); + expect(links[0].getAttribute('data-path')).toBe(path); + expect(links[0].textContent).toBe(path); + expect(root.textContent).toBe(`Saved the capture to ${path} — have a look.`); + }); + + it('links a path inside inline code, which is how agents usually write one', () => { + const root = linkify('

See /home/a/out/report.pdf for the numbers.

'); + + const links = paths(root); + expect(links).toHaveLength(1); + expect(links[0].getAttribute('data-path')).toBe('/home/a/out/report.pdf'); + // Still inside the span — the code styling is not lost. + expect(links[0].closest('code')).not.toBeNull(); + }); + + it('links every path in one text node and preserves the text between them', () => { + const root = linkify('

Compare /tmp/before.png with /tmp/after.png please

'); + + expect(paths(root).map((a) => a.getAttribute('data-path'))).toEqual(['/tmp/before.png', '/tmp/after.png']); + expect(root.textContent).toBe('Compare /tmp/before.png with /tmp/after.png please'); + }); + + it('never re-cuts text already inside an anchor', () => { + // marked autolinks URLs; a path-looking tail inside one must stay whole, and + // a nested
is invalid markup that would swallow the outer link's click. + const root = linkify('

https://example.com/x/y.png

'); + + expect(paths(root)).toHaveLength(0); + expect(root.querySelectorAll('a')).toHaveLength(1); + expect(root.querySelector('a')!.getAttribute('href')).toBe('https://example.com/x/y.png'); + }); + + it('leaves text with no path untouched', () => { + const root = linkify('

Ratio 3/4 on 2026/08/16, see src/app.ts

'); + + expect(paths(root)).toHaveLength(0); + expect(root.textContent).toBe('Ratio 3/4 on 2026/08/16, see src/app.ts'); + }); + + it('cannot turn model text into markup', () => { + // The anchor is built with createElement + textContent, so even a + // path-shaped payload stays text. (`<` also ends a match, so the linkifier + // never spans into it in the first place.) + const root = linkify('

/tmp/x.png<img src=x onerror=alert(1)>.png

'); + + expect(root.querySelector('img')).toBeNull(); + expect(root.textContent).toContain('.png'); + for (const link of paths(root)) { + expect(link.innerHTML).toBe(link.textContent); + } + }); + + it('is wired into message rendering and the click delegate', () => { + // The linkifier is only reachable through these two call sites; losing + // either leaves inert paths (no linkify) or dead links (no handler). + expect(APP_SOURCE).toContain('this._linkifyFilePaths(renderedText)'); + expect(APP_SOURCE).toMatch(/closest\('a\.rv-path'\)/); + expect(APP_SOURCE).toMatch(/openFilePreview\(filePath, this\.activeSessionId\)/); + }); +}); diff --git a/test/routes/file-routes-attachment-path-guard.test.ts b/test/routes/file-routes-attachment-path-guard.test.ts index 8a8543863..90e1066bc 100644 --- a/test/routes/file-routes-attachment-path-guard.test.ts +++ b/test/routes/file-routes-attachment-path-guard.test.ts @@ -56,6 +56,7 @@ import { registerExternalAttachment, type AttachmentRecord, } from '../../src/attachment-registry.js'; +import { SseEvent } from '../../src/web/sse-events.js'; const mockedStat = vi.mocked(fs.stat); const mockedRealpathSync = vi.mocked(realpathSync); @@ -355,4 +356,51 @@ describe('file-routes attachment path guard (COD-53)', () => { attachmentRegistry.clearSession('test-session-mlc'); }); }); + + // ===== Quiet registration (click-to-preview) ===== + // The file-preview overlay registers a clicked out-of-workspace path to mint + // an id it can render by. It is already putting the file on screen, so the + // usual attachment card + unread badge would announce what the user is + // looking at. `notify: false` suppresses ONLY the broadcast — the guard, the + // registry entry and the by-id routes are identical either way. + describe('quiet registration', () => { + const outside = '/tmp/claude-1000/scratchpad/probe-run-native.png'; + + it('broadcasts by default, so the CLI and publish paths keep their card', async () => { + mockedStat.mockResolvedValue({ size: 128, isFile: () => true, mtimeMs: 5 } as never); + const res = await harness.app.inject({ + method: 'POST', + url: `/api/sessions/${harness.ctx._sessionId}/attachments`, + payload: { path: outside }, + }); + + expect(res.statusCode).toBe(200); + expect(harness.ctx.broadcast).toHaveBeenCalledWith(SseEvent.AttachmentDetected, expect.anything()); + }); + + it('registers and serves a clicked path without broadcasting when notify is false', async () => { + const content = Buffer.from('PNGDATA'); + mockedStat.mockResolvedValue({ size: content.length, isFile: () => true, mtimeMs: 5 } as never); + mockedCreateReadStream.mockReturnValue(Readable.from([content]) as never); + + const res = await harness.app.inject({ + method: 'POST', + url: `/api/sessions/${harness.ctx._sessionId}/attachments`, + payload: { path: outside, notify: false }, + }); + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body); + expect(body.data.fileName).toBe('probe-run-native.png'); + expect(harness.ctx.broadcast).not.toHaveBeenCalled(); + + // The preview renders from this route, so the id has to be live. + const rawRes = await harness.app.inject({ + method: 'GET', + url: `/api/sessions/${harness.ctx._sessionId}/attachments/${body.data.attachmentId}/raw`, + }); + expect(rawRes.statusCode).toBe(200); + expect(rawRes.headers['content-type']).toBe('image/png'); + }); + }); }); From cbc54fc98dc57f7a21baea3fa6fe689dc62836d4 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sun, 16 Aug 2026 17:35:43 +0200 Subject: [PATCH 2/4] feat(files): play video and audio from outside the workspace too A clip an agent wrote inside the workspace played with a working scrub bar, while the same file in /tmp was refused as an unsupported type. The workspace preview classified media with its own inline extension sets and the attachment allowlist had no media at all, so the two paths disagreed about what a video is. - VIDEO_ATTACHMENT_EXTENSIONS and AUDIO_ATTACHMENT_EXTENSIONS now live in attachment-registry.ts and are imported by file-content's classification, so both paths answer the same. mp4/webm/mov/m4v/ogv and mp3/wav/ogg/oga/m4a/aac/flac/opus join the attachment allowlist. - Real MIME types for those extensions. Without one the raw route falls back to application/octet-stream, which a