Merge pull request #735: fix(links): preserve wrapped configuration filenames

This commit is contained in:
Codeman maintainer committed 2026-11-04 16:55:44 +01:00
commit 76e9f364f9
12 files changed
+500 -38

No files matched your search

+166 -4
View File
@@ -52,13 +52,13 @@ const { findFilePathLinks, resolveLinkedFilePath, terminalPathLine } = loadHelpe
const paths = (text: string) => findFilePathLinks(text).map((f) => f.path);
/** An xterm buffer stand-in: hard-broken rows (no `isWrapped`), as Claude Code emits them. */
function buffer(lines: string[], cols = 80) {
function buffer(lines: string[], cols = 80, wrappedRows = new Set<number>()) {
return {
length: lines.length,
getLine: (row: number) =>
row >= 0 && row < lines.length
? {
isWrapped: false,
isWrapped: wrappedRows.has(row),
translateToString: (trim?: boolean) => (trim ? lines[row].replace(/\s+$/, '') : lines[row].padEnd(cols)),
}
: undefined,
@@ -157,6 +157,10 @@ describe('terminalPathLine', () => {
// A path that already ended, and a next line opening a new rooted path.
expect(terminalPathLine(buffer(['see /tmp/a/b.png', 'c/d.png']), 0, 80)).toMatchObject({ endRow: 0 });
expect(terminalPathLine(buffer(['/home/joe/dir/', '/home/x.png']), 0, 80)).toMatchObject({ endRow: 0 });
expect(terminalPathLine(buffer(['see (/mnt/raid1/folder)', ' file.conf)']), 0, 80)).toMatchObject({ endRow: 0 });
expect(terminalPathLine(buffer(['see (/mnt/raid1/folder', ' /mnt/other/file.conf)']), 0, 80)).toMatchObject({
endRow: 0,
});
});
// `git ls-files`, `find` and `ls -R` print one path per row. An extensionless
@@ -237,13 +241,15 @@ function loadApp() {
app.sessions = new Map([['s1', { workingDir: '/mnt/build/neon_getaway-mgr-hebel' }]]);
app.openFilePreview = vi.fn();
app.openLogViewerWindow = vi.fn();
app._registerExternalPreview = vi.fn(async () => ({ attachmentId: 'att-fixture' }));
app.showToast = vi.fn();
app._isExternalPreviewPath = (p: string) => p.startsWith('~/');
return app;
}
function linksOn(lines: string[], row: number) {
function linksOn(lines: string[], row: number, cols = 80, wrappedRows = new Set<number>()) {
const app = loadApp();
app.terminal = { cols: 80, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines) } };
app.terminal = { cols, registerLinkProvider: vi.fn(), buffer: { active: buffer(lines, cols, wrappedRows) } };
const provider = app.registerFilePathLinkProvider() as Provider;
let out: Link[] = [];
provider.provideLinks(row, (links) => {
@@ -253,6 +259,162 @@ function linksOn(lines: string[], row: number) {
}
describe('terminal link provider', () => {
it.each(['svg', 'markdown', 'avif', 'bmp', 'ico', 'zip'])(
'keeps the workspace opener for a hard-wrapped .%s rejected by the attachment registry',
async (extension) => {
for (const row of [1, 2]) {
const path = `/home/me/proj/out/system_architecture.${extension}`;
const { app, links } = linksOn(
['Saved the diagram (/home/me/proj/out/system_archi', ` tecture.${extension}).`],
row,
120
);
app.sessions = new Map([['s1', { workingDir: '/home/me/proj' }]]);
app._registerExternalPreview.mockResolvedValue({ status: 400, error: 'Unsupported attachment type' });
await links[0].activate({}, links[0].text);
const opener = extension === 'zip' ? app.openLogViewerWindow : app.openFilePreview;
expect(opener).toHaveBeenCalledWith(path, 's1');
expect(app._registerExternalPreview.mock.calls).toEqual([[path, 's1']]);
expect(app.showToast).not.toHaveBeenCalled();
}
}
);
it.each([
[
['Saved the screenshot (/home/me/proj/out/ghost1b_game', ' play.png).'],
'/home/me/proj/out/ghost1b_gameplay.png',
],
[
[' ⎿ Wrote (/mnt/build/neon/builds/captures/ghost1b_gamepl', ' ay.png) ok'],
'/mnt/build/neon/builds/captures/ghost1b_gameplay.png',
],
[
['Saved the screenshot (/home/me/proj/out/ghost1b_', ' very_long_game', ' play.png).'],
'/home/me/proj/out/ghost1b_very_long_gameplay.png',
],
])('preserves a parenthesised filename without spaces across hard wraps: %s', async (rows, path) => {
for (let row = 1; row <= rows.length; row++) {
const { app, links } = linksOn(rows, row, 120);
expect(links.map((link) => link.text)).toEqual([path]);
await links[0].activate({}, links[0].text);
expect(app.openFilePreview).toHaveBeenCalledWith(path, 's1');
expect(app._registerExternalPreview.mock.calls).toEqual([[path, 's1']]);
}
});
it('uses the spaced filename only when the unspaced hard-wrap target is missing', async () => {
const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf';
const rows = [
'Found it: I prepared a corrected configuration (/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo',
' eReader.conf).',
];
for (const row of [1, 2]) {
const { app, links } = linksOn(rows, row, 120);
const unspaced = path.replace('Kobo eReader', 'KoboeReader');
expect(links.map((link) => link.text)).toEqual([unspaced]);
expect(links[0].range.end).toEqual({ x: ' eReader.conf'.length + 1, y: 2 });
app._registerExternalPreview.mockImplementation(async (candidate: string) =>
candidate === path ? { attachmentId: 'att-fixture' } : { status: 404, error: 'File not found' }
);
await links[0].activate({}, links[0].text);
expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1');
expect(app._registerExternalPreview.mock.calls).toEqual([
[unspaced, 's1'],
[path, 's1'],
]);
}
});
it.each([403, 500, 503])('does not try another path after a guarded lookup returns HTTP %s', async (status) => {
const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120);
app._registerExternalPreview.mockResolvedValue({ status, error: 'Lookup refused' });
await links[0].activate({}, links[0].text);
expect(app._registerExternalPreview).toHaveBeenCalledTimes(1);
expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1');
expect(app.showToast).not.toHaveBeenCalled();
});
it('keeps the original opener when the lookup cannot reach the server', async () => {
const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120);
app._registerExternalPreview.mockResolvedValue({ error: 'Connection lost' });
await links[0].activate({}, links[0].text);
expect(app._registerExternalPreview).toHaveBeenCalledTimes(1);
expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1');
});
it('uses the original opener when both hard-wrap filenames are missing', async () => {
for (const row of [1, 2]) {
const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], row, 120);
app._registerExternalPreview.mockResolvedValue({ status: 404, error: 'Attachment file not found' });
await links[0].activate({}, links[0].text);
expect(app._registerExternalPreview.mock.calls).toEqual([
['/mnt/data/KoboeReader.conf', 's1'],
['/mnt/data/Kobo eReader.conf', 's1'],
]);
expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1');
expect(app.showToast).not.toHaveBeenCalled();
}
});
it.each(['Path: ', '- ', '- Path: ', ' Path: '])(
'resolves the filename without its parenthesised prefix %j',
async (prefix) => {
for (const row of [1, 2]) {
const { app, links } = linksOn([`Saved the notes (${prefix}/home/me/proj/out/My`, ' Notes.md).'], row, 120);
app._registerExternalPreview
.mockResolvedValueOnce({ status: 404, error: 'Attachment file not found' })
.mockResolvedValueOnce({ attachmentId: 'att-spaced' });
expect(links.map((link) => link.text)).toEqual(['/home/me/proj/out/MyNotes.md']);
await links[0].activate({}, links[0].text);
expect(app._registerExternalPreview.mock.calls).toEqual([
['/home/me/proj/out/MyNotes.md', 's1'],
['/home/me/proj/out/My Notes.md', 's1'],
]);
expect(app.openFilePreview).toHaveBeenCalledWith('/home/me/proj/out/My Notes.md', 's1');
}
}
);
it.each([400, 403, 500, 503])('does not open an unregistered spaced alternative after HTTP %s', async (status) => {
const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120);
app._registerExternalPreview
.mockResolvedValueOnce({ status: 404, error: 'File not found' })
.mockResolvedValueOnce({ status, error: 'Alternative unavailable' });
await links[0].activate({}, links[0].text);
expect(app._registerExternalPreview.mock.calls).toEqual([
['/mnt/data/KoboeReader.conf', 's1'],
['/mnt/data/Kobo eReader.conf', 's1'],
]);
expect(app.openFilePreview).not.toHaveBeenCalled();
expect(app.openLogViewerWindow).not.toHaveBeenCalled();
expect(app.showToast).toHaveBeenCalledWith('Alternative unavailable', 'error');
});
it('keeps the session selected at click time while a path lookup is pending', async () => {
const { app, links } = linksOn(['Saved (/mnt/data/Kobo', ' eReader.conf).'], 1, 120);
const pending = links[0].activate({}, links[0].text);
app.activeSessionId = 'another-session';
await pending;
expect(app._registerExternalPreview).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1');
expect(app.openLogViewerWindow).toHaveBeenCalledWith('/mnt/data/KoboeReader.conf', 's1');
});
it('opens the complete parenthesised configuration path from either wrapped row', () => {
const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf';
const text = `Found it: I prepared a corrected configuration (${path}).`;
const split = text.indexOf(' eReader') + 1;
const rows = [text.slice(0, split), text.slice(split)];
for (const row of [1, 2]) {
const { app, links } = linksOn(rows, row, split, new Set([1]));
expect(links.map((link) => link.text)).toEqual([path]);
expect(links[0].range.start).toEqual({ x: text.indexOf(path) + 1, y: 1 });
expect(links[0].range.end).toEqual({ x: 'eReader.conf'.length + 1, y: 2 });
links[0].activate({}, links[0].text);
expect(app.openLogViewerWindow).toHaveBeenCalledWith(path, 's1');
}
});
it('links a broken path once, across both rows, and opens the whole path', () => {
const { app, links } = linksOn(BROKEN, 2);
expect(links.map((l) => l.text)).toEqual([BROKEN_PATH]);
+53
View File
@@ -212,6 +212,59 @@ function loadApp(fetchImpl: (url: string, init?: { body?: string }) => unknown)
return { app, elements, keyHandlers, fetch: context.fetch as ReturnType<typeof vi.fn> };
}
describe('guarded attachment registration results', () => {
it.each(['svg', 'markdown', 'avif', 'bmp', 'ico', 'zip'])(
'names a rejected workspace .%s path without describing it as external',
async (extension) => {
const { app } = loadApp(async () => ({
ok: false,
status: 400,
json: async () => ({ error: 'Unsupported attachment type' }),
}));
const path = `/home/joe/wiki/out/system architecture.${extension}`;
const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, path, 's1');
expect(result.status).toBe(400);
expect(result.error).toContain(path);
expect(result.error).not.toContain('outside the session workspace');
}
);
it('retains the external-file explanation for a rejected external type', async () => {
const { app } = loadApp(async () => ({
ok: false,
status: 400,
json: async () => ({ error: 'Unsupported attachment type' }),
}));
const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(
app,
'/mnt/data/system architecture.svg',
's1'
);
expect(result.error).toBe(
'Cannot preview .svg from outside the session workspace (images, video, audio, PDF, Office documents and text files only).'
);
});
it.each([404, 403, 500, 503])('preserves HTTP %s for the activation decision', async (status) => {
const { app, fetch } = loadApp(async () => ({
ok: false,
status,
json: async () => ({ error: 'Cannot read file' }),
}));
const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, '/mnt/data/a.conf', 's1');
expect(result).toEqual({ error: 'Cannot read file', status });
expect(JSON.parse(fetch.mock.calls[0][1].body)).toEqual({ path: '/mnt/data/a.conf', notify: false });
});
it('does not turn a connection failure into a missing file', async () => {
const { app } = loadApp(async () => {
throw new Error('Connection lost');
});
const result = await Object.getPrototypeOf(app)._registerExternalPreview.call(app, '/mnt/data/a.conf', 's1');
expect(result).toEqual({ error: 'Connection lost' });
});
});
const LISTING = {
folderId: 'fld_1',
path: '/mnt/raid1/styleframes',
+9
View File
@@ -78,6 +78,15 @@ function linkify(html: string): HTMLElement {
const paths = (root: HTMLElement) => Array.from(root.querySelectorAll('a.rv-path'));
describe('response viewer file-path linkifier', () => {
it('links a parenthesised configuration filename with spaces without changing the prose', () => {
const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf';
const text = `Prepared a corrected configuration (${path}).`;
const root = linkify(`<p>${text}</p>`);
expect(paths(root).map((a) => a.dataset.path)).toEqual([path]);
expect(paths(root)[0].dataset.folder).toBeUndefined();
expect(root.textContent).toBe(text);
});
it.each([
['- 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx', ['Design/Build-SIPOC.pptx']],
['- notes/a b/c.md', ['b/c.md']],
+26
View File
@@ -28,6 +28,32 @@ const LONG =
'/mnt/c/Users/someone/OneDrive - Contoso/01_Projects/Other/kb/out/deliverables/fy27-messaging-market-story.docx';
describe('spaced file paths', () => {
it('keeps a parenthesised configuration filename whole inside a sentence', () => {
const path = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf';
const text = `Prepared a corrected configuration (${path}).`;
expect(findFilePathLinks(text)).toEqual([{ path, index: text.indexOf(path) }]);
});
it('keeps separate parenthesised paths and the surrounding prose separate', () => {
const first = '/mnt/data/Kobo/Kobo eReader.conf';
const second = '/home/me/My Docs/notes.md';
expect(paths(`Updated (${first}) and (${second}).`)).toEqual([first, second]);
expect(paths('Do not link (/etc/private files/settings.conf).')).toEqual([]);
expect(paths('Do not link (/mnt/data/../private files/settings.conf).')).toEqual([]);
});
it('does not take a comma-separated list inside parentheses as one spaced filename', () => {
expect(paths('Updated (/home/me/proj, config.json)')).toEqual(['/home/me/proj', 'config.json']);
});
it.each([
['/mnt/c/Users/me/Smith, John/report.docx', '/mnt/c/Users/me/Smith, John/report.docx'],
['- /home/me/Clients/Acme, Inc/contract.pdf', '/home/me/Clients/Acme, Inc/contract.pdf'],
['Saved to: /home/me/proj/Invoice 12, final.pdf', '/home/me/proj/Invoice 12, final.pdf'],
])('keeps commas in a non-parenthesised spaced path: %s', (line, path) => {
expect(findFilePathLinks(line)).toEqual([{ path, index: line.indexOf(path) }]);
});
it('links an own-line absolute path with spaces whole', () => {
expect(paths(LONG)).toEqual([LONG]);
expect(paths('/mnt/c/proj/My Big Docs/report.docx')).toEqual(['/mnt/c/proj/My Big Docs/report.docx']);
+133
View File
@@ -0,0 +1,133 @@
/** Real xterm clicks resolve the complete filename or report its lookup error from either wrapped row. */
import { readFileSync } from 'node:fs';
import { resolve } from 'node:path';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { chromium, type Browser } from 'playwright';
const publicDir = resolve(import.meta.dirname, '../src/web/public');
const basePath = '/mnt/user/Scratch/Kobo/Set-KoboCovers/Kobo eReader.conf';
describe('wrapped configuration file links (Chromium)', () => {
let browser: Browser;
beforeAll(async () => {
browser = await chromium.launch({ headless: true });
});
afterAll(async () => {
await browser.close();
});
it.each([
{ wrap: 'soft', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' },
{ wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 200, prefix: '' },
{ wrap: 'hard', originalExists: true, extension: 'conf', registrationStatus: 200, prefix: '' },
{ wrap: 'hard', originalExists: true, extension: 'svg', registrationStatus: 400, prefix: '' },
{ wrap: 'hard', originalExists: true, extension: 'markdown', registrationStatus: 400, prefix: '' },
{ wrap: 'hard', originalExists: false, extension: 'svg', registrationStatus: 400, prefix: '' },
{ wrap: 'hard', originalExists: false, extension: 'conf', registrationStatus: 404, prefix: '' },
{ wrap: 'hard', originalExists: false, extension: 'markdown', registrationStatus: 200, prefix: 'Path: ' },
])(
'handles .$extension links on both rows: $wrap, original exists: $originalExists, prefix: $prefix',
async ({ wrap, originalExists, extension, registrationStatus, prefix }) => {
const path = basePath.replace('.conf', `.${extension}`);
const original = path.replace('Kobo eReader', 'KoboeReader');
const expected = wrap === 'hard' && (originalExists || registrationStatus === 404) ? original : path;
const lookupFails = registrationStatus === 400 && !originalExists;
const lookups: string[] = [];
const page = await browser.newPage({ viewport: { width: 1920, height: 600 } });
try {
await page.route('http://fixture.test/', (route) =>
route.fulfill({
contentType: 'text/html',
body: '<div id="terminal"></div>',
})
);
await page.route('http://fixture.test/api/sessions/fixture/attachments', async (route) => {
const body = route.request().postDataJSON();
expect(body.notify).toBe(false);
lookups.push(body.path);
const exists = body.path === path || (originalExists && body.path === original);
const status = exists ? registrationStatus : 404;
await route.fulfill({
status,
json:
status === 200
? { success: true, data: { attachmentId: 'att-fixture' } }
: { success: false, error: status === 400 ? 'Unsupported attachment type' : 'File not found' },
});
});
await page.goto('http://fixture.test/');
await page.addStyleTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/css/xterm.css') });
await page.addScriptTag({ path: resolve(publicDir, '../../../node_modules/@xterm/xterm/lib/xterm.js') });
await page.addScriptTag({ content: 'window.CodemanApp = class CodemanApp {};' });
for (const file of ['constants.js', 'terminal-ui.js', 'panels-ui.js']) {
await page.addScriptTag({ content: readFileSync(resolve(publicDir, file), 'utf8') });
}
await page.evaluate(
async ({ path, wrap, workingDir, prefix }) => {
const w = window as any;
const boundary = path.lastIndexOf(' ');
const first = `Found it: prepared a corrected configuration (${prefix}${path.slice(0, boundary)}`;
const terminal = new w.Terminal({ cols: first.length + (wrap === 'soft' ? 1 : 20), rows: 6 });
terminal.open(document.getElementById('terminal'));
w.opened = [];
w.app = Object.assign(new w.CodemanApp(), {
terminal,
activeSessionId: 'fixture',
sessions: new Map([['fixture', { workingDir }]]),
openFilePreview: (file: string) => w.opened.push(file),
openLogViewerWindow: (file: string) => w.opened.push(`log:${file}`),
openFolderGrid: (file: string) => w.opened.push(`folder:${file}`),
showToast: (message: string) => w.opened.push(`error:${message}`),
});
w.app.registerFilePathLinkProvider();
const text = `${first}${wrap === 'soft' ? ' ' : '\r\n '}${path.slice(boundary + 1)}).`;
await new Promise<void>((done) => terminal.write(text, done));
await new Promise<void>((done) => requestAnimationFrame(() => requestAnimationFrame(() => done())));
},
{ path, wrap, workingDir: registrationStatus === 400 ? '/mnt/user/Scratch/Kobo' : '/workspace', prefix }
);
expect(lookups).toEqual([]);
for (const row of [0, 1]) {
const point = await page.evaluate(
async ({ path, row }) => {
const app = (window as any).app;
const terminal = app.terminal;
const links = await new Promise<any[]>((done) => app._terminalLinkProvider.provideLinks(row + 1, done));
const link = links.find((candidate) => candidate.text === path);
if (!link) throw new Error(`The complete filename is not linked on row ${row + 1}`);
const text = terminal.buffer.active.getLine(row).translateToString(true);
const col = row === 0 ? link.range.start.x : text.indexOf('eReader') + 1;
const box = terminal.element.querySelector('.xterm-screen').getBoundingClientRect();
return {
x: box.left + (col - 0.5) * (box.width / terminal.cols),
y: box.top + (row + 0.5) * (box.height / terminal.rows),
};
},
{ path: wrap === 'hard' ? original : path, row }
);
await page.mouse.move(point.x, point.y);
await page.waitForFunction(() => (window as any).app._linkHovered === true);
await page.mouse.click(point.x, point.y);
await page.waitForFunction((count) => (window as any).opened.length === count, row + 1);
const opened = await page.evaluate(() => (window as any).opened as string[]);
if (lookupFails) {
for (const message of opened) {
expect(message).toMatch(/^error:/);
expect(message).toContain(path);
expect(message).not.toContain('outside the session workspace');
}
} else {
expect(opened).toEqual(Array(row + 1).fill(expected));
}
await page.mouse.move(0, 0);
}
expect(lookups).toEqual(
wrap === 'soft' ? [] : originalExists ? [original, original] : [original, path, original, path]
);
} finally {
await page.close();
}
}
);
});