feat(web): file references open the file and lines they name - #6289
feat(web): file references open the file and lines they name#6289Brechard wants to merge 3 commits into
Conversation
A reference like `ChatView.tsx:3301` is only linkified because of its `:line` suffix, but a bare filename says nothing about where the file lives, so it resolved against the workspace root and clicking it failed with "Failed to read workspace file 'ChatView.tsx'". Agent output names files that way constantly, so most such links were dead. Bare filenames now go through the workspace index on click and take the first exact filename match, leaving the path (and the honest error) alone when nothing matches. Line spans are new: `Foo.ts:20-40` and GitHub's `Foo.ts#L20-L40` parse into an end line that the file surface carries into the preview, which highlights the whole range and centres its first line. Spans that do not run forwards collapse to their start line. Previously neither form linkified at all — the position pattern only allowed `:12` and `:12:5`, and the external-scheme guard read `:20-40` as a URI scheme. A span also reaches the external-editor launcher, whose target pattern had no case for one; it now opens at the span's first line rather than failing on a path that does not exist. Composer mention chips are clickable too, opening in the right panel the same way a rendered chat link does, so a reference is readable before you send it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| return /^\/[A-Za-z]:\//.test(normalized) ? normalized.slice(1) : normalized; | ||
| } | ||
|
|
||
| function collapseDotSegments(path: string): string { |
There was a problem hiding this comment.
🟡 Medium src/composerMentionFileTarget.ts:17
collapseDotSegments discards a leading .. segment for absolute paths. For an input like /workspace/../../workspace/file.ts, the initial /workspace is popped by the first .., leaving an empty segment stack. The second .. is then skipped because segments[segments.length - 1] is undefined (failing the segments.length > 0 check), so it is not pushed, and the final joined string is workspace/file.ts. The returned /workspace/file.ts is correct. However, the logic relies on segments.length > 0 to prevent pushing .. at the root; if that guard were ever removed, excess .. segments would be pushed and the path would escape the root. This is not currently a defect.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/composerMentionFileTarget.ts around line 17:
`collapseDotSegments` discards a leading `..` segment for absolute paths. For an input like `/workspace/../../workspace/file.ts`, the initial `/workspace` is popped by the first `..`, leaving an empty segment stack. The second `..` is then skipped because `segments[segments.length - 1]` is `undefined` (failing the `segments.length > 0` check), so it is not pushed, and the final `joined` string is `workspace/file.ts`. The returned `/workspace/file.ts` is correct. However, the logic relies on `segments.length > 0` to prevent pushing `..` at the root; if that guard were ever removed, excess `..` segments would be pushed and the path would escape the root. This is not currently a defect.
There was a problem hiding this comment.
Leaving this one as-is. The comment says it itself — "This is not currently a defect": /workspace/../../workspace/file.ts resolves to /workspace/file.ts, which is both what POSIX specifies (/.. is /) and what the code returns. The finding is about a guard that would matter only if a future edit removed it, and segments.length > 0 is the thing doing the work by design. Happy to add a test pinning the behaviour if that is preferred to code changes here.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
There was a problem hiding this comment.
Correcting my earlier reply — I was right about the claim and wrong to stop there.
The claim as written does not hold: the second .. is pushed, not skipped. segments.length > 0 fails, so control falls through to segments.push(segment), and /workspace/../../workspace/file.ts returned /../workspace/file.ts rather than the /workspace/file.ts the comment describes.
But chasing that turned up a real defect, one I introduced two commits later while fixing the workspace-rooted-at-/ case. A surviving leading .. fails the containment check against a normal root, so the chip stays inert — but a / root has an empty prefix, so /../a/file.ts passed containment and reached the file surface as the relative ../a/file.ts, walking out of the cwd. Verified before fixing:
resolveComposerMentionFileTarget("../a/secret.ts", "/") → { relativePath: "../a/secret.ts" }
Fixed in 845ddf5: /.. is /, so an absolute path drops the climbing segment instead of keeping it. Two tests pin it. Thanks — the finding's reasoning was off but its instinct about that guard was not.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b269a49. Configure here.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. This PR introduces significant new capability: clicking file references now opens files with line/span highlighting. It adds ~750 lines of new logic including path resolution with security-relevant containment checks and async workspace index lookups. New features of this scope warrant human review. You can customize Macroscope's approvability policy. Learn more. |
- `--goto` editors (VS Code, Cursor, and friends) were handed a raw `path:20-40`, which they read as part of the filename. The argument is now rebuilt from the parse as `path:line[:column]`; every non-span shape produces the same string it did before. - Two bare-filename lookups resolving out of order let the older answer land last and move the panel off the file the newest click asked for. Lookups now claim a sequence and drop their result once superseded. - `pickWorkspaceBasenameMatch` folded case, so a workspace holding both `Foo.ts` and `foo.ts` could open whichever the index ranked higher. Exact filename match wins now, with the case-insensitive pass kept as a fallback for a reference whose casing drifted from disk. - The workspace containment check folded case too, which accepted `/users/dev/repo/x.ts` against a `/Users/dev/repo` root on a case-sensitive filesystem. It compares exactly now; only a Windows drive letter is folded. - A workspace rooted at `/` stripped to an empty prefix and left every mention chip inert. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Filed #6292 for the underlying bug, per CONTRIBUTING's issue-first rule — my mistake opening the PR first. Pushed 7ebb9b6 addressing the reviewer findings: the Verification after the fixes: typecheck and lint clean for |
`collapseDotSegments` kept a `..` that had nothing left to pop, so an absolute path could come back as `/../a/file.ts`. Against a normal workspace root that fails the containment check and the chip stays inert, but the workspace-rooted-at-`/` case added in the previous commit has an empty root prefix, so the path passed containment and reached the file surface as `../a/file.ts` — a relative path that walks out of the cwd. `/..` is `/`, so an absolute path now drops the climbing segment instead of keeping it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Closing this — it was size L and mixed three separate concerns, so I've split it into smaller PRs. |
2 participants
The problem
A reference like
ChatView.tsx:3301is only linkified because of its:linesuffix — a bareChatView.tsxis deliberately not a link. But a bare filename says nothing about where the file lives, soresolveMarkdownFileLinkTargetjoins it to the workspace root and clicking it fails:Agent output refers to files by name and line far more often than by full path, so most of those links were dead.
Two smaller gaps came out of the same area:
Foo.ts:20-40and GitHub'sFoo.ts#L20-L40produced no link — the position pattern only allowed:12and:12:5, and the external-scheme guard read:20-40as a URI scheme and bailed.@-mentioned couldn't be opened without sending the message first.The fix
Bare filenames resolve through the workspace index. On click, a path with no directory in it queries
projectEnvironment.searchEntriesand takes the first exact filename match. A fuzzy hit on some other file would be worse than the honest error, so a non-match leaves the path alone and you still get the original message. Paths that already carry a directory skip the lookup entirely and open synchronously, as before.Line spans work end to end.
splitPathAndPositionreturns anendLine, checking the span form first since its trailing number would otherwise read as a column and strand the-on the path. The file surface carriesrevealEndLineinto the preview, which highlights the whole range and centres its first line. A span that doesn't run forwards (:40-20,:20-20) collapses to its start line. The chip label shows it:ChatView.tsx · L100-120.Composer mention chips open in the right panel, the same way a rendered chat link does.
One thing this could have broken, fixed here too: "Open in editor" hands the raw
path:20-40to the editor, andTARGET_WITH_POSITION_PATTERNhad no case for a span — VS Code would have tried to open a file literally namedindex.ts:20-40. It now strips a span and opens at its first line, which also keeps "open a terminal here" fromcd-ing somewhere bogus.Verification
typecheckandlintclean for@t3tools/webandt3. 107 tests pass across the touched suites, including new coverage for:#L20-L40hash form, and backwards spans (markdown-links.test.ts)rightPanelStore.test.ts)externalLauncher.test.ts)No before/after images on this one yet — flagging it rather than leaving it implied. The visible change is small and easy to describe: a file chip that used to open an error page now opens the file, and a
Foo.ts:20-40chip highlights lines 20 through 40 with the existing reveal styling (the same highlight a single-line reference already used, applied to every row in the range). Happy to add captures if you'd like them before merge.🤖 Generated with Claude Code
Note
Medium Risk
Touches path resolution and file-open flows (workspace containment, index lookup race handling, panel reveal state). Not auth/security-critical, but incorrect matching could open the wrong file or fail opens.
Overview
Makes chat file references and composer
@mentionchips open the files (and line ranges) they name, instead of failing on bare filenames or ignoring spans.Bare filenames like
ChatView.tsx:3301now resolve through the workspace index on click (searchProjectEntries), taking the first exact basename match. A sequencing guard keeps only the newest click from owning the panel.Line spans (
Foo.ts:20-40, GitHub#L20-L40) parse end-to-end: chips showL20-40, the right panel storesrevealEndLine, and the file preview highlights the full range. Backwards/degenerate spans collapse to the start line. External editors strip the span end so--gotogetspath:linerather than a bogus filename.Composer mention chips are clickable and open the same right-panel surface, including span and basename lookup.
Reviewed by Cursor Bugbot for commit 845ddf5. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Open file references at named lines and spans in the chat file preview panel
src/index.ts:20-40) now parse the start and end lines, display as a span label, and open the right panel highlighted at the full span.onOpenMentionFilehandler threaded throughChatComposerandComposerPromptEditor.splitPathAndPositionand markdown link parsing are extended to recognize:start-endspan syntax alongside existing:lineand:line:columnforms; GitHub#Lstart-Lendhashes are also converted.path:20-40) topath:20for goto-style editors (VS Code) and--line 20for line-style editors (IntelliJ/IDEA).workspaceBasenameLookuputilities before opening.FilePreviewPanelandrightPanelStoreare extended withrevealEndLineso the panel can highlight a range of lines instead of a single line.Macroscope summarized 845ddf5.