Skip to content

feat(terminal): link file paths that contain spaces as one path - #687

Merged
Ark0N merged 6 commits into
Ark0N:masterfrom
aakhter:pr/terminal-spaced-paths
Oct 27, 2026
Merged

Ark0N merged 6 commits into
Ark0N:masterfrom
aakhter:pr/terminal-spaced-paths

Conversation

@aakhter

@aakhter aakhter commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Problem

A file path that contains spaces (/mnt/c/OneDrive - Contoso/report.docx, Saved to: ~/My Docs/plan.md) was cut at the first space, in the terminal and the response viewer alike, so it produced two links to the wrong files.

Approach

findFilePathLinks() in constants.js takes a spaced path whole when the line bounds it: the whole line is the path, or the path follows a list marker and/or a short Label:. A relative spaced path needs the marker or the label. A spaced line that is one path but cannot be linked (an unknown root, a . or .. segment) links nothing rather than a fragment. Because this is the shared helper, the terminal link provider and the response viewer behave the same, and an absolute spaced path outside the workspace goes through the existing external-preview route like any other out-of-workspace path.

Tests

  • New test/spaced-file-path-links.test.ts: own-line, labelled and bulleted forms, refusal cases, no merging of two paths on one line, and a linear-time check.
  • npm run typecheck, lint, format:check, check:frontend-syntax, check:public-assets and npm run test:ci pass.
  • Checked in Chromium on an isolated instance: a shell printed Saved to: <dir>/My Big Docs/report.md and the link provider returned the whole spaced path.

Every path pattern ended a path at whitespace, so
"/mnt/c/OneDrive - Contoso/a.docx" linked "/mnt/c/OneDrive" plus a relative
"Contoso/a.docx", two links to the wrong files, in the terminal and the
response viewer alike.

findFilePathLinks() now takes a spaced path whole where the line bounds it:
the whole line is the path, or it follows a list marker and/or a short
"Label:". A relative spaced path needs the marker or label. The candidate
must read as one path (no earlier word ends on an extension, starts a new
"/" path or is an arrow). A spaced line that is one path but not linkable
(unknown root, "." or ".." segment) links nothing instead of a fragment.
@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thanks for this, aakhter. This PR makes findFilePathLinks() link a file path that contains spaces (/mnt/c/OneDrive - Contoso/report.docx, Saved to: ~/My Docs/plan.md) as one link instead of two wrong fragments, in the terminal and the response viewer alike. The OneDrive case is a real annoyance on WSL, and bounding the path by the line's shape (whole line, list marker, short label) is a sound way to do it. Static checks and CI are green.

Two things need to change before merge:

1. Prose after a list marker or label becomes part of the path (src/web/public/constants.js:2256-2261). spacedFilePathOnLine() rejects an earlier word only when it ends on an extension, ends in : or is an arrow, so plain words pass as path segments. Each of these lines links src/foo.ts (or test/foo.test.ts) on master; on this branch the whole span is one link to a file that does not exist:

line master this PR
- Updated src/foo.ts src/foo.ts Updated src/foo.ts
1. Added a test in test/foo.test.ts test/foo.test.ts Added a test in test/foo.test.ts
• Ran npm test -- test/foo.test.ts test/foo.test.ts Ran npm test -- test/foo.test.ts
Error: cannot open src/foo.ts src/foo.ts cannot open src/foo.ts
Note: see src/foo.ts src/foo.ts see src/foo.ts
- Moved ../a/b.ts ../a/b.ts Moved ../a/b.ts

In the terminal, - Updated src/foo.ts opens <workingDir>/Updated src/foo.ts. Bulleted summaries like these are what Claude Code and Codex print after most turns, so this breaks links that work today. It also makes the two surfaces disagree: the response viewer receives Updated src/foo.ts without the - (markdown consumed it) and still links src/foo.ts.

A rule that fixed every line above in a quick prototype, with all of your test cases still passing: after link is computed, defer to the ordinary detectors when the candidate is relative, linkable, and its first /-containing word is its last word.

const firstSlash = words.findIndex((word) => word.includes('/'));
if (link && firstSlash === words.length - 1 && !path.startsWith('/')) return null;

The trade-off: Saved to: 03 Attachments/plan.pptx (spaces only in the top directory) goes back to master's behaviour, while 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx, ~/My Docs/plan.md, notes/a b/c.md and every absolute case keep working. Two residuals it does not catch, worth pinning either way: - Fixed bug in src/My Docs/a.md and /home/me/proj contains a.md. Use this rule or a better one, and please add the lines in the table as regression tests in test/spaced-file-path-links.test.ts; the current "does not take prose into a path" test covers only the case without a marker.

