upstream取り込み: v2 terminal modifier-keyed links + DnD (#3512 + #3542)#312
Merged
upstream取り込み: v2 terminal modifier-keyed links + DnD (#3512 + #3542)#312
Conversation
… reveal (superset-sh#3512) * feat(desktop): v2 terminal honors terminalLinkBehavior setting When the user's "Terminal file links" setting is "file-viewer", Cmd/Ctrl-clicking a file path in a v2 workspace terminal now opens the file in an in-app FilePane instead of the external editor — matching the v1 behavior the setting already controls. Directories and the "external-editor" setting continue to fall through to the existing openFileInEditor path. * feat(desktop): modifier-keyed v2 terminal file links + folder sidebar reveal Replaces the settings-based branch with a modifier-key pattern: - Cmd/Ctrl-click a file path → opens in an in-app FilePane. - Cmd/Ctrl+Shift-click a file or directory → opens in the external editor (with an upfront toast for remote workspaces, same pattern as FilesTab's Open in editor guard). - Cmd/Ctrl-click a directory path → force-opens the sidebar, reveals the folder in the file tree (ancestors expand, row scrolls into view and highlights). Implementation reuses the existing selectedFilePath → fileTree.reveal machinery in FilesTab by promoting selectedFilePath from a pane-store derivation to a useState, synced from the active file pane via useEffect. Folder focus is just a direct setSelectedFilePath — the existing sidebar code path handles reveal + scroll + highlight without changes. Folder paths now also flow through getParentForCreation, so the "New File" toolbar button creates inside the focused folder. Three callbacks (onOpenFile / onRevealPath / onOpenExternal) are plumbed from page.tsx through usePaneRegistry to TerminalPane. The shift-modifier path goes through openExternal, which checks workspaceHost.hostMachineId !== machineId and toasts for remote workspaces instead of firing a mutation the remote won't satisfy. v1 code untouched; DB schema untouched; v2 settings UI untouched (terminalLinkBehavior still honored by v1). * refactor(desktop): drop callback refs in v2 TerminalPane, use effect deps directly * refactor(desktop): move v2 pane actions into a PaneActionsProvider context Removes the terminal-specific callback plumbing from usePaneRegistry (which should only care about how to render each pane kind) and moves onOpenFile / onRevealPath / onOpenExternal into a React context scoped to the workspace page. TerminalPane consumes via usePaneActions() instead of taking them as props. * refactor(desktop): drop PaneActionsProvider, pass actions through usePaneRegistry The context indirection wasn't worth it for a single consumer (TerminalPane). Passing the three callbacks through usePaneRegistry options is simpler and has no actual downside since usePaneRegistry is only called from one place anyway. * fix(desktop): v2 terminal folder reveal switches sidebar to Files tab Previously revealing a directory only toggled rightSidebarOpen + setSelectedFilePath, but the sidebar would stay on whichever tab the user had last (e.g., Changes/Review), leaving FilesTab unmounted so the reveal effect never fired. Update the same v2WorkspaceLocalState transaction to also force sidebarState.activeTab back to "files". * fix(desktop): auto-expand revealed directory + extract useOpenInExternalEditor - useFileTree.reveal now also expands the target itself when it's a directory, using stateRef so there's no staleness concern. All reveal call sites benefit. - Extract useOpenInExternalEditor hook (remote check + toast + mutate) so TerminalPane can consume it directly instead of through a callback. Drops one prop from usePaneRegistry and removes the local workspaceHost liveQuery from page.tsx. FilesTab's handleOpenInEditor could migrate to the same hook in a follow-up to dedupe the pattern. * feat(desktop): v2 terminal URL links open in internal browser by default Cmd/Ctrl-click a URL in a v2 terminal now opens a BrowserPane in the current tab. Cmd/Ctrl+Shift-click still opens in the external browser. Widens TerminalLinkHandlers.onUrlClick to receive the MouseEvent (v1's helper just threads it through unused — behavior unchanged). * feat(desktop): v2 terminal shows hover tooltip describing cmd-click action Adds onLinkHover/onLinkLeave callbacks to TerminalLinkHandlers, wired through LinkDetectorAdapter, UrlLinkProvider, and WordLinkDetector so every detected link participates. In v2 TerminalPane, a new LinkHoverTooltip component tracks hover + live modifier state (global keydown/keyup listeners scoped to hover duration) and portals a positioned tooltip to document.body when meta/ctrl is held. Content flips on shift: - File: Open in editor | shift: Open externally - Folder: Reveal in sidebar | shift: Open externally - URL: Open in browser | shift: Open in external browser v1's helpers.ts doesn't opt into the new callbacks, so v1 hover behavior is unchanged. * refactor(desktop): match tooltip styling, surface configured editor name - Tooltip now uses the same bg-foreground/text-background/rounded-md/px-3/py-1.5/text-xs tokens as the project's TooltipContent, so it visually matches the rest of the app (was a custom border/popover style before). - Shift-modifier tooltip text now says "Open in Cursor" / "Open in VS Code" / etc. based on the user's configured defaultEditor setting, resolved via electronTrpcClient.settings.getDefaultEditor + getAppOption display label. Falls back to "Open externally" if no editor is configured. * refactor(desktop): split LinkHoverTooltip hook/component, tighten modifier listener - Move useLinkHoverState into its own hooks/ folder (was exported alongside the component in violation of the one-component-per-file rule). - Effect now re-subscribes on hover start/end only (deps: hovering boolean), not on every modifier change. - Filter to Meta/Control/Shift/Alt key events so typing a letter while hovering doesn't churn state. - Skip setState when modifier/shift values didn't actually change, avoiding identity-change re-renders on repeat keydowns. - Extract tooltip offset constant. * fix(desktop): block external open while host data loads, surface editor-query failures - useOpenInExternalEditor now treats an unloaded workspaceHost as non-local (workspaceHost?.hostMachineId !== machineId) so Cmd+Shift-click doesn't fire the mutation against a potentially remote workspace before locality is confirmed. - LinkHoverTooltip's getDefaultEditor catch now console.warn's the error before falling back to null, so settings RPC failures stay observable.
…rset-sh#3542) * fix(desktop): wire drag-and-drop for files in v2 terminal panes Drops from Finder or the file tree now focus the terminal, shell-escape the path, and paste it at the cursor. A dashed overlay indicates an active drag. * fix(desktop): handle multi-file terminal drops, align priority with v1 Check dataTransfer.files before text/plain (native Finder drags win over internal drags) and escape every dropped path, not just the first.
📝 WalkthroughWalkthroughターミナルリンクプロバイダーとマネージャーに、ホバー/リーブコールバック機能を追加し、ファイルオープン/パス表示の統合を実装します。リンク検出器、ホバー状態追跡、外部エディタ統合、ドラッグ&ドロップペースト機能も新規追加します。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts (1)
484-503:⚠️ Potential issue | 🟠 Major
revealのパス境界判定をディレクトリ境界ベースにしてください。Line 486 の
startsWith(rootPath)だと、rootPath=/work/appに対して/work/app-oldも通り、ターミナルリンク経由でルート外のexpand()/listDirectoryが走り得ます。既存のisWithinPathを使うと境界を正しく扱えます。修正案
- if (!rootPath || !absolutePath.startsWith(rootPath)) return; + if (!rootPath || !isWithinPath(rootPath, absolutePath)) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts` around lines 484 - 503, The reveal function currently uses startsWith(rootPath) to check path membership which allows paths like /work/app-old; change the membership checks to use the existing isWithinPath utility (e.g., replace startsWith(rootPath) with isWithinPath(rootPath, absolutePath)) and also use isWithinPath when iterating ancestors (replace the current.length/rootPath.length checks with isWithinPath(rootPath, current) or equivalent) so expand() / listDirectory() are only called for true descendants; keep references to reveal, rootPath, isWithinPath, expand and stateRef.current.entriesByPath when making the changes.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx (1)
39-57: 必要になるまで default editor の取得を遅延してもよさそうです。現状は tooltip が表示されない terminal pane の mount 時にも毎回
getDefaultEditor.query()が走ります。defaultEditorは Shift + file link のラベルでだけ必要なので、その条件に絞ると余分な tRPC 呼び出しを減らせます。♻️ 変更例
export function LinkHoverTooltip({ hoveredLink }: LinkHoverTooltipProps) { const [defaultEditor, setDefaultEditor] = useState<ExternalApp | null>(null); + const needsDefaultEditor = + hoveredLink?.modifier === true && + hoveredLink.shift && + hoveredLink.info.kind !== "url"; useEffect(() => { + if (!needsDefaultEditor) return; let cancelled = false; electronTrpcClient.settings.getDefaultEditor .query() @@ return () => { cancelled = true; }; - }, []); + }, [needsDefaultEditor]); if (!hoveredLink || !hoveredLink.modifier) return null;Also applies to: 59-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx around lines 39 - 57, 現在の useEffect 無条件でマウント時に electronTrpcClient.settings.getDefaultEditor.query() を実行しているため不要な tRPC 呼び出しが発生しています。修正は LinkHoverTooltip コンポーネント内の該当 useEffect を「tooltip が表示され、かつ Shift+file-link のラベルが必要なとき」にのみ実行するように条件化し（例：依存配列に tooltipVisible や isShiftFileLink フラグを追加してその変化でフェッチする）、fetch のキャンセルロジック（cancelled フラグや cleanup）を維持して setDefaultEditor の呼び出しを保護してください。同様の変更をもう一箇所の useEffect（該当する別の getDefaultEditor.query() 呼び出し、コメントの指摘箇所 59-61 相当）にも適用してください。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx:
- Around line 183-188: onUrlClick の外部ブラウザ起動失敗ログで full URL を出力しないように修正してください: 電子
RPC 呼び出しの catch 内 (electronTrpcClient.external.openUrl.mutate のエラーハンドラ) で error
はログに残して構いませんが、渡される url をそのまま出力しないでください。代わりに new URL(url).origin
を安全に取得する処理を入れ（無効な URL の可能性を try/catch で扱う）、ログは「[v2 Terminal] Failed to open URL
origin: <origin>」か「[v2 Terminal] Failed to open URL (origin
unknown)」のどちらかにし、クエリやパス等の機密部分は一切出力しないようにしてください (関数: onUrlClick、呼び出し:
electronTrpcClient.external.openUrl.mutate)。
---
Outside diff comments:
In `@apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.ts`:
- Around line 484-503: The reveal function currently uses startsWith(rootPath)
to check path membership which allows paths like /work/app-old; change the
membership checks to use the existing isWithinPath utility (e.g., replace
startsWith(rootPath) with isWithinPath(rootPath, absolutePath)) and also use
isWithinPath when iterating ancestors (replace the
current.length/rootPath.length checks with isWithinPath(rootPath, current) or
equivalent) so expand() / listDirectory() are only called for true descendants;
keep references to reveal, rootPath, isWithinPath, expand and
stateRef.current.entriesByPath when making the changes.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsx:
- Around line 39-57: 現在の useEffect 無条件でマウント時に
electronTrpcClient.settings.getDefaultEditor.query() を実行しているため不要な tRPC
呼び出しが発生しています。修正は LinkHoverTooltip コンポーネント内の該当 useEffect を「tooltip が表示され、かつ
Shift+file-link のラベルが必要なとき」にのみ実行するように条件化し（例：依存配列に tooltipVisible や
isShiftFileLink フラグを追加してその変化でフェッチする）、fetch のキャンセルロジック（cancelled フラグや
cleanup）を維持して setDefaultEditor の呼び出しを保護してください。同様の変更をもう一箇所の useEffect（該当する別の
getDefaultEditor.query() 呼び出し、コメントの指摘箇所 59-61 相当）にも適用してください。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f8b028b-3c82-4330-b5e3-fb590884161d
📒 Files selected for processing (18)
apps/desktop/src/renderer/hooks/host-service/useFileTree/useFileTree.tsapps/desktop/src/renderer/lib/terminal/links/link-detector-adapter.tsapps/desktop/src/renderer/lib/terminal/links/word-link-detector.tsapps/desktop/src/renderer/lib/terminal/terminal-link-manager.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useOpenInExternalEditor/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useOpenInExternalEditor/useOpenInExternalEditor.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/LinkHoverTooltip.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/components/LinkHoverTooltip/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkHoverState/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/hooks/useLinkHoverState/useLinkHoverState.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/TerminalPane/utils.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/multi-line-link-provider.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/link-providers/url-link-provider.ts
Comment on lines
+183
to
+188
| onUrlClick: (event, url) => { | ||
| if (event.shiftKey) { | ||
| electronTrpcClient.external.openUrl.mutate(url).catch((error) => { | ||
| console.error("[v2 Terminal] Failed to open URL:", url, error); | ||
| }); | ||
| return; |
There was a problem hiding this comment.
失敗ログに URL 全体を出さないでください。
url には query token、認証コード、個人情報が含まれ得るため、外部ブラウザ起動失敗時でも full URL を console.error に渡すのは避けたいです。origin だけ、または URL なしのログにしてください。
修正案
onUrlClick: (event, url) => {
if (event.shiftKey) {
electronTrpcClient.external.openUrl.mutate(url).catch((error) => {
- console.error("[v2 Terminal] Failed to open URL:", url, error);
+ let origin: string | undefined;
+ try {
+ origin = new URL(url).origin;
+ } catch {
+ origin = undefined;
+ }
+ console.error("[v2 Terminal] Failed to open URL:", {
+ origin,
+ error,
+ });
});
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onUrlClick: (event, url) => { | |
| if (event.shiftKey) { | |
| electronTrpcClient.external.openUrl.mutate(url).catch((error) => { | |
| console.error("[v2 Terminal] Failed to open URL:", url, error); | |
| }); | |
| return; | |
| onUrlClick: (event, url) => { | |
| if (event.shiftKey) { | |
| electronTrpcClient.external.openUrl.mutate(url).catch((error) => { | |
| let origin: string | undefined; | |
| try { | |
| origin = new URL(url).origin; | |
| } catch { | |
| origin = undefined; | |
| } | |
| console.error("[v2 Terminal] Failed to open URL:", { | |
| origin, | |
| error, | |
| }); | |
| }); | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx
around lines 183 - 188, onUrlClick の外部ブラウザ起動失敗ログで full URL を出力しないように修正してください: 電子
RPC 呼び出しの catch 内 (electronTrpcClient.external.openUrl.mutate のエラーハンドラ) で error
はログに残して構いませんが、渡される url をそのまま出力しないでください。代わりに new URL(url).origin
を安全に取得する処理を入れ（無効な URL の可能性を try/catch で扱う）、ログは「[v2 Terminal] Failed to open URL
origin: <origin>」か「[v2 Terminal] Failed to open URL (origin
unknown)」のどちらかにし、クエリやパス等の機密部分は一切出力しないようにしてください (関数: onUrlClick、呼び出し:
electronTrpcClient.external.openUrl.mutate)。
![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: 39b0d6bfac
ℹ️ 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".
Comment on lines
+177
to
+180
| if (link.isDirectory) { | ||
| onRevealPath(link.resolvedPath); | ||
| } else { | ||
| onOpenFile(link.resolvedPath); |
There was a problem hiding this comment.
This branch now relies on link.isDirectory to decide between onRevealPath and onOpenFile, but word-link hits are still synthesized as non-directories in the link manager. That means Cmd/Ctrl+click on bare directory names detected via the word provider (for example dot-directories shown by ls -a) is treated as a file and opened in a file pane instead of revealing the folder in the sidebar, which breaks the new directory-link behavior for a common terminal output pattern.
Useful? React with 👍 / 👎.
MocA-Love
added a commit
that referenced
this pull request
Apr 18, 2026
- 064db9f feat(desktop): modifier-keyed v2 terminal file links + folder sidebar reveal (superset-sh#3512) → PR #312 - 0b62387 fix(desktop): wire drag-and-drop for files in v2 terminal panes (superset-sh#3542) → PR #312
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
upstream から v2 terminal 系 2 commit を同梱で取り込む PR。behind 3 → 1。
取り込み commit
064db9f770b62387bafork 適応修正
page.tsx cherry-pick conflict 3箇所を手動マージ:
navigate/rightSidebarOpenViewWidth/electronTrpc.settings/showPresetsBarsetup を維持しつつ、upstream のconst collections = useCollections()を追加recordRecentlyViewedhelper を維持したまま、selectedFilePath→activeFilePanePath+useState+useEffectパターンに upstream rename を適用usePaneRegistryが options{ onOpenFile, onRevealPath }引数を取る形へonOpenFile 型吸収: fork の
openFilePane(filePath, displayName?)と upstream 契約(path, openInNewTab?)が第二引数の型で衝突。terminal Cmd+click は displayName を使わないのでuseCallback((filePath) => openFilePane(filePath))でラップし stable ref 化。TerminalPane.tsx BrowserPaneData mode: 064 は新規 BrowserPane を
{ url }だけで作るが、forkBrowserPaneDataはmode: \"docs\" | \"preview\" | \"generic\"が必須。terminal URL open にはmode: \"generic\"を付与。revealPath: upstream そのまま。sidebarState schema の
activeTab: \"changes\" | \"files\"とfileTree.reveal()経路は fork でも整合（Codex 確認済）。fork 独自領域 (非変更)
SpreadsheetViewer,GitHubSyncService,auto-updater,quit lifecycle,Better Auth,Electron IPC clipboard,shiki-theme.ts,fileDocumentStore,packages/panesはいずれも未タッチ。検証
Codex pre-review
Yes（全6項目）:
activeTab = \"files\"+selectedFilePath→fileTree.reveal()）テストチェックリスト
Summary by CodeRabbit
リリースノート