fix(vscode): make markdown file links in assistant output clickable#2204
fix(vscode): make markdown file links in assistant output clickable#2204B143KC47 wants to merge 4 commits into
Conversation
Markdown links pointing at local files rendered as plain anchors, which the webview never follows: react-markdown's sanitizer empties file:// and vscode://file/ hrefs, and the openFile bridge rejected absolute paths and :line suffixes anyway. Only http(s) links worked. Route path-like hrefs (plain paths, file:// and vscode://file/ URIs, with an optional trailing :line(:col) suffix) through the existing FileLink / openFile bridge instead of an anchor, preserving those schemes with a custom urlTransform. The openFile handler now accepts absolute paths that resolve inside the selected working directory (relativized before the existing containment check, so out-of-workspace paths still fail) and reveals the requested line via the editor selection. Bare absolute paths in plain text (outside a markdown link) are unchanged; text enrichment still only links workspace-relative paths. Fixes MoonshotAI#2194
…ransform to anchors A root-level relative path with a line suffix (README.md:5) was read as an unknown protocol by both the sanitizer pass-through and the file-link parser, leaving those links dead. Strip the :line suffix before the scheme test, preserve exactly what the parser accepts, and apply the pass-through only to anchor hrefs so image sources keep the default sanitizer behavior. A :0 suffix is kept as part of the path instead of producing a line reference the bridge validation would reject.
|
There was a problem hiding this comment.
Pull request overview
This PR fixes VS Code extension chat Markdown rendering so assistant-emitted links to local files (including file:// / vscode://file/ and :line(:col) suffixes) become actionable by routing them through the extension host openFile bridge instead of relying on webview anchor navigation.
Changes:
- Add
parseFileLink+fileAwareUrlTransformto classify/preserve file-ish hrefs while keeping the default sanitizer for non-anchor URLs. - Render Markdown
<a>elements asFileLinkbuttons when the href is a local file target, callingbridge.openFile(path, line?). - Extend the extension-host
openFilehandler to accept absolute paths (when inside the selected workdir) and optionally reveal a specific line; add unit tests for parsing + handler behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/vscode/webview-ui/tsconfig.json | Includes the new parsing unit test in the webview-ui TS project. |
| apps/vscode/webview-ui/src/services/bridge.ts | Extends openFile bridge API to accept an optional line. |
| apps/vscode/webview-ui/src/lib/link-utils.ts | Adds file-link parsing + a urlTransform to preserve file-ish hrefs for anchors. |
| apps/vscode/webview-ui/src/components/Markdown.tsx | Routes local-file markdown links through FileLink (bridge) and plugs in urlTransform. |
| apps/vscode/tsconfig.json | Excludes the new parsing test from the extension TS project to avoid project conflicts. |
| apps/vscode/test/workspace-paths.test.ts | Adds tests for absolute-path handling and line selection in openFile. |
| apps/vscode/test/file-link-parsing.test.ts | Adds unit tests covering link parsing variants and sanitizer behavior. |
| apps/vscode/src/handlers/file.handler.ts | Updates openFile to relativize absolute paths (when inside workdir) and support line reveal. |
| apps/vscode/shared/bridge.ts | Validates the new openFile line param on the RPC boundary. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (VSCODE_FILE_URI.test(raw)) { | ||
| raw = raw.replace(VSCODE_FILE_URI, ""); | ||
| } else if (EXTERNAL_SCHEME.test(raw)) { |
| function isAbsolutePathInput(input: string): boolean { | ||
| // Same detection as resolveWorkspacePath's rejection branch. | ||
| return path.posix.isAbsolute(input.replaceAll("\\", "/")) | ||
| || path.win32.isAbsolute(input) | ||
| || /^[A-Za-z]:/.test(input); | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d43b205de7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return path ? { path, line } : null; | ||
| } | ||
| } | ||
| return raw ? { path: raw } : null; |
There was a problem hiding this comment.
When a local Markdown link includes a fragment, such as [Usage](README.md#usage) or file:///workspace/README.md#L20, this returns the entire href as the filesystem path. The extension consequently stats a literal file named README.md#usage, so the link remains unopenable; separate the fragment before returning the path and optionally interpret supported line fragments.
Useful? React with 👍 / 👎.
| if (FILE_URI.test(raw)) { | ||
| // `file:///C:/x` keeps `/C:/x` after the scheme — drop the extra slash. | ||
| raw = raw.replace(FILE_URI, "").replace(/^\/(?=[A-Za-z]:[\\/])/, ""); |
There was a problem hiding this comment.
For an authority-bearing file URI such as file://server/share/project/file.ts (the standard form for a Windows UNC path), stripping only the scheme turns it into the relative path server/share/project/file.ts. In a workspace on that share, openFile then resolves this underneath the workspace directory rather than to the linked UNC file and the click fails; parse the URI authority and convert it to the corresponding UNC path.
Useful? React with 👍 / 👎.
Review follow-ups: normalize the extra leading slash in Windows-drive vscode://file/ URIs, map an authority-bearing file:// URI to its UNC path instead of a workspace-relative one, split #fragments off the path (reading GitHub-style #L20 as a line reference), and stop classifying drive-relative inputs as absolute so they keep being refused.
… integers Hardening from review: vscode://fileevil/... no longer classifies as a file link (lookahead keeps the strip prefix-only), and line references must be safe integers on both the parser and the bridge validator so an oversized :line suffix degrades to a plain path link instead of a rejected request.
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Related Issue
Resolve #2194
Problem
Markdown links in assistant output that point at local files are never clickable in the VS Code extension — only
https://links work. Three gaps stack up: react-markdown's URL sanitizer emptiesfile:///vscode://file/hrefs, the webview renders every link as a plain<a>(which the webview only follows for external schemes), and theopenFilebridge rejects absolute paths and:linesuffixes anyway.What changed
parseFileLink(webviewlib/link-utils.ts): classifies an href as a local file target (plain relative/absolute path — including root-level ones likeREADME.md:5—file://,vscode://file/, native Windows path, each with an optional trailing:line(:col)suffix) or an external link. A:0suffix is kept as part of the path rather than becoming an invalid line reference.arenderer routes file targets through the existingFileLink→bridge.openFilepath (external links keep the anchor), and a customurlTransformpreserves exactly the hrefsparseFileLinkaccepts — anchor hrefs only, so image/mediasrcattributes keep the default sanitizer behavior, andjavascript:URLs are still stripped.openFilegains an optional validatedlineparam, and the handler now accepts absolute paths by relativizing them against the working directory before the existing containment check. Out-of-workspace paths still returnok: false, and alineis revealed via thevscode.openselection.Review follow-ups included: Windows
vscode://file/C:/...URIs drop the bogus leading slash, authority-bearingfile://server/share/...URIs map to their UNC path (withlocalhostmeaning no authority), drive-relativeC:fooinputs stay refused instead of being classified absolute,#fragmentsare split off the path (GitHub-style#L20reads as a line reference), thevscode://fileauthority boundary is enforced (vscode://fileevil/...is not a file link), and line references must be safe integers on both the parser and the bridge validator so an oversized:linesuffix degrades to a plain path link instead of a rejected request.Scope note: bare absolute paths in plain text (outside a markdown link) remain non-clickable; text enrichment still only links workspace-relative paths. If that variant should also be linkified, it is better tracked as a separate enhancement.
Tests:
file-link-parsing.test.tscovers every href variant from the issue plus the Windows/UNC/fragment/authority/oversized-line cases and external-scheme passthrough;workspace-paths.test.tsgains absolute-inside (opens), absolute-outside (refused), drive-relative (refused), and line-selection cases. All fail without the fix. No changeset, matching previous extension-only fixes.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.