mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 14:39:42 +02:00
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 <a>, 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) <noreply@anthropic.com>
This commit is contained in:
@@ -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 <name> = /.../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 <name> = /.../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
|
||||
|
||||
@@ -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('<!DOCTYPE html><html><body></body></html>');
|
||||
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(`<p>Saved the capture to ${path} — have a look.</p>`);
|
||||
|
||||
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('<p>See <code>/home/a/out/report.pdf</code> for the numbers.</p>');
|
||||
|
||||
const links = paths(root);
|
||||
expect(links).toHaveLength(1);
|
||||
expect(links[0].getAttribute('data-path')).toBe('/home/a/out/report.pdf');
|
||||
// Still inside the <code> 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('<p>Compare /tmp/before.png with /tmp/after.png please</p>');
|
||||
|
||||
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 <a> is invalid markup that would swallow the outer link's click.
|
||||
const root = linkify('<p><a href="https://example.com/x/y.png">https://example.com/x/y.png</a></p>');
|
||||
|
||||
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('<p>Ratio 3/4 on 2026/08/16, see src/app.ts</p>');
|
||||
|
||||
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('<p>/tmp/x.png<img src=x onerror=alert(1)>.png</p>');
|
||||
|
||||
expect(root.querySelector('img')).toBeNull();
|
||||
expect(root.textContent).toContain('<img src=x onerror=alert(1)>.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\)/);
|
||||
});
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user