2. .replace(/\s+$/, '') is quadratic on a long run of interior whitespace (src/web/public/constants.js:2251). A line of /home/a + 40,000 spaces + x takes 1.3 s on this branch and under 1 ms on master, and doubling the run quadruples it. The path.length > 1000 cap runs after the replace, so it does not bound it. The terminal is capped at 12 stitched rows, but the response viewer passes whole text nodes. text.slice(index).trimEnd() is equivalent and linear (1 ms for the same line). Please also add an interior-whitespace case to "stays linear on a long spaced line" (test/spaced-file-path-links.test.ts:99): 'a '.repeat(20000) uses single spaces and does not reach this path.

Smaller items:

  • The findFilePathLinks() JSDoc (constants.js:2157) still says rooted paths come first. Spaced paths now run first and can reserve a span that links nothing. Please update it, the "Path forms beyond the rooted one" paragraph in docs/architecture-invariants.md:410, and the findFilePathLinks() sentence in the root CLAUDE.md "File-path links (terminal + chat)" entry, so the next change to these detectors knows the reservation exists.
  • Optional, fine as a follow-up: Claude Code starts its own lines with ● and ⎿, which are not in the marker list, so ● Saved to /mnt/c/My Docs/a.docx still splits.

Once 1 and 2 are in with their tests, I'll take another look and merge.

@aakhter

aakhter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the requested changes in efa647d.

  • Applied the suggested conservative relative-path rule, with all six prose regressions and the top-directory-only space trade-off pinned in tests.
  • Replaced trailing-whitespace regex removal with trimEnd() and added 40,000- and 80,000-interior-space performance regressions.
  • Updated the detector JSDoc and both architecture docs to describe spaced-span reservation, including unlinkable candidates.

The two residual ambiguities (- Fixed bug in src/My Docs/a.md and /home/me/proj contains a.md) remain accepted as spaced paths, explicitly pinned and documented. Optional Claude markers remain a follow-up.

Validation: 65 targeted tests pass; typecheck, lint, frontend syntax and format checks pass. The branches combine without conflicts and pass all 121 combined link tests. Chromium also verified the prose case against shipped code. CI is green on this head, including the full unit/integration suite and xterm package tests.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thanks for the quick turnaround, aakhter. This PR makes findFilePathLinks() link a file path that contains spaces (/mnt/c/OneDrive - Contoso/report.docx, Saved to: ~/My Docs/plan.md) as one link in the terminal and the response viewer. The trimEnd() fix, the interior-space timing tests, the six prose regressions and the doc updates are all in, and CI is green on efa647d.

Point 1 from last round is still open, and that is on the rule I suggested, not on your implementation: it has holes I did not test for.

1. Prose with a / before the path is still taken into the link (src/web/public/constants.js:2261-2269). The deferral fires only when the first /-containing word is the last word, so any earlier word with a slash (45/45, read/write, CI/CD, I/O, src/, ../shared) defeats it, and a generic label (Error:, Tests:, Summary:) admits the same prose. Each of these links the path on master and the sentence on this branch:

line master this PR
1. Ran 45/45 tests in test/foo.test.ts test/foo.test.ts Ran 45/45 tests in test/foo.test.ts
- Fixed CI/CD pipeline in .github/workflows/ci.yml .github/workflows/ci.yml Fixed CI/CD pipeline in .github/workflows/ci.yml
- Added read/write support in config.ts config.ts Added read/write support in config.ts
- Tests: 45/45 pass in test/foo.test.ts test/foo.test.ts 45/45 pass in test/foo.test.ts
Error: I/O failure reading data.json data.json I/O failure reading data.json
- Moved src/ into lib/a.ts src/ and lib/a.ts Moved src/ into lib/a.ts
- Copied files from ../shared into lib/a.ts lib/a.ts Copied files from ../shared into lib/a.ts
- Ran 45/45 tests in ~/proj/foo.test.ts ~/proj/foo.test.ts Ran 45/45 tests in ~/proj/foo.test.ts

Test counts and CI/CD-style words are everyday content in the bulleted summaries Claude Code and Codex print, so this still breaks links that work today.

Part of this has an exact fix. A word after the first that starts with ~/, ./ or ../ opens a new path, the same as one starting with / (line 2261 checks only /, while PATH_TOKEN_STARTS_NEW_PATH already treats ~/ as a new path), and a word before the last that ends in / is a folder path of its own. That clears the last three rows.

