fix(messages): fixed crash when clicking on a file url#515
fix(messages): fixed crash when clicking on a file url#515Samigos wants to merge 3 commits intoDimillian:mainfrom
Conversation
Samigos
changed the title
|
@codex review |
![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: fe1e95818c
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (isLikelyFileHref(url)) { | ||
| const directPath = getLinkablePath(url); | ||
| if (directPath) { | ||
| return directPath; | ||
| } |
There was a problem hiding this comment.
resolveHrefFilePath returns the raw href path as soon as it looks file-like, so links such as ./docs/My%20Guide.md are passed to onOpenFileLink with %20 still encoded. useFileLinkOpener then forwards that raw string to open_workspace_in, which means real files containing spaces (or other encoded characters) fail to open even though the link is valid markdown.
Useful? React with 👍 / 👎.
| if ( | ||
| trimmed.startsWith("~/") || | ||
| trimmed.startsWith("./") || | ||
| trimmed.startsWith("../") | ||
| ) { |
There was a problem hiding this comment.
This classifies every ./... and ../... markdown link as a file link, so non-file relative links (for example documentation routes like ./help/getting-started) are intercepted by handleFileLinkClick instead of behaving as normal anchors. That regresses existing markdown navigation behavior for relative links that are not local files.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f23ed161
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (trimmed.startsWith("/")) { | ||
| return hasLikelyLocalAbsolutePrefix(trimmed); | ||
| } |
There was a problem hiding this comment.
The new absolute-path branch only returns true for a small prefix allowlist, so links like /workspace/.../file.tsx:12 or /usr/... are not routed through onOpenFileLink even though they are valid file paths and getLinkablePath can parse them. In those environments the link still falls back to a normal anchor navigation, which means the crash path this change is fixing remains reproducible for common Linux/container paths.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f23ed161
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <a | ||
| href={href ?? toFileLink(hrefFilePath)} | ||
| onClick={(event) => handleFileLinkClick(event, hrefFilePath)} | ||
| onContextMenu={(event) => handleFileLinkContextMenu(event, hrefFilePath)} |
There was a problem hiding this comment.
The new hrefFilePath branch always binds onClick to handleFileLinkClick, which unconditionally calls event.preventDefault() before invoking optional onOpenFileLink. For Markdown call sites that do not pass onOpenFileLink (for example the PR summary/comment renderer in src/features/git/components/GitDiffViewerPullRequestSummary.tsx), file-like relative links such as docs/setup.md now become inert instead of behaving as normal anchors. This is a regression introduced by the new href-file routing path and should fall back to plain anchor behavior when no file opener is available.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c216c249ab
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (hasLikelyFileName(trimmed)) { | ||
| return true; | ||
| } | ||
| return hasLikelyLocalAbsolutePrefix(trimmed) && pathSegmentCount(trimmed) >= 3; |
There was a problem hiding this comment.
In isLikelyFileHref, absolute links under /workspace/ are treated as file paths once they have 3+ path segments, so a markdown link like /workspace/settings/profile is routed through onOpenFileLink instead of normal anchor navigation. In Messages, this means route-style links can be intercepted and fail with the file opener path, even though this change is intended to preserve non-file relative links as normal anchors.
Useful? React with 👍 / 👎.
2 participants
Issue
There is a crash when clicking on a file url in a message.
Summary
/help/getting-started)codex-file:links from navigating/crashing by safely swallowing clickValidation