Repository navigation
fix(web): Open in editor on a chat file chip opens the file the chip names - #18389
juliusmarminge merged 6 commits into
Conversation
…names A click on a chip with a bare filename (`user.ts:261`) looks the name up in the workspace index, so it opens src/.../user.ts. Open in editor and the copy actions skipped that lookup and used `<workspace>/user.ts`, so VS Code opened an empty unsaved file at the root and the copied paths pointed there too. Open in editor, Reveal and both copy actions now resolve through one helper that asks the index first, the same lookup the click and Reveal already used. Fixes pingdotgg#14624. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a localized web bug fix that resolves existing chat file-chip actions through the workspace index while preserving fallback paths and source positions. It adds focused regression coverage without changing schemas, defaults, deployment, or security-sensitive code. You can add or adjust custom eligibility rules. Learn more. |
|
Note This comment is posted by Julius' dot The focused tests cover the resolved paths, but the changed menu actions have no before/after client evidence. Please add a short recording showing the bare-filename chip opening the existing nested file in the editor and both copy actions producing the correct paths, as required by the verification guide. A recording of the equivalent fix in your Windows fork is fine if you identify the build. |
|
Warning Review limit reachedOnly developers with an assigned seat can use this organization's usage-based review budget, and seats here are assigned manually. Ask an admin to assign a seat, or change the review continuation mode in Billing. Next included review available in 12 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
Issue #14624 requires the Windows desktop file context menu action Open in VS Code to open the nested file. The whole-PR diff changes only apps/web chat file-chip actions in ChatMarkdown.tsx a… |
Implement the workspace-path lookup in the desktop file context-menu flow used by #14624. Add an automated test that verifies a nested file opens in VS Code with its full path. Alternatively, remove the direct link if this PR intentionally … |
| Out of Scope Changes check | |
The changed behavior targets web chat file-chip actions: Open in editor, Reveal, and copy actions. Issue #14624 targets the apps/desktop file context menu. The diff provides no evidence that… |
Move the web chat-chip fix to a PR linked to the relevant web issue, or establish that the desktop file context menu uses this exact flow and add coverage for that flow. Keep the #14624 PR focused on the desktop context-menu behavior. |
-
Check name Status Explanation Title check The title clearly identifies the primary fix: opening the file named by a chat file chip in the editor. Description check The description clearly explains the problem, implementation, scope, verification results, screenshots, recording, and unverified environments. It does not use the template headings or include an expl… Full details: Description check
-
Explanation
The description clearly explains the problem, implementation, scope, verification results, screenshots, recording, and unverified environments. It does not use the template headings or include an explicit maintainer approval comment, but the required information is otherwise substantially complete.
-
Full details: Linked Issues check
-
Explanation
Issue
#14624requires the Windows desktop file context menu action Open in VS Code to open the nested file. The whole-PR diff changes onlyapps/webchat file-chip actions inChatMarkdown.tsxandmarkdownFileHostPath.ts. It does not change or test theapps/desktopcontext-menu flow. The web chip tests therefore do not establish compliance with the linked issue.Resolution
Implement the workspace-path lookup in the desktop file context-menu flow used by
#14624. Add an automated test that verifies a nested file opens in VS Code with its full path. Alternatively, remove the direct link if this PR intentionally fixes a separate web chat-chip issue.
Full details: Out of Scope Changes check
-
Explanation
The changed behavior targets web chat file-chip actions: Open in editor, Reveal, and copy actions. Issue
#14624targets theapps/desktopfile context menu. The diff provides no evidence that the desktop context menu usesChatMarkdown.tsx, so these changes are outside the linked issue's stated feature scope.Resolution
Move the web chat-chip fix to a PR linked to the relevant web issue, or establish that the desktop file context menu uses this exact flow and add coverage for that flow. Keep the
#14624PR focused on the desktop context-menu behavior.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
) into fork The branch behind pingdotgg#18389 (pingdotgg#14624: chip Open in editor and copy paths find a bare filename in the workspace). Fork main already does this, more broadly, through resolveChipHostPath from the File menus work (e3600f0, 146ecf8), so the fork's versions are kept: this records the branch as landed and changes no content. One known difference: the PR's copy actions keep the link's `:line` suffix, the fork's copy the plain path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Taking over this one. I will merge current main, simplify the path resolution and tests, and verify the editor and copy actions with before/after evidence. |
…en-resolves-path # Conflicts: # apps/web/src/components/ChatMarkdown.tsx
juliusmarminge
added
the
macroscope-review
Dismissing prior approval to re-evaluate 3296fb9
# Conflicts: # apps/web/src/components/ChatMarkdown.tsx
Dismissing prior approval to re-evaluate 2d3e3b9
A chat chip naming a nested file by basename opened the correct file in the preview, but Open in editor and both copy actions used a nonexistent file at the workspace root. Resolve these actions through the preview's existing workspace-index lookup, preserving line/column and the authored fallback when no match is found.
Clipboard writes begin with a pending payload before lookup completes, following the existing media-copy pattern to retain Safari user activation. Safari itself remains unverified.
Retains @chickenputty's original work in #18389, with main merged and the helper and callbacks simplified. The Electron desktop client loads this web renderer; native mobile has a separate renderer and is outside this change. Related: #14624.
Validation: 69 focused tests in four files before → 69 in three existing files after. Replaced the standalone suite with two path regressions; the indexed-path regression fails when lookup is disabled. The two relevant browser suites also pass, 75 before → 76 after, including a regression that starts the clipboard write before the delayed lookup resolves. Changed-file type-aware lint, formatting and web typecheck pass. The original 117-test count refers to an older test layout and was replaced with current results.
Headless Linux live verification used an isolated server and synthetic conversation fixture naming
HostResources.ts:10:2, which exists atsrc/resourceTelemetry/HostResources.ts. On main, the menu sent the workspace-root path. On this head, the actual editor RPC spawned acodeargv recorder with--goto <workspace>/src/resourceTelemetry/HostResources.ts:10:2. Both copy actions passed that nested path, retaining the workspace-labelled relative format and position. The file preview also opened the nested source. Clipboard writes were recorded at the browser API boundary; native clipboard integration and a real VS Code window were not verified. No provider was started. A local DejaVu font fixture made screenshots legible.Before — Copy full path points at the workspace root:
After — Copy full path names the existing nested file:
Menu actions recording
Not verified: macOS, Windows, iOS, Android, Electron desktop, a real desktop editor, native clipboard, remote relay/tunnel or a separate physical client. Existing environment-scoped RPC and permission paths are retained; no contracts or provider adapters change.
Implementation: Codex GPT-6.1 Sol through T3 Code; tests: Codex GPT-6 Astra; independent review: Claude Opus 5.5 and Codex GPT-6 Sol.