The rest cannot be solved by word shape: Ran 45/45 tests in test/foo.test.ts and 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx look alike to any syntactic rule. So I would like the relative and ~/ forms to require a label that names a path, from a short allowlist (Path:, File:, Files:, Saved to:, Saved as:, Wrote:, Written to:, Output:, Created:, Location:, Attachment:), instead of any list marker or any one-to-four-word label. Absolute paths keep the own-line and list-marker forms, and a marker followed by an allowed label (- Path: ...) still works. I prototyped that plus the ~/ check against every line above: all of them go back to master's link, while Saved to: 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx, File: 03 Attachments/..., Saved to: ~/My Docs/plan.md, - /mnt/c/My Docs/a.docx and the OneDrive line still link whole. The cost is the bare-marker relative form (- notes/a b/c.md, - 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx), which goes back to master's behaviour, so those test cases flip. If you find a rule that keeps those and clears the table, use it instead. Either way, please add every line in the table to test/spaced-file-path-links.test.ts as a regression.

2. The response viewer never sees the marker or a bold label (src/web/public/app.js:3020, test/spaced-file-path-links.test.ts:63). Markdown consumes the bullet and a **Label:** before _linkifyFilePaths() gets the text, so the viewer still splits paths the terminal now links whole. Run through marked and the linkifier on this branch:

markdown viewer links
- 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx Design/Build-SIPOC.pptx
**Saved to:** 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx Design/Build-SIPOC.pptx
**Saved to:** ~/My Docs/plan.md ~/My and Docs/plan.md

Absolute paths work in all of these shapes. The findFilePathLinks() JSDoc (constants.js:2162) promises a path form is clickable in both surfaces or in neither, and the multi-line test is titled as covering the viewer, but a text node that carries \n- only occurs inside a code block. With the allowlist from point 1 the bullet row matches the terminal again. For the bold label, either pass the label context in from _linkifyFilePaths(), or state the gap in the JSDoc and in the architecture-invariants paragraph. Please pin whichever you choose with the jsdom linkify(html) helper already in test/response-viewer-file-links.test.ts, fed the HTML markdown produces (<li>03 Attachments/Build SIPOC Design/Build-SIPOC.pptx</li>, <p><strong>Saved to:</strong> 03 Attachments/...</p>), and retitle the multi-line test so it says it covers code blocks.

Smaller items, which I can also fold in at merge time if you prefer:

  • An unlinkable candidate costs a link master got right (constants.js:2187-2190 and 2264). /usr/bin/python3 script.py and /usr/bin/env node build.js (the shape make echoes) link script.py and build.js on master and nothing here, and /review the changes in src/foo.ts loses src/foo.ts. Refusing /etc/secret files/passwords.txt is the right call, but reserving an unlinkable candidate's span only when its last word contains a / keeps that case and frees the command-line shape.
  • Two consecutive spaces are a column gap, not part of a name (constants.js:2256): /home/me/proj modified notes.md now links as one path. Refusing a candidate with a double space inside is a one-line guard.
  • The new paragraph is pasted verbatim into CLAUDE.md:227 and docs/architecture-invariants.md:412. CLAUDE.md keeps the rules and points at architecture-invariants for the mechanism, and the in-line sentence you updated in its "File-path links" entry already carries the rule, so please drop the standalone CLAUDE.md copy and keep the paragraph in architecture-invariants.

Once 1 and 2 are in with their tests, I'll take another look and merge.

@aakhter

aakhter commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor Author

Addressed the two required items from this round in 74a107c.

  • Relative and ~/ spaced candidates now require one of the requested explicit path labels (case-insensitive), optionally after a marker. All eight slash-prose examples are regressions. Later /, ~/, ./, ../ words start separate paths, and an earlier word ending in / is treated as a separate folder. Bare-marker relative paths deliberately return to ordinary detection; allowed labels retain whole spaced paths.
  • The response viewer carries a leading <strong> path label in a paragraph/list item into detection, then maps indices back to the original text node. Labels, displayed text and markup stay intact. New jsdom cases feed the shipped marked renderer's HTML to linkify(html), covering consumed bullets, bold relative/home labels, absolute paths, generic-label prose and block boundaries. The multiline test now explicitly describes code-block text.
  • Updated the JSDoc and architecture paragraph, and removed the duplicate standalone CLAUDE.md paragraph.

Remaining limits are explicit: paths split across other inline elements are still detected per text node; the own-line absolute-path ambiguity remains. I left the two smaller detector changes (unlinkable-span reservation for command-like lines and double-space column gaps) for your offered merge-time handling. Optional Claude markers also remain a follow-up.

Validation: 109 targeted tests passed; typecheck, lint, frontend syntax and formatting checks passed. Chromium verified the actual rendered bold label and CI/CD prose case, including visible links and unchanged label text. The full local suite passed 12,341 tests (12 skipped). This branch combines cleanly with the submitted #686 fixes and all 165 combined link/viewer tests pass. CI on this exact head is green, including the full unit/integration suite and xterm package tests.

