diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 6dc61ed7..2f68fad8 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. `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 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 b89bfdb7..b0bf0f2b 100644 --- a/src/web/public/constants.js +++ b/src/web/public/constants.js @@ -2569,7 +2569,8 @@ function findTerminalUrls(text) { * 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://`. + * and is not itself a new `scheme://`; 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. * @@ -2623,6 +2624,10 @@ function terminalUrlContinuationExtent(buffer, row, cols, maxRows) { break; if (/^[a-z][a-z0-9+.-]*:\/\//i.test(token) || !/^[A-Za-z0-9%_~]/.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. + const rest = nextText.slice(indent + token.length); + if (!/[%&=/?]/.test(token) && rest.trim() !== '' && !/^ {2,}/.test(rest)) break; const nextLogical = terminalLogicalLine(buffer, endRow + 1, cols, bound); if (!nextLogical) break; if (trailingColumns) firstRowUrlEnd = last.rawEnd; diff --git a/test/terminal-url-continuation.test.ts b/test/terminal-url-continuation.test.ts index 6d97ac2f..64b4868a 100644 --- a/test/terminal-url-continuation.test.ts +++ b/test/terminal-url-continuation.test.ts @@ -89,6 +89,19 @@ const TABLE_WITH_TRAILING_COLUMNS: Row[] = [ TABLE[4], ]; const TABLE_COLS = 220; +const PROSE_ROWS = [ + [ + '⏺ The guide lives at https://example.com/guide/', + ' self-hosted runners need a token first.', + 'https://example.com/guide/', + ], + ['Docs: https://example.com/guide/', 'Self-hosting is covered in chapter 3.', 'https://example.com/guide/'], + ['Open http://localhost:3000/', 'e.g. to check the dashboard', 'http://localhost:3000/'], + ['Endpoint https://api.example.com/v1/', 'user_id is required', 'https://api.example.com/v1/'], + ['Repo: https://github.com/org/repo/', 'well-known issues are tracked', 'https://github.com/org/repo/'], + ['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-'], +]; describe('findTerminalUrls (grammar)', () => { const { findTerminalUrls } = loadPure(); @@ -178,6 +191,28 @@ describe('terminalUrlContinuationExtent (wrap joining)', () => { for (const row of [0, 1]) expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toBeNull(); }); + it.each(PROSE_ROWS)('does not join 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(); + }); + + 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); + for (const row of [0, 1]) + expect(terminalUrlContinuationExtent(buffer, row, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + } + ); + + 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); + expect(terminalUrlContinuationExtent(buffer, 0, 100, 12)).toMatchObject({ startRow: 0, endRow: 1 }); + } + ); + it('does not glue a following prose line onto a URL that ends a line', () => { const buffer = fakeBuffer( [{ text: 'Download from https://example.com/files/' }, { text: 'then run the installer' }], @@ -295,6 +330,15 @@ describe('terminal link provider with wrapped table URLs', () => { return { provide, openSpy }; } + it.each(PROSE_ROWS)('preserves the original clickable URL before prose: %s', (first, next, url) => { + 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'); + 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]) {