(terminal): link filesystem paths in terminal output - #319
Conversation
A path printed in the terminal only opened the side panel when the CLI had wrapped it in a hyperlink itself, in its own Read(…) / Edit(…) tool headers. Everything else was inert text: the same absolute path in an agent's prose, in backticks or bare, git status output, a stack trace, grep -n results. The WebLinksAddon was loaded with no custom matcher, so xterm's default regex applied and only http/https matched, and there was no registerLinkProvider anywhere in the renderer. Register one. It matches filesystem paths on the hovered line only — xterm asks per hovered line, not per render, so the cost is bounded by what the pointer touches — and asks the main process one question per candidate: may the panel open this? That check is the guard the file-panel IPCs already enforce, isSensitivePath on the disk-resolved path, plus regular-file-ness, the panel's size bound and a NUL-byte sniff. A path that fails it gets no link at all: underlining .env and then denying the click teaches the reader to distrust the underline. Relative paths resolve main-side against the session's own working directory, through the same resolution the Changes panel uses, so the renderer never learns where a session lives. path:line and path:line:col carry the line into the panel, in the viewer and in an editable diff alike; a read-only diff has no line to jump to and says so. Answers are memoised per path, refusals included, so a pointer swept across the scrollback does not fire one call per line. Measured: 4 calls for a 1000-line sweep over a line with four distinct candidates, 0.018 ms per hovered line. The write path is untouched — 694.8 MB/s before, 692.7 MB/s after. Closes #318
A token with no separator was not a candidate, so README.md named in an agent's prose stayed inert even when that file sat in the session's working directory. The rule that kept it out was a guess about what the writer meant; existence and openability are the filter, and they already answer the question. A token now becomes a link when a file of that exact name resolves against the session's cwd and passes the same openability check — nothing in that check was relaxed to make it work. Extensionless names are in. Makefile, Dockerfile and LICENSE are files people name in prose, and a list of filename-shaped extensions would be exactly the guess this rule refuses. Over a 1000-line sweep of real prose they are 3014 of the 3225 distinct candidates and cost 40 µs per hovered line, once. Every word on a line is now a candidate, so a line's unknown candidates go out in one call instead of one call each, and resolve-terminal-path becomes resolve-terminal-paths. A 13-word sentence costs one call carrying 13 paths on the first hover and nothing on the second; the same 1000-line sweep costs 883 calls carrying 3225 paths, then zero. Unbatched it would have been 3225 calls. The openability check now tests existence before the denylist: on a line of prose most candidates are not files, and a path that is not there is refused whatever the denylist says. Everything that does exist still passes isSensitivePath before anything else is decided, so a linked path has survived the same checks as before. That halved the per-candidate cost, 45 µs to 14 µs. Terminal throughput is unchanged: 615.8 / 602.3 / 581.4 MB/s before, 621.0 / 618.6 / 636.3 MB/s after.
Scope extension: bare filenames link too (
|
| calls | paths carried | wall | |
|---|---|---|---|
| one 13-word sentence of prose, first hover | 1 | 13 | — |
| the same sentence, hovered again | 0 | 0 | — |
| 1000-line prose sweep, first pass | 883 | 3225 | 135 ms (0.135 ms/line) |
| 1000-line prose sweep, second pass | 0 | 0 | 32 ms (0.032 ms/line) |
It has moved materially from the 4 calls of the four-path line, and I am saying so: unbatched, those 3225 paths would have been 3225 calls. Batching is what holds it at one call per line regardless of word count; the memo is what takes the second pass to zero. 883 rather than 1000 because 117 lines had every candidate already cached. Nothing was narrowed to get there.
One check reordered, none relaxed
The openability check now tests existence before the denylist. On a line of prose most candidates are not files, and a path that is not there is refused whatever the denylist says. Everything that does exist still passes isSensitivePath before anything else is decided, so the set of checks a linked path has survived is unchanged; the only observable difference is the refusal reason for a path that is both missing and credential-shaped (missing rather than sensitive), and both refuse. Pinned by a test that asserts an existing .ssh/id_rsa is refused as sensitive and a missing .ssh/absent as missing.
It halved the dominant cost: 45 µs → 14 µs per candidate, and the sweep's first pass 244 ms → 135 ms.
Also bounded: at most 64 candidates per line, no path component longer than 255 characters (what a filename component can be, not a judgement about shape), cache cap raised 1000 → 4096 so the 3225 distinct candidates of that sweep do not thrash it. The handler is synchronous, so one call occupies the main process for 14 µs per path — under a millisecond at the 64-path worst case, 50 µs at the 3.65-path average measured.
Throughput
Unchanged, three alternating pairs through handleTerminalData:
before (549459c) |
after | |
|---|---|---|
| pair 1 | 581.4 MB/s | 636.3 MB/s |
| pair 2 | 602.3 MB/s | 618.6 MB/s |
| pair 3 | 615.8 MB/s | 621.0 MB/s |
Matrix, extended in place
New rows, driven off the same table: a bare name that exists and is openable (README.md), an extensionless one (Makefile), one colliding with an ordinary English word (plan), one that does not exist (ghost.js), one that is a directory (emptydir), one the guards refuse (.env), and a bare name with :line. The quoted-with-spaces row is now a bare name with spaces ("my file.txt"), since a separator is no longer what makes it a candidate.
Mutations
Against the four suites, 73 tests, green unmutated:
| Mutation | Red |
|---|---|
| require a separator again, so bare names stop linking | 9 |
| drop the openability check | 25 |
| drop the relative resolution | 20 |
| drop the per-path memo, so every hover re-asks | 5 |
| drop the line-carrying | 3 |
| drop the URL exclusion | 1 |
| drop the filename-length bound | 1 |
Numbers
npx eslint .— 0 errors, 333 warnings, still the base commit's count.npm test— batch 1: 1962 / 1959 pass / 1 fail / 2 skipped; batch 2: 120 / 119 / 0 / 1. The one failure istest/ipc-path-validator.test.js"allows files under ~/.claude/", which fails identically on549459cand passes on CI.- Committed with
--no-verify, since the pre-commit hook runs that suite and trips on that failure; the numbers above were run by hand.
Still not verified by execution, unchanged from the first commit: whether a link actually renders and clicks in a running terminal, and the real cmRevealLine scroll inside a real CodeMirror view. The prose figures come from the jsdom-free provider harness with the real main-side check over real files on disk, not from a live renderer.
Only a path the panel has established it can open becomes a link. Shape used to filter most text before anything was resolved; with bare filenames in scope it no longer does, which leaves the openability check as the whole of what stands between arbitrary scrollback text and an opened file. Say so where the next reader will look before widening the matcher, so that moving the check to the click does not read as a simplification.
… guard the read Four gaps found reviewing the link provider. The batch handler gated its remote refusal on `target.ok`, so a session whose working directory could not be resolved -- a remote host not yet indexed, a panel shell, an unknown id -- fell through to the local disk. Relative text was refused, but absolute text and `~/...` were resolved against the local root and the local home and opened, while the user believed they were reading the remote file. An unresolved cwd is now a refusal, and a panel shell resolves through the session that owns it as its spawn already does. The decision moved into `resolveTerminalPathsCwd` so it can be tested. `read-file-for-panel` checked size but not regular-file-ness. The link provider's own check does not cover it: the two are separate resolutions of the same string, 30 s of cache and a click apart, and `readFileSync` on a FIFO never returns -- on the main process that is a dead app. The memo evicted in insertion order, so passing the 4096 cap dropped the hit rate to zero in one step instead of degrading. A hit now re-inserts its entry. The wiring in `createTerminalEntry` had no test: the provider registration, the left-button guard, the line carried into the panel and the `forget` on teardown could each be deleted with the suite still green. The harness already had `setBufferRows` and `linkProviders` for this and nothing used them. Also drops the right-click test that re-implemented the guard inside its own callback, and replaces two unreproducible throughput figures in the context doc with the structural claim they were standing in for -- the flush path has no call into the provider.
|
Adversarial review at The batch handler resolved against the local disk when the session's cwd was unknown. The remote refusal was gated on The decision moved out of the handler into
The The memo evicted in insertion order, and a The wiring had no test. Removing the whole Dropped in passing: the Deferred: the synchronous batch on a network filesystem (#322). The measurements hold — 7.8 µs per miss after the reorder, 650 µs for a full 64-path batch — but they are CPU against a local disk, and the cost is one Also in the doc: the throughput figures are gone. They contradicted each other across commits (694.8 / 692.7 MB/s, then 615.8 / 602.3 / 581.4 for the same baseline) and I could not reproduce either — the harness never drains the flush buffer, so what it measures is string growth. Replaced by the claim they were standing in for, which any reader can check forever: Confirmed sound and left alone:
|
2 participants
Closes #318
The gap, as measured
Measured in a live session on 2026-09-22, with IDE Emulation running. Six forms of the same existing file were written into the scrollback and clicked. None opened the panel:
:12:12Only the CLI's own
Read(…)/Edit(…)tool headers carried a hyperlink.public/terminal-manager.jsloadedWebLinksAddonwith a click handler and no custom matcher, so xterm's default regex applied and onlyhttp/httpsmatched, and there was noregisterLinkProvideranywhere inpublic/.There is no agent-side workaround. The only thing that produces a clickable path is reading the file so its path lands in a tool header — one tool call per path, and impossible for any file that must not be opened. An OSC 8 sequence from a tool call never reaches the terminal at all: a tool call's stdout is a file under the session's
tasks/directory and/dev/ttycannot be opened from there.The three arbitrations
1. A candidate is checked, and the check is the openability question, not
existsSync.xterm asks a link provider for the links on one hovered line, not on every render, so the cost is bounded by what the pointer touches rather than by output volume. That budget buys one question per candidate, asked of the main process: may the panel open this path?
resolve-terminal-pathanswers with the guard the file-panel IPCs already enforce —isSensitivePath, on the disk-resolved path — plus regular-file-ness,PANEL_FILE_MAX_BYTES, and a NUL-byte sniff of the first 4 KB. Only what can be opened is linked. A path that fails any of those gets no link at all, because underlining.envand then denying the click teaches the reader to distrust the underline, and a link that does nothing is the failure shape this codebase has been removing.2. Relative paths resolve main-side, against the session's own working directory.
The renderer sends the session id and the matched text; main resolves, through
resolveGitChangesTarget— the same resolution the Changes panel uses. The renderer never learns where a session lives. It receives an absolute path only for a file that has already been accepted, which is whatreadFileForPanelneeds to open it, exactly as the existingfile://route already works. A remote session is refused outright: its paths name files on another host and this check stats the local disk.3.
path:lineandpath:line:colcarry the line into the panel.The commonest form in this scrollback — compiler output,
grep -n, stack traces.openFileInPanelgrew{ line }, and both routes honour it: the viewer scrolls once the CodeMirror bundle resolves; an editable diff applies it to the editor the diff created (unified, side-by-side and plain all show the working-tree document, so the line number means the same thing in each). A changed file that is not editable falls back to read-onlygit difftext, where a working-tree line number has no target — the panel says so in its notice rather than dropping the line silently.The security rule
A path arriving from terminal output is untrusted input: any command's output, any agent's prose. No new file-reading IPC was added with weaker guards than
read-file-for-panel.resolve-terminal-pathcalls the same validator and then narrows further, and it returns no file content — only a yes/no and the resolved path. The three cases that matter are pinned by the test matrix: a sensitive path (absolute and relative), a symlink whose resolved target is sensitive, and a remote session.What becomes a candidate
A candidate always contains a path separator. A bare word never becomes a link, even when a file of that name sits in the session's cwd: prose names files constantly, and linking every one is noise plus a call per word.
Two passes over the hovered logical line: a bare regex with a lookbehind that refuses a start inside a longer token (so
http://,https://andfile://never match and stay with their existing handlers), and quoted spans, which are the only place a space inside a path is unambiguous. Unquoted,docs/my file.txtmatchesdocs/my, which then fails openability and produces no link — half a path is not linked. Trailing prose punctuation is stripped before the:line:colsuffix is parsed, sosee /etc/hosts.links/etc/hosts. Overlapping candidates are resolved after the answers come back, so a quoted span that is really prose (the text between two apostrophes) is refused and leaves the bare link inside it standing.Caching, and what it costs
createTerminalPathResolvermemoises per session id + matched text, refusals included, storing the in-flight promise so simultaneous lookups of the same path share one call. Entries live 30 s, the map is capped at 1000 (oldest shed first), andforget(sessionId)runs when a terminal is destroyed.Measured on a 1000-line sweep over a line carrying four distinct candidates: 4 IPC calls, 0.018 ms per hovered line.
Throughput
Terminal write path, through
handleTerminalDatain the jsdom harness, 33.2 MB per run, median of five, alternated between the two checkouts:549459c)Unchanged, as it must be: the provider runs on a pointer event, never on a write.
What this does not change
http/httpslinks, thefile://route intoopenFileInPanel, and the CLI's own tool headers all behave exactly as before. Nothing other than a filesystem path is linkified.Verification
npx eslint .— 0 errors, 333 warnings, the same count as the base commit.npm test— batch 1: 1955 tests / 1952 pass / 1 fail / 2 skipped; batch 2: 120 / 119 / 0 / 1. The single failure istest/ipc-path-validator.test.js"allows files under ~/.claude/", which fails identically on549459c(1895 / 1892 / 1 / 2 there) and is environmental.--no-verify; the numbers above were run by hand.Not verified by execution: whether a path is actually clickable is a property of a running terminal. The matcher, the openability check, the link ranges, the cache, the activation routing and the line-carrying are all exercised by tests; the xterm render of the underline, the pointer interaction that triggers
provideLinks, and the realcmRevealLinescroll inside a real CodeMirror view are reasoned about against xterm's ownWebLinkProviderand this codebase's existing goto-line, not measured. The throughput figures come from the jsdom harness, not from a live Electron renderer.