@Ark0N

Ark0N commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Thanks again, aakhter. This PR makes findFilePathLinks() link a file path that contains spaces as one link in the terminal and the response viewer. Both items from last round are in: relative and ~/ spaced paths now need a path label, every slash-prose line from the table is pinned, and the response viewer carries a leading bold label, with tests fed real marked output. Static checks pass, and so do the 1065 tests that load constants.js or app.js.

One thing needs to change before merge. It is the same class as last round, this time on the absolute side.

1. Prose after an absolute path is still taken into the link (src/web/public/constants.js:2256-2266). Two shapes the earlier tables did not cover.

(a) A path:line suffix, the shape grep -n, rg -n and compilers print. An indented line is read the same way, so grep output inside an agent's tool result is affected too:

line master this PR
/home/me/proj/README.md:12:See docs/setup.md /home/me/proj/README.md, docs/setup.md the whole line
/home/me/proj/CLAUDE.md:42:Deep detail lives in docs/architecture-invariants.md /home/me/proj/CLAUDE.md, docs/architecture-invariants.md the whole line
/home/me/proj/src/a.ts:42:5 - error in b.ts /home/me/proj/src/a.ts, b.ts the whole line
/home/me/proj/src/index.ts:10 updated config.json /home/me/proj/src/index.ts, config.json the whole line

A : followed by a digit inside an earlier word means the path already ended there. Adding || /:\d/.test(word) to the earlier-word check on line 2265 restores master on all four.

(b) An absolute path after a generic label. The prefix regex on line 2256 accepts any one-to-four-word label, and line 2260 checks the allowlist only for relative paths, so Error:, Warning: and Note: work like Path: for absolute ones. The JSDoc (line 2237) and the architecture-invariants paragraph say absolute paths keep only the own-line and marker forms:

line master this PR
Error: /home/me/proj contains a.md /home/me/proj, a.md /home/me/proj contains a.md
Warning: /home/me/proj is not tracked, see notes.md /home/me/proj, notes.md /home/me/proj is not tracked, see notes.md
Working directory: /home/me/proj, see README.md /home/me/proj, README.md /home/me/proj, see README.md
- Note: /tmp/out has the logs in run.log /tmp/out, run.log /tmp/out has the logs in run.log

Requiring the allowlist after any label, absolute or not, restores all four:

if (prefix[2] ? !SPACED_FILE_PATH_LABEL_PATTERN.test(prefix[2]) : !path.startsWith('/')) return null;

The cost: Saved the report to: /mnt/c/My Docs/a.docx goes back to master's split. The own-line /home/me/proj contains a.md you already pinned stays as it is and is not part of this.

I applied both guards locally: every row in both tables returns to master's links and the existing link tests still pass. Please add every line in both tables to test/spaced-file-path-links.test.ts as regressions.

