diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 2f68fad8..68559b28 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -409,7 +409,7 @@ A file path an agent prints is a link on both surfaces it can appear on, and cli **Path forms beyond the rooted one.** `findFilePathLinks(text)` (constants.js) is the single entry point both consumers call; it returns the rooted matches first, then `~/` and relative paths (`RELATIVE_FILE_PATH_LINK_PATTERN`, same extension list, pinned equal by `test/link-provider-regex.test.ts`), then folder paths, each only where it overlaps neither an earlier match nor an `http(s)` URL. A relative match must start at a boundary (line start, whitespace, quote, bracket, `=`, `,`, `>`), so it can never begin inside a word, an absolute path or a URL; brackets, `=` and `,` end it (`(shot.png)`, `OUT=shots/a.png`). `resolveLinkedFilePath()` resolves a relative path against the session's `workingDir` (`.`/`..` collapsed, so `../x.png` lands outside and routes like any external path) and passes absolute and `~/` paths through. ⚠️ `~` is the home of the SESSION'S host, which only the server knows: `~/` paths always take the attachment route, where `registerExternalAttachment` expands them (`homedir()` locally, `$HOME` inside the remote probe script), and the guard runs on the expanded, realpath-resolved path as before. ⚠️ Programs hard-wrap their own output short of the terminal edge (Claude Code breaks tool output at 59-118 columns), so xterm's `isWrapped` never fires. `terminalPathLine()` (link provider only; touch selection keeps `terminalLogicalLine`) joins the next row when the previous row ends in a path token with a `/` and no extension and the join reaches an extension; a row that starts a new path is never joined: rooted or `~/`, `./`/`../`, or a relative one that opens on the same top directory the broken token did (the next entry of `git ls-files`/`find`/`ls -R`: `src/components` + `src/index.ts` are two paths, not `src/componentssrc/index.ts`), while a wrapped path continues INSIDE a name, so `…/captures/2` + `026-10-10/…` still joins; a 3+ row chain is kept only if a later row completes the path, and the broken row must be at least as long as the continuing one (keeps `cd /mnt/foo` + a longer next line apart). Tests: `test/file-path-link-forms.test.ts`. -The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers and version tokens never continue a URL. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently, gated by `pathContinuation()` and the per-cell row-length rule; accepted paths displace other links on their cells. Full-width divider rows stop `terminalLogicalLine()` stitching for both links and touch selection. OSC 8 links confirm the real target URI before the shared opener runs. +The terminal provider scans `terminalLinkLine()`, which extends `terminalPathLine()` for URL continuations in indented paragraphs and table cells. List markers, bare numbers and version tokens never continue a URL. A token with only hyphens, underscores or dots must end its cell (row end or a gap of at least two spaces), so prose such as `self-hosted runners` is not appended. A row end counts as a cell end, so `⏺ Docs live at https://example.com/guide/` followed by ` self-hosted.` still links `.../guide/self-hosted`, and an indented `and/or the API reference covers it.` still links `.../guide/and/or`; both are pinned as known trade-offs. A continuation must be indented (a column-0 row is new output, which keeps `status=200`, `24/7` and `really?` out), and a shell prompt (`user@host:`), a home path (`~/`) or an HTTP status line never continues a URL. A divider row never continues the row above it, so a URL that reaches the right edge does not absorb a full-width `────` row below. A real plain slug followed by prose falls back to the original fragment link; `%`, `&`, `=`, `/` or `?` still qualify as stronger URL punctuation. `terminalCellWrappedPaths()` follows each table cell independently, gated by `pathContinuation()` and the per-cell row-length rule; accepted paths displace other links on their cells. Full-width divider rows stop `terminalLogicalLine()` stitching for both links and touch selection. OSC 8 links confirm the real target URI before the shared opener runs. **Folder grid: a folder is granted, then served by bare file name.** A folder path (rooted or `~/` whose last segment has no extension, relative only with a trailing `/`; `FOLDER_PATH_LINK_PATTERN` + `folderLinkPath()`) opens `openFolderGrid()` (panels-ui.js), as does the preview's "Show folder" button. `POST /api/sessions/:id/folders {path}` (`src/folder-listing.ts`) resolves the folder (locally a bounded `probePathKind(…, { pastCap: true })` first, then async `fs.realpath`; remotely one ssh round trip via `remoteListFolder`, through `runWithRemoteSshLimit`), runs the attachment guard on it (blocklist, `confineToWorkspace`, and in multi-user mode `folderAllowedForUser`: a non-admin only inside their own `userSpacePath` or the session's `workingDir`, as the filesystem path picker), locally BEFORE reading it, lists non-hidden subfolders and files of a supported attachment type (each file also blocklist-checked, newest first, capped at `MAX_FOLDER_ENTRIES`), and GRANTS it an id. Files are then fetched as `GET …/folders/:folderId/files/:name/(raw|preview|thumbnail)`: `:name` must be a bare file name of a supported type (no `/`, `\`, NUL, no leading `.`, since the listing hides those), the local file is probed bounded (`unknown` → 503, never 404), and the joined path goes through the SAME `resolveServableAttachmentPath` as an attachment by id (symlink re-resolution, remote re-probe, guard), then the same gate registration applies to what a symlink RESOLVES to (a supported extension on the resolved path and a regular file: locally `fs.stat`, remotely the probe kind; else 400, so `shot.png -> ../secret.bin` is not served and a FIFO or device behind a supported name never reaches `open`), then `folderAllowedForUser` once more on the resolved file, before `serveServableRaw` (the attachment raw route's body, shared) streams it. So browser requests still never carry a path. ⚠️ Grants live in their own registry (`folderGrants`, 32 per session, cleared with the session's attachments), NOT the attachment registry: that one is capped at 200 per session, and paging a folder of 300 renders would evict the session's real attachments. ⚠️ A local folder is never touched unbounded (#516 rule): a folder on a network mount that stopped answering answers 503 with `describeUnknownPath()`, not 404 and not a frozen server. ⚠️ The remote listing script tells "missing" (`E|n`, 404) from "is a file" (`E|f`, 400 `Not a folder`), because the client falls back to the file preview on `Not a folder`; a remote extensionless file otherwise answered "Folder not found". ⚠️ `.folder-grid-overlay` sits at `z-index: 5090`, one step BELOW the preview (5100) and above the response viewer (5000): a tile's preview opens on top and closing it lands back on the grid. ⚠️ ←/→/Esc are handled in a document CAPTURE listener with `stopPropagation`, because the terminal keeps keyboard focus under the overlays and an arrow that reached xterm would be typed into the session; inputs other than xterm's helper textarea keep their keys. ⚠️ A route PARAMETER is capped by the router (Fastify's default is 100 characters, answered as 404 "Route not found"), and a file name goes up to 255, percent-encoded: the server sets `routerOptions.maxParamLength` to `MAX_ROUTE_PARAM_LENGTH` (route-helpers.ts), the route test harness uses the same constant, and a wiring guard pins it. ⚠️ `~/` in a Docker case is refused with 400 (attachment POST and `POST …/folders`): the session is local for these routes, but `~` inside the container is the container's own home, and expanding it on the host would open a different file than the one the agent printed. Tests: `test/routes/folder-grid-routes.test.ts` (incl. the listing script on a real `sh`), `test/folder-grid-ui.test.ts`. diff --git a/src/web/public/constants.js b/src/web/public/constants.js index b0bf0f2b..3f8f5d23 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2333,6 +2333,9 @@ function terminalLogicalLine(buffer, row, cols, maxRows) { const continuesPrevious = (r) => { if (r <= 0) return false; if (lineAt(r)?.isWrapped) return true; + // A divider row never continues the row above, even when that row reaches + // the right edge: a URL ending at the edge must not absorb `────` below it. + if (/^[\s─━═╌╍┄┅┈┉-]+$/u.test(lineAt(r)?.translateToString(true) || '')) return false; const prev = lineAt(r - 1); const prevText = prev?.translateToString(true) || ''; // A full-width table divider is a row boundary, not a hard-wrapped line. @@ -2565,11 +2568,12 @@ function findTerminalUrls(text) { * linked a fragment. A row continues the URL above it only when ALL hold: * - the URL fragment ends on a character a wrapper breaks after * (`/ ? & = - _ % # + ~`); a following table column may be excluded; - * - the next row is indented no further right than the column the URL + * - the next row is indented, but no further right than the column the URL * started in (a table cell or list item continuation, not unrelated text); * - that row's first token looks like URL material (contains one of * `% & = / ? . - _`), is not a bare number/version or list marker, - * and is not itself a new `scheme://`; without `% & = / ?`, the token + * and is not itself a new `scheme://`, a shell prompt (`user@host:`), + * a home path (`~/`) or an HTTP status line; without `% & = / ?`, the token * must finish its cell (row end or a gap of at least two spaces). * The last rule is what stops prose such as "…/files/" + "then run it" from * being glued into one link. @@ -2611,7 +2615,10 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { const nextText = next.translateToString(true) || ''; const indent = nextText.length - nextText.replace(/^\s+/, '').length; const token = nextText.slice(indent).split(/\s/, 1)[0]; - if (!token || indent > urlStartCol || /^(?:\d+[.)]|[-*•])(?:\s|$)/.test(nextText.slice(indent))) break; + // A hard continuation is indented (a table cell, a list item, Claude's + // two-space paragraph indent); a column-0 row is a new line of output. + if (!token || !indent || indent > urlStartCol || /^(?:\d+[.)]|[-*•])(?:\s|$)/.test(nextText.slice(indent))) + break; // When other table columns follow the URL, the next row must align with // the start of its cell. A new numbered row must not become URL text. if ( @@ -2623,6 +2630,9 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { ) break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.test(token)) break; + // A shell prompt (`user@host:~/dir$`), a home path and an HTTP status line + // are never the rest of a URL, however much URL punctuation they carry. + if (/^[\w.-]+@[\w.-]+:|^~\/|^HTTP\/\d/i.test(token)) break; if (!/[%&=/?.\-_]/.test(token) || /^v?\d+(?:\.\d+)*(?:[-+][\w.-]+)?[.,;:!?)]?$/i.test(token)) break; // Hyphens, underscores and dots also occur in prose. Without stronger // URL punctuation, the token must finish its cell, not introduce a sentence. diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 64b4868a..b03a5ccf 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -30,15 +30,20 @@ type Link = { function loadPure() { const context = vm.createContext({ console, window: undefined }); vm.runInContext( - `${CONSTANTS}\nglobalThis.__urls = findTerminalUrls; globalThis.__extent = terminalUrlContinuationExtent;`, + `${CONSTANTS}\nglobalThis.__urls = findTerminalUrls; globalThis.__extent = terminalUrlContinuationExtent; globalThis.__logical = terminalLogicalLine;`, context, { filename: 'constants.js' } ); const ctx = context as unknown as { __urls: (text: string) => UrlRef[]; __extent: (buffer: unknown, row: number, cols: number, maxRows?: number) => Extent; + __logical: (buffer: unknown, row: number, cols: number, maxRows?: number) => Extent; + }; + return { + findTerminalUrls: ctx.__urls, + terminalUrlContinuationExtent: ctx.__extent, + terminalLogicalLine: ctx.__logical, }; - return { findTerminalUrls: ctx.__urls, terminalUrlContinuationExtent: ctx.__extent }; } function fakeBuffer(rows: Row[], cols: number) { @@ -102,6 +107,29 @@ const PROSE_ROWS = [ ['Callback https://example.com/cb?code=', 'Re-run the login', 'https://example.com/cb?code='], ['Branch https://github.com/o/r/tree/feat-', 'x.y happened', 'https://github.com/o/r/tree/feat-'], ]; +// Rows that carry URL punctuation but are never the rest of the URL above them: +// a shell prompt, an HTTP status line, a home path, and column-0 prose. Each +// third element is the URL, the fourth the path link expected on the second row. +const NOT_CONTINUATION_ROWS: [string, string, string, string | null][] = [ + ['https://registry.npmjs.org/', 'arkon@box:~/proj$', 'https://registry.npmjs.org/', '~/proj'], + ['https://pypi.org/simple/', 'dev@laptop:/srv/app$', 'https://pypi.org/simple/', '/srv/app'], + ['$ curl -sI https://example.com/', 'HTTP/2 200', 'https://example.com/', null], + ['$ curl -i http://localhost:3000/api/', 'HTTP/1.1 200 OK', 'http://localhost:3000/api/', null], + ['Docs: https://example.com/guide/', '~/notes.md has more', 'https://example.com/guide/', '~/notes.md'], + ['fetch https://api.example.com/items/', 'status=200 took=12ms', 'https://api.example.com/items/', null], + ['Status page: https://status.example.com/', '24/7 monitoring is on.', 'https://status.example.com/', null], + ['Did you mean https://example.com/search?q=', 'really? no.', 'https://example.com/search?q=', null], +]; +// Known trade-offs: a row end counts as a cell end, so a short indented word +// with URL punctuation still continues the URL. Pinned so a change is deliberate. +const TRADE_OFF_ROWS: [string, string, string][] = [ + ['⏺ Docs live at https://example.com/guide/', ' self-hosted.', 'https://example.com/guide/self-hosted'], + [ + '⏺ The docs are at https://example.com/guide/', + ' and/or the API reference covers it.', + 'https://example.com/guide/and/or', + ], +]; describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); @@ -144,7 +172,7 @@ describe('findTerminalUrls (grammar)', () => { }); describe('terminalUrlContinuationExtent (wrap joining)', () => { - const { terminalUrlContinuationExtent } = loadPure(); + const { terminalUrlContinuationExtent, terminalLogicalLine } = loadPure(); it('spans all three rows of a URL wrapped inside a table cell, from any of them', () => { const buffer = fakeBuffer(TABLE, COLS); @@ -196,10 +224,42 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); }); + it.each(NOT_CONTINUATION_ROWS)('does not join a prompt, status line, home path or prose after %s', (first, next) => { + const buffer = fakeBuffer([{ text: first }, { text: next }], 100); + for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); + }); + + // Indented, so only the prompt / home path / status-line rule can refuse them. + it.each(['arkon@box:~/proj$', 'dev@laptop:/srv/app$', '~/notes.md', 'HTTP/2 200', 'HTTP/1.1 200 OK'])( + 'does not join an indented %s either', + (next) => { + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + next }], 100); + for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); + } + ); + + it.each(TRADE_OFF_ROWS)('still joins the known trade-off after %s', (first, next) => { + const buffer = fakeBuffer([{ text: first }, { text: next }], 100); + for (const row of [0, 1]) + expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + }); + + it('does not let a URL at the right edge absorb a full-width divider row below it', () => { + const cols = 40; + const url = 'https://example.com/' + 'a'.repeat(cols - 'https://example.com/'.length); + for (const divider of ['─'.repeat(cols), '-'.repeat(cols)]) { + const buffer = fakeBuffer([{ text: url }, { text: divider }, { text: 'next' }], cols); + expect(terminalLogicalLine(buffer, 0, cols, 12)).toMatchObject({ startRow: 0, endRow: 0 }); + expect(terminalLogicalLine(buffer, 1, cols, 12)).toMatchObject({ startRow: 1 }); + } + }); + + // Real continuations are indented (codex cells, Claude's two-space paragraph + // indent); a column-0 row is refused, so these fixtures are indented. it.each(['my-post-title', 'user_id', 'x.html', 'my-post-title ', 'my-post-title Other column'])( 'keeps a weak-punctuation continuation that ends its cell: %s', (next) => { - const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: next }], 100); + const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + next }], 100); for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); } @@ -208,7 +268,10 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { it.each(['part%20two', 'x=1&y=2', 'path/slug', 'query?x=1'])( 'keeps stronger URL punctuation followed by prose: %s', (token) => { - const buffer = fakeBuffer([{ text: 'Docs: https://example.com/guide/' }, { text: token + ' more text' }], 100); + const buffer = fakeBuffer( + [{ text: 'Docs: https://example.com/guide/' }, { text: ' ' + token + ' more text' }], + 100 + ); expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); } ); @@ -339,6 +402,39 @@ describe('terminal link provider with wrapped table URLs', () => { expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); }); + it.each(NOT_CONTINUATION_ROWS)('keeps the URL and the second row separate: %s', (first, next, url, pathOnNext) => { + const { provide, openSpy } = harness([{ text: first }, { text: next }], 100); + const links = provide(1).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toEqual([url]); + links[0].activate(null, links[0].text); + expect(openSpy).toHaveBeenCalledExactlyOnceWith(url, '_blank', 'noopener,noreferrer'); + const second = provide(2); + expect(second.filter((link) => /^https?:/.test(link.text))).toEqual([]); + if (pathOnNext) expect(second.map((link) => link.text)).toContain(pathOnNext); + }); + + it.each(TRADE_OFF_ROWS)('pins the known trade-off link after %s', (first, next, joined) => { + const { provide } = harness([{ text: first }, { text: next }], 100); + for (const line of [1, 2]) { + const links = provide(line).filter((link) => /^https?:/.test(link.text)); + expect(links.map((link) => link.text)).toContain(joined); + } + }); + + it('keeps a full-width divider row out of a URL that reaches the right edge', () => { + const cols = 40; + const url = 'https://example.com/' + 'a'.repeat(cols - 'https://example.com/'.length); + for (const divider of ['─'.repeat(cols), '-'.repeat(cols)]) { + const { provide } = harness([{ text: url }, { text: divider }], cols); + expect( + provide(1) + .filter((link) => /^https?:/.test(link.text)) + .map((link) => link.text) + ).toEqual([url]); + expect(provide(2).filter((link) => /^https?:/.test(link.text))).toEqual([]); + } + }); + it('opens the whole URL from any of its rows', () => { const { provide, openSpy } = harness(TABLE, COLS); for (const line of [2, 3, 4]) {