Smaller items. I'll fold these in at merge time unless you'd rather take them:

  • The two from last round are still open, and I have tightened the rule I suggested for the first one. An unlinkable candidate still costs a link master got right: /usr/bin/python3 script.py, /usr/bin/env node build.js and /review the changes in src/foo.ts link nothing. Reserving only when the last word has a / frees the first two but not /review ..., whose last word is src/foo.ts. With a first-word check as well, all three come back and /etc/secret files/passwords.txt stays refused:
    if (path.startsWith('/') && !/^\/[^/]+\//.test(words[0])) return null; // after `const words`
    if (!link && !words[words.length - 1].includes('/')) return null; // after `const link`
    The column gap (/home/me/proj modified notes.md links as one path) is || path.includes(' ') on line 2262.
  • src/web/public/app.js:3027: chat renders with breaks: true, so a bold label that starts a later line of a paragraph (Done.\n**Saved to:** 03 Attachments/..., or a **Size:** 2 MB line above it) comes after a <br> and is not carried, and the path still splits. Also accepting beforeLabel.nodeName === 'BR' fixes both. The new test cases call marked.parse(markdown) with default options; { breaks: true, gfm: true } matches what _renderMarkdown passes.
  • src/web/public/constants.js:2270: a leading ./ on a labelled spaced path is read as a . segment, so Saved to: ./My Docs/plan.md links nothing while Saved to: ./notes.md links. Strip a leading ./ before the segment check.
  • src/web/public/app.js:3029 reads SPACED_FILE_PATH_LABEL_PATTERN outside the typeof findFilePathLinks guard on line 2982, whose comment says a stale constants.js must degrade rather than throw. A typeof check keeps that promise.
  • test/spaced-file-path-links.test.ts:70 is titled "links a spaced path after the list marker", but its first expectation now pins the fragment for a bare marker. A title that says a bare marker defers relative paths would match what it tests.

Once 1 is in with its tests, I'll take another look and merge.

A spaced path is no longer taken across a `path:line` suffix (grep -n, rg -n
and compiler output) and an absolute path after a generic label such as
`Error:` or `Note:` goes back to the ordinary detectors: any label must now
be a path label. `Saved the report to: /mnt/c/My Docs/a.docx` splits as on
master, pinned as a documented trade-off.

Also:
- a rooted candidate needs a directory before its first space, and an
  unlinkable one reserves its span only while its last word holds a `/`, so
  `/usr/bin/env node build.js` and `/review the changes in src/foo.ts` keep
  their links while `/etc/secret files/passwords.txt` still links nothing
- a run of spaces is a column gap, not part of a name
- a leading `./` is not read as a segment
- the response viewer carries a bold path label that starts a later line of
  a paragraph (after a `<br>`), and degrades if the label pattern is missing
- response viewer tests render with the options chat uses
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review. All of it is in, including the smaller items.

  1. Prose after an absolute path
  • (a) An earlier word holding a : followed by a digit now ends the path, so path:line output from grep, rg and compilers keeps master's links.
  • (b) Every label, absolute or relative, now has to match the path-label allowlist, using the check you suggested. Saved the report to: /mnt/c/My Docs/a.docx goes back to master's split, and a test pins that as a documented trade-off.
  • All eight lines from both tables are regressions in test/spaced-file-path-links.test.ts. They assert the full findFilePathLinks() output, index and folder flag included, and each one matches what master returns. I checked that against the merge-base constants.js.
  • Mutation check: with guard (a) removed, the 4 line-suffix regressions fail. With guard (b) removed, the 4 generic-label regressions and the trade-off test fail (5 in total).

Smaller items

  • Unlinkable candidates: both guards are in, the first-word directory check and the last-word / check. /usr/bin/python3 script.py, /usr/bin/env node build.js and /review the changes in src/foo.ts get master's links back, and /etc/secret files/passwords.txt still links nothing. All four are tested.
  • Column gap: a candidate that contains two spaces in a row is refused. /home/me/proj modified notes.md is pinned to master's links.
  • A leading ./ is removed before the segment check. Saved to: ./My Docs/plan.md now links whole, and Saved to: ./notes.md still links.
  • app.js: a bold path label is also carried when it comes after a <br>, with any whitespace text node in between skipped. Done.\n**Saved to:** ... and a **Size:** 2 MB line above it are both tested. Done.\nSee **Saved to:** ... is not carried. Removing the BR acceptance fails both new cases.
  • app.js: SPACED_FILE_PATH_LABEL_PATTERN is now read behind a typeof check. If constants.js is stale, the label is just not carried, and nothing throws.
  • The response-viewer tests now render with { breaks: true, gfm: true }, the same options _renderMarkdown uses.
  • I renamed the test at line 70 to say that a bare marker defers relative paths and that a path label after it links them.
  • The JSDoc on spacedFilePathOnLine() and the architecture-invariants paragraph now describe the new rules.

Verification: the two spaced-path test files pass (94 tests). I also ran every non-browser test file that loads constants.js or app.js through the CI config: 138 files, 2933 tests, all passing. lint, format:check, check:frontend-syntax, check:public-assets and typecheck are clean.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thanks for another thorough round, aakhter. This PR makes findFilePathLinks() link a file path that contains spaces as one link in the terminal and the response viewer. Everything from last round is in and tested: the path:line and generic-label guards, the command-line and column-gap guards, the leading ./, the <br> label carry and the typeof check. Typecheck, lint, format, frontend syntax and the link tests pass locally.

One thing needs to change before merge. It is the same class as the earlier rounds, this time behind the path labels.

1. Prose after an allowed path label is taken into the link (src/web/public/constants.js:2269-2289). The allowlist decides that a path follows the label, but nothing checks that the words before the first / word belong to it. Each of these links the path on master and the sentence on this branch, in the terminal and, with the label in bold, in the response viewer:

line master this PR
Wrote: 120 lines to src/foo.ts src/foo.ts 120 lines to src/foo.ts
Created: new file src/components/Button.tsx src/components/Button.tsx new file src/components/Button.tsx
Output: written to logs/run.log logs/run.log written to logs/run.log
Files: 3 changed in src/foo.ts src/foo.ts 3 changed in src/foo.ts
Location: line 42 in src/foo.ts src/foo.ts line 42 in src/foo.ts
Path: see src/foo.ts src/foo.ts see src/foo.ts
- Output: stored in dist/app.js dist/app.js stored in dist/app.js
1. File: changes in src/a.ts src/a.ts changes in src/a.ts
Created: 2026-10-24 build/x.json build/x.json 2026-10-24 build/x.json

The round-one guard caught all of these. In 74a107c it was removed when the allowlist came in, instead of staying next to it, and the Saved to: 03 Attachments/plan.pptx expectation changed from master's split (the trade-off accepted in round one) to the whole path in the same commit. Putting the guard back after const link (line 2286) restores master on all nine rows:

const firstSlash = words.findIndex((word) => word.includes('/'));
if (link && firstSlash === words.length - 1 && !path.startsWith('/')) return null;

I ran your test file against that change: the only expectation that fails is Saved to: 03 Attachments/plan.pptx at test/spaced-file-path-links.test.ts:124, which goes back to Attachments/plan.pptx as in round one. 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx, ~/My Docs/plan.md, ./My Docs/plan.md, notes/a b/c.md and every absolute case keep linking whole. The cost is a labelled relative path whose only space is in its top directory (Saved to: My Docs/plan.md), which splits as on master. Please:

  • restore the guard and change line 124 back to Attachments/plan.pptx;
  • add the nine rows above to test/spaced-file-path-links.test.ts as regressions;
  • pin the one residual the guard does not catch, Files: src/components and utils/x.ts (its first / word is not its last), as a documented trade-off, and mention it in the architecture-invariants paragraph.

Smaller items, which I can fold in at merge time unless you would rather take them:

  • src/web/public/constants.js:2276: a command line whose executable sits under a known root is still one link, because the candidate is linkable and the last-word rule never runs. /opt/homebrew/bin/node server.js, /home/me/.venv/bin/python run.py, /home/me/proj/node_modules/.bin/vitest run test/foo.test.ts and /home/me/proj/node_modules/.bin/tsc -p tsconfig.json link the whole line here and server.js, run.py, test/foo.test.ts, tsconfig.json on master. Refusing a candidate whose first word ends in bin/<name> or that contains a flag word restores master on all four and keeps the OneDrive, My Big Docs, screenshot and Docs - Copy lines whole (the bare - is not a flag); your tests still pass with it:
    if (/\/\.?bin\/[^/]+$/.test(words[0]) || words.some((word) => /^-{1,2}\w/.test(word))) return null;
  • src/web/public/constants.js:2235: the JSDoc example still says "after a short label (one to four words + :)"; only the path labels qualify now.
  • CLAUDE.md:225: the findFilePathLinks() sentence predates the last round (absolute paths after a label also need a path label now, and an unlinkable span is reserved only while its last word holds a /). A short pointer to the architecture-invariants paragraph is enough.

Once 1 is in with its tests, I'll take another look and merge.

A path label says a path follows, not that the words before it belong
to the path. Restore the guard that defers a relative candidate whose
only slash word is its last, so `Wrote: 120 lines to src/foo.ts` links
src/foo.ts as on master. `Saved to: My Docs/plan.md` and
`Saved to: 03 Attachments/plan.pptx` split again as a documented
trade-off; `Files: src/components and utils/x.ts` is pinned as the
residual the guard does not catch.

Refuse command lines whose executable sits under a bin/ or .bin/
directory, or that carry a flag word, so they keep master's links.

Update the JSDoc example, the CLAUDE.md pointer and the
architecture-invariants paragraph.
A side-by-side sweep of master and this branch over about 590 realistic lines
found more shapes where a spaced candidate swallowed prose or a command:

- An earlier word that is a lowercase connective or verb (and, or, to, in,
  the, is, has, contains, then, as and a few more) now makes the candidate a
  sentence, for own-line, list-marker and labelled forms alike:
  `/home/me/proj contains a.md`, `- /tmp/out has the logs in run.log`,
  `Path: /home/me/proj/out contains a.md`, `Files: src/components and
  utils/x.ts`, `Wrote: 3 files to docs/design notes/overview.md`. `at` and
  `of` stay out so screenshot names and `Copy of report.docx` still link
  whole; a name such as `Pros and Cons.md` splits as on master.
- A bare `=` word is refused like the arrows.
- An executable under `scripts/` or `tools/` is refused like one under
  `bin/` (`/home/me/proj/scripts/deploy prod.yml`).
- An earlier word holding an extension anywhere (`a.ts#L12`) ends the path.

The two residuals pinned earlier (`/home/me/proj contains a.md` and
`Files: src/components and utils/x.ts`) now return master's links.
@aakhter

aakhter commented Oct 25, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, you're right. The guard should have stayed next to the allowlist rather than being replaced by it. Pushed 5e1daaf and df9f93d.

Item 1:

  • I put the round-one guard back right after const link, so a relative candidate whose only / word is its last is left to the ordinary detectors.
  • Saved to: 03 Attachments/plan.pptx is back to master's Attachments/plan.pptx. Next to it I pinned Saved to: My Docs/plan.md -> Docs/plan.md as the documented trade-off.
  • All nine rows from your table are regressions now. Each one asserts the full findFilePathLinks() output against master's link and index. Three of them (**Wrote:**, - **Output:**, **Created:**) are also tested through the response viewer, with the label in bold, using marked output.
  • 03 Attachments/Build SIPOC Design/Build-SIPOC.pptx, ~/My Docs/plan.md, ./My Docs/plan.md, notes/a b/c.md and /mnt/c/My Docs/a.docx after Saved to: still link whole. That is pinned in one test.
  • Files: src/components and utils/x.ts no longer needs to be a residual. It returns master's utils/x.ts now (see below). The architecture-invariants paragraph describes the guard and the My Docs split.

Smaller items, all taken:

  • I added the bin/flag guard after the first-word guard. /opt/homebrew/bin/node server.js, /home/me/.venv/bin/python run.py, .../node_modules/.bin/vitest run test/foo.test.ts and .../node_modules/.bin/tsc -p tsconfig.json now return exactly what master returns. Note that master returns two links on each of these lines: a folder link on the executable path plus the file (for example /opt/homebrew/bin/node as a folder, then server.js). The tests pin both. A new test keeps a macOS screenshot name (Screenshot 2026-10-24 at 10.15.32 AM.png) and Docs - Copy whole, with and without a list marker. The existing OneDrive and My Big Docs tests still pass.
  • I fixed the JSDoc example: only path labels qualify. The comment also describes the prose guard and the command-line refusal.
  • I updated the findFilePathLinks() sentence in CLAUDE.md. Any label needs to be a path label, an unlinkable span is reserved only while its last word holds a /, and the sentence points to the architecture-invariants paragraph.

Found and fixed before you had to (df9f93d):

  • I ran master's findFilePathLinks() and this branch's side by side over 587 lines of Claude Code, Codex, git, grep/rg, compiler, stack-trace, shell and config output, plus 41 markdown samples through the response viewer with { breaks: true, gfm: true }. That turned up more shapes in the same class:
    • An earlier word that is a lowercase connective or verb (and, or, to, in, the, is, has, contains, then, as and a few others) now makes the candidate a sentence. This applies to own lines, list markers and labels alike: - /tmp/out has the logs in run.log, Path: /home/me/proj/out contains a.md, Wrote: 3 files to docs/design notes/overview.md, and also the two pinned residuals, /home/me/proj contains a.md (round one) and Files: src/components and utils/x.ts (round five). All of them return master's links. at and of are left out on purpose, so Screenshot ... at 10.00.00.png and Copy of report.docx still link whole. The cost is a name such as Pros and Cons.md, which splits as on master. That case is pinned as a trade-off.
    • A bare = word is refused the same way the arrows are. An executable under scripts/ or tools/ is refused the same way bin/ is (/home/me/proj/scripts/deploy prod.yml). An earlier word that holds an extension anywhere (foo.ts#L12 foo.ts) ends the path.
  • Each line is a regression test that asserts master's full output, and four of them also go through the response viewer.
  • What still differs from master is the intended joins (OneDrive, My Docs, Application Support, screenshots, Program Files (x86), YAML path: values and so on) plus the own-line absolute ambiguity you accepted earlier. That ambiguity is now pinned as /home/me/proj/out report.pdf, which reads the same as /home/me/docs/Read Me.txt.

Checks:

  • I removed each guard to confirm the tests catch it. Without the connective guard, 21 tests fail. Without =, 1 fails. Without the scripts//tools/ extension of the bin guard, 3 fail. Without the extension-anywhere check, 1 fails. Without the round-one prose guard, 4 still fail, and without the bin/flag guard, 7 fail.
  • The link test files pass, along with every non-browser test that loads constants.js or app.js (119 files, 2594 tests).
  • lint, format:check, check:frontend-syntax, check:public-assets and typecheck pass.

@Ark0N

Ark0N commented Oct 25, 2026

Copy link
Copy Markdown
Owner

Thanks again, aakhter. This PR makes findFilePathLinks() link a file path that contains spaces as one link in the terminal and the response viewer, and everything from the last round is in: the label-prose guard is back next to the allowlist, the nine rows are pinned, and the bin/flag, connective-word, =, scripts/ and tools/ guards all have tests. Typecheck, lint, format, frontend syntax, public assets and the link tests pass, and the branch merges cleanly onto current master.

I ran master's and this branch's detectors side by side over another batch of shell, test-runner and chat output. Three things came up. All are small, so I will apply them at merge rather than send you another round.

1. A shell prompt that shows an absolute working directory links the whole command line (src/web/public/constants.js:2290). On WSL the working directory is /mnt/c/..., outside $HOME, so the prompt prints it in full, and every earlier command in a shell session's scrollback reads as one spaced path:

line master this PR
/mnt/c/proj ❯ code notes.md /mnt/c/proj, notes.md the whole line
/home/me/proj $ cat notes.md /home/me/proj, notes.md the whole line
/home/me/proj$ cat notes.md /home/me/proj, notes.md the whole line
/home/me/proj ❯ git add src/a.ts /home/me/proj, src/a.ts the whole line
/mnt/c/Users/me/OneDrive - Contoso/proj ❯ code notes.md /mnt/c/Users/me/OneDrive, notes.md the whole line
/home/me/proj/out ✓ report.pdf /home/me/proj/out, report.pdf the whole line

A word with no letter or digit is never part of a file name, apart from a bare -, so I will refuse an earlier word like that, and a first word that ends in $:

if (words[0].endsWith('$')) return null;
// in the earlier-word loop:
if (word !== '-' && !/[\p{L}\p{N}]/u.test(word)) return null;

Your OneDrive, Program Files (x86), screenshot and Docs - Copy cases still link whole, and both of your test files pass with it. I will add the table above as regressions.

2. A leading ../ on a labelled spaced path links nothing (src/web/public/constants.js:2302). Saved to: ../My Docs/plan.md reserves its span and links nothing, while Saved to: ../notes.md links: the same gap the leading ./ had last round. Widening the strip to /^(?:~?\/|(?:\.\.?\/)+)/ fixes it, and an interior .. stays refused.

3. The response viewer reads a text node that starts mid-line as a line of its own (src/web/public/app.js:3034). Only an allowlisted bold label is carried, so **Note:** /home/me/proj/out report.pdf and Use `--config` /home/me/proj/cfg data.json link the whole span in the viewer while the terminal splits the same text. I will carry any leading bold label and let spacedFilePathOnLine() judge it, and give a text node that follows another inline element a placeholder word so it is not read as the start of a line. All 32 viewer tests still pass, and a path inside inline code still links whole.

Thank you for sticking with this through six rounds. The side-by-side corpus run in your last push was the right way to close out this class of bug. I will apply the three items above with their tests and merge.

@Ark0N
Ark0N merged commit df9f93d into Ark0N:master Oct 27, 2026
2 checks passed
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
- constants.js:2296,2301 spacedFilePathOnLine: refuse a first word ending in `$`
  and an earlier word with no letter or digit (other than a bare `-`), so a
  shell prompt showing an absolute working directory (`/mnt/c/proj ❯ code
  notes.md`, `/home/me/proj$ cat notes.md`) keeps master's links; the six rows
  from the review are pinned, plus OneDrive, `Program Files (x86)` and
  `Docs - Copy` still linking whole.
- constants.js:2312: the leading strip is now `/^(?:~?\/|(?:\.\.?\/)+)/`, so
  `Saved to: ../My Docs/plan.md` links whole; an interior `..` still links
  nothing. Tests for the detector and the terminal provider.
- app.js:3032 _linkifyFilePaths: carry any leading bold label and let
  spacedFilePathOnLine() judge it, and give a text node that follows another
  inline element a placeholder word, so `**Note:** /home/me/proj/out
  report.pdf` and ``Use `--config` /home/me/proj/cfg data.json`` split as in
  the terminal; a spaced path inside inline code still links whole. The
  now-unused label-pattern guard is gone.
- JSDoc (findFilePathLinks, SPACED_FILE_PATH_LABEL_PATTERN,
  spacedFilePathOnLine) and the spaced-path paragraph in
  docs/architecture-invariants.md updated to match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ark0N pushed a commit that referenced this pull request Oct 27, 2026
… spaces as one path

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Ark0N

Ark0N commented Oct 27, 2026

Copy link
Copy Markdown
Owner

Merged in 1.46.0 (https://github.com/Ark0N/Codeman/releases/tag/codeman%401.46.0). Thank you for sticking with this through six rounds. At merge I applied the three items: a prompt that prints an absolute working directory no longer links the whole command line (the six-row table is now regression tests), a labelled ../ spaced path links, and the response viewer carries any leading bold label and treats a text node after an inline element as mid-line. The release review added one case on top (3fdba31): a spaced working directory followed by a $ prompt (/mnt/c/Users/me/OneDrive - Contoso/proj$ code notes.md) now splits like master. One known limit stays: a spaced path that a CLI hard-wraps at one of its spaces is joined without that space, so it links whole but to a name without the space (master linked two fragments there). Breaks at a space and inside a word look the same on screen, so that one needs a design choice first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants