chore(upstream): PR1 低リスク修正5件バンドル (#3457 #3464 #3462 #3466 #3461)#176
Merged
chore(upstream): PR1 低リスク修正5件バンドル (#3457 #3464 #3462 #3466 #3461)#176
Conversation
superset-sh#3457) Gate `onFileLinkClick` behind metaKey/ctrlKey in v1 helpers so file-path links once again require cmd/ctrl+click, matching pre-superset-sh#3382 behavior. The shared `LinkDetectorAdapter` (used by v2) still activates on plain click and is untouched.
…erset-sh#3464) The v2-workspace layout rendered child routes without WorkspaceTrpcProvider during the tick between workspaceId changing and the useLiveQuery join resolving. If the outgoing page hadn't finished unmounting, its hooks (TerminalPane, useGitChangeEvents, etc.) crashed with "useWorkspaceClient must be used within WorkspaceClientProvider". Hold the render until useLiveQuery reports isReady so the new workspace mounts into a valid provider on the same tick, and show an explicit "Workspace not found" state when the collection has hydrated but the id doesn't resolve.
…set-sh#3462) * fix(desktop): use native clipboard for copy path in v2 sidebar navigator.clipboard.writeText silently fails when focus is elsewhere (e.g., terminal), so the toast showed but nothing was actually copied. Switch to useCopyToClipboard which routes through Electron's native clipboard via tRPC, matching the v1 pathway. * feat(desktop): wire up Copy Path in v2 dashboard sidebar The workspace item context menu's Copy Path just toasted "coming soon". Fetch the worktreePath from the local host service for local workspaces, copy via electronTrpc.external.copyPath (native clipboard), and show a proper success toast. Non-local workspaces fall back to an explanatory error since the path only exists on the owning machine. * feat(desktop): wire up Open in Finder in v2 dashboard sidebar Share the local worktreePath resolver between Copy Path and Open in Finder, and route Open in Finder through electronTrpc.external.openInFinder instead of toasting "coming soon". * refactor(desktop): extract shared PathActionsMenuItems, wrap copy in try/catch Addresses review feedback on PR superset-sh#3462: - Pulled the Reveal in Finder / Copy Path / Copy Relative Path items shared between FileContextMenu and FolderContextMenu into a single PathActionsMenuItems component. - Wrap useCopyToClipboard calls in try/catch so IPC failures surface a proper error toast instead of failing silently. * refactor(desktop): hide Open in Finder & Copy Path for non-local workspaces - Gate the Open in Finder / Copy Path items in the dashboard sidebar workspace context menu behind isLocalWorkspace so they only appear for workspaces on the active machine. - Drop the now-redundant hostType guard from the actions hook. - Wrap openInFinder in try/catch in PathActionsMenuItems so native action failures surface a toast (addresses coderabbit/cubic P2 on superset-sh#3462).
Press Escape on any settings page to navigate back to where settings was opened from. Skips when focus is in a form field, contenteditable, or any open Radix layer (dialog, popover, dropdown).
…perset-sh#3461) * chore(desktop): auto-restart host-service on bundle change in dev Watches the built host-service.js in NODE_ENV=development and restarts running instances via the coordinator when electron-vite rewrites the bundle. Fast edit→reload loop for packages/host-service and src/main/host-service without restarting Electron. Not true HMR — in-memory host-service state (PTYs, watchers, chat streams) is torn down on each reload. * fix(desktop): wait for host-service bundle to stabilize before reload Rollup rewrites host-service.js via unlink+rewrite, so the fs.watch fires while the file is missing or partially written. The coordinator now polls statSync until size is non-zero and stable for 150ms (5s deadline) before calling restartAll, avoiding MODULE_NOT_FOUND on respawn.
Two adjustments on top of the cherry-picked upstream commits: 1. apps/desktop/src/main/index.ts — upstream superset-sh#3461 uses getHostServiceCoordinator() at the dev-reload call site, but fork imports that symbol as getHostServiceManager to minimize diff with the quit-lifecycle code that still uses the legacy name. Rewrite the call to use the local alias so the desktop main process still builds. 2. apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsx — upstream superset-sh#3466 uses [data-state="open"] to suppress Escape when a Radix overlay is open, but that selector also matches Radix Collapsible (settings/agents AgentCard uses Collapsible with data-state="open"), so expanding any card silently disabled the new Escape-to-close behavior. Narrow the selector to role-based overlay elements only (dialog, alertdialog, menu, listbox).
![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: 5100fa25b5
ℹ️ 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".
Address Codex review on #176: the role-based selector missed Radix Popover/HoverCard (e.g. FontFamilyCombobox, ProjectTargetingField), so Escape inside an open popover could navigate away from Settings instead of dismissing the popover. Radix Popper renders popover/hovercard content inside a wrapper with `data-radix-popper-content-wrapper`, which Collapsible (the original regression trigger) never uses. Add a descendant selector for that wrapper to cover popper-based overlays without re-catching Collapsible.
Owner
Author
|
22330a3 で対応しました。Radix Popover/HoverCard は [data-radix-popper-content-wrapper] 配下に描画され、その内部要素に data-state="open" が付与されるので、descendant selector を追加しました。Collapsible は popper を使わないので従来通り除外されます。 |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 51 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 Walkthroughウォークスルーデスクトップアプリのホストサービス開発リロード機能の追加、ローカルワークスペース用UIの条件付きレンダリング、ファイルシステム操作（Finderで表示、パスコピー）の新しい再利用可能コンポーネントへの統合、ワークスペースレイアウトの準備完了状態チェック、設定レイアウトのEscapeキーハンドラー追加、ターミナルファイルリンククリックの修飾キー要件化が含まれます。 変更内容
Sequence Diagram(s)sequenceDiagram
participant MainProcess as Main Process
participant HSC as HostServiceCoordinator
participant FSWatcher as File Watcher
participant Config as Config Provider
participant OrgInstances as Organization Instances
MainProcess->>HSC: enableDevReload(configProvider)
HSC->>FSWatcher: Initialize file watcher on host-service.js directory
Note over FSWatcher: Watch for file changes
FSWatcher->>FSWatcher: File changed detected
FSWatcher->>FSWatcher: Debounce: check file size stability
alt File stabilized
FSWatcher->>HSC: Trigger reload callback
HSC->>Config: Invoke configProvider()
Config-->>HSC: Returns SpawnConfig | null
alt Config available & Active instances exist
HSC->>OrgInstances: restartAll(config)
OrgInstances-->>HSC: Restart complete
Note over HSC: Reload succeeded
else No active instances
Note over HSC: Skip restart (no active instances)
end
else Reload already in progress
Note over HSC: Skip (reloading guard active)
end
MainProcess->>HSC: Cleanup function called
HSC->>FSWatcher: Close watcher
HSC->>HSC: Clear debounce timers
推定コードレビュー労力🎯 3 (Moderate) | ⏱️ ~25分 関連の可能性があるPR
ポエム
🚥 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 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
🧹 Nitpick comments (2)
apps/desktop/src/main/index.ts (1)
607-613:enableDevReloadの戻り値（クリーンアップ関数）が未使用です。
enableDevReloadはクリーンアップ関数を返しますが、現在は破棄されています。プロセス終了時にOSがリソースを解放するため実害はありませんが、明示的なクリーンアップがベストプラクティスです。♻️ クリーンアップ関数を保持する案
+let cleanupDevReload: (() => void) | null = null; + if (IS_DEV) { - getHostServiceManager().enableDevReload(async () => { + cleanupDevReload = getHostServiceManager().enableDevReload(async () => { const { token } = await loadToken(); if (!token) return null; return { authToken: token, cloudApiUrl: mainEnv.NEXT_PUBLIC_API_URL }; }); }そして
before-quitハンドラ内で呼び出す:// before-quit handler内 cleanupDevReload?.();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 607 - 613, enableDevReload's returned cleanup function is currently discarded; capture it and invoke it on app shutdown. Store the result of getHostServiceManager().enableDevReload(...) into a variable (e.g., cleanupDevReload) in the same scope where enableDevReload is called, and then call cleanupDevReload() inside the Electron 'before-quit' handler (or null-check and call) so the dev reload resources are explicitly released. Ensure the variable is accessible to the before-quit handler and handle the case where enableDevReload might return undefined.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems/PathActionsMenuItems.tsx (1)
43-45: UI 文字列「Reveal in Finder」はクロスプラットフォーム対応時の UX 問題です。Windows では「Show in Explorer」、Linux では「Show in File Manager」が標準的です。バックエンド で使用している
shell.showItemInFolder()は Electron の クロスプラットフォーム API であり、複数プラットフォーム対応を示唆しています。現在、同じハードコード文字列が複数ファイル（FileTreeItem、FileSearchResultItem、FolderRow、FileItem など）に存在します。アプリが複数プラットフォームに対応する場合、
process.platformに基づいて表示テキストを切り替えることを検討してください。🤖 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/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems/PathActionsMenuItems.tsx around lines 43 - 45, Replace the hardcoded "Reveal in Finder" label with a platform-aware string: create a single helper (e.g., getRevealInFileManagerLabel) that returns "Reveal in Finder" for darwin, "Show in Explorer" for win32, and "Show in File Manager" for linux/others, and use it in PathActionsMenuItems where ContextMenuItem currently displays "Reveal in Finder" and in other components noted (FileTreeItem, FileSearchResultItem, FolderRow, FileItem) so all usages are centralized; update ContextMenuItem to call getRevealInFileManagerLabel() for its child text and keep handleRevealInFinder as the action handler.
🤖 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/settings/layout.tsx`:
- Around line 156-157: Escape key handler currently calls navigate({ to:
originRoute }) which pushes a new history entry; change it to perform a replace
or history-back behavior so closing Settings does not add to history. Update the
handler around event.preventDefault() to call navigate with replace: true (e.g.,
navigate({ to: originRoute, replace: true })) or use a history-back equivalent
(navigate(-1)) so the close action does not create an extra entry; ensure the
referenced symbols are the existing navigate call and originRoute variable in
the same handler.
---
Nitpick comments:
In `@apps/desktop/src/main/index.ts`:
- Around line 607-613: enableDevReload's returned cleanup function is currently
discarded; capture it and invoke it on app shutdown. Store the result of
getHostServiceManager().enableDevReload(...) into a variable (e.g.,
cleanupDevReload) in the same scope where enableDevReload is called, and then
call cleanupDevReload() inside the Electron 'before-quit' handler (or null-check
and call) so the dev reload resources are explicitly released. Ensure the
variable is accessible to the before-quit handler and handle the case where
enableDevReload might return undefined.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems/PathActionsMenuItems.tsx:
- Around line 43-45: Replace the hardcoded "Reveal in Finder" label with a
platform-aware string: create a single helper (e.g.,
getRevealInFileManagerLabel) that returns "Reveal in Finder" for darwin, "Show
in Explorer" for win32, and "Show in File Manager" for linux/others, and use it
in PathActionsMenuItems where ContextMenuItem currently displays "Reveal in
Finder" and in other components noted (FileTreeItem, FileSearchResultItem,
FolderRow, FileItem) so all usages are centralized; update ContextMenuItem to
call getRevealInFileManagerLabel() for its child text and keep
handleRevealInFinder as the action handler.
🪄 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: 9642da77-d8d9-44de-81e6-13338d20df9a
📒 Files selected for processing (12)
apps/desktop/src/main/index.tsapps/desktop/src/main/lib/host-service-coordinator.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/DashboardSidebarWorkspaceItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/components/DashboardSidebarWorkspaceContextMenu/DashboardSidebarWorkspaceContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarWorkspaceItem/hooks/useDashboardSidebarWorkspaceItemActions/useDashboardSidebarWorkspaceItemActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FileContextMenu/FileContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/FolderContextMenu/FolderContextMenu.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems/PathActionsMenuItems.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/components/PathActionsMenuItems/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsxapps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
Address CodeRabbit review on #176: closing Settings via Escape with plain navigate() pushes a new history entry so the browser back button re-enters Settings instead of returning to whatever preceded it. Switch to navigate({ replace: true }) — the originRoute is already stored globally, so replacing /settings with it is the behavior users expect.
Owner
Author
|
e192adc で対応しました。replace: true に変更して履歴にSettingsエントリが残らないようにしました。 |
4 tasks
MocA-Love
added a commit
that referenced
this pull request
Apr 15, 2026
-s ours merge to record that upstream commits a3e34bf through de70163 (13 commits) are semantically already present on origin/main via the PR1-6 cherry-pick series (PRs #176, #177, #178, #179, #180, #182), plus fork-adaptation fixes layered on top. This merge target is de70163 specifically (not upstream/main) so newer upstream commits (9fff075 and later) remain visible in future behind counts. Upstream commits covered by this audit merge: - a3e34bf fix(desktop): restore cmd+click requirement for v1 terminal file links (superset-sh#3457) [PR1/#176] - 57557f8 fix(desktop): gate v2 workspace children on collection readiness (superset-sh#3464) [PR1/#176] - 4ee2e61 fix(desktop): use native clipboard for copy path in v2 sidebar (superset-sh#3462) [PR1/#176] - 87d6e93 feat(desktop): close settings with Escape key (superset-sh#3466) [PR1/#176] - 9c7f5f4 chore(desktop): auto-restart host-service on bundle change in dev (superset-sh#3461) [PR1/#176] - 93140d9 fix(mcp): accept MCP resource URL as valid OAuth audience (superset-sh#3459) [PR2/#177] - be9e000 fix(desktop): drive tray menu off events, fetch real org name (superset-sh#3458) [PR2/#177] - c5f791e feat(v2): unify workspace delete through host-service (superset-sh#3443) [PR3/#178] - 2c24d93 feat(desktop): paginated branch picker with checkout + open actions (superset-sh#3397) [PR4/#179] - 2bf1049 feat(desktop/hotkeys): v1 directional pane focus + best-effort v1 override migrator (superset-sh#3460) [PR5/#180] - 1294a7d feat(desktop/hotkeys): restore Cmd+Alt+Arrow for tab/workspace nav (superset-sh#3472) [PR5/#180] - de70163 feat(desktop): v2 review tab first pass — PR info, checks, comments (superset-sh#3463) [PR6/#182] Intentionally skipped (version bump, fork has independent versioning): - 1e23353 chore(desktop): bump version to 1.5.5 (superset-sh#3473) Fork-adaptation fixes layered on top of the cherry-picks: - PR1: host-service-coordinator alias import fix, settings Escape selector narrowing (role-based + popper wrapper), Escape close uses replace navigation - PR2: dual quit mode preservation (requestQuit "release"/"stop"), trayUpdateToken guard for stale async fetchHostInfo results - PR4: ChangesHeader.normalizeBranchName regex rewrite (lint false positive), worktree add uses fullRef for remote-tracking refs, syncTimedOut reset on pendingId change, GIT_REFS.md barrel example fix - PR5: migrate.ts re-sanitize of existing localStorage overrides (v2 marker bump intent), FOCUS_PANE_* enabled:isActive for KeepAliveWorkspaces, CATEGORY_ORDER merges Navigation (upstream) and Browser (fork) - PR6: normalizeThreadsToComments flattens all thread.comments (not just first), CommentPane overrides <a> (openUrl) and <img> (SafeImage), zero-badge suppression, pr-null comments gate Fork features verified intact (Explore agent audit of combined 36d4de4..35d95f3 range): - BROWSER_RELOAD / BROWSER_HARD_RELOAD hotkeys - dual quit mode menu in tray - v1 terminal cold-restore + retry reconnect (out of range but unaffected) - KeepAliveWorkspaces (FOCUS_PANE_* gated on isActive) - useCommandPalette + addMemoTab in v2 workspace - host-service-coordinator rename alias pattern
This was referenced Apr 18, 2026
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
3 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 superset-sh/superset から、低リスクな小修正5件をまとめて取り込みます。前回マージ (PR #159〜#163) 以降に追加された12コミットのうち、最もリスクの低いものを切り出した最初のPRです。
取り込む upstream コミット
a3e34bf0d57557f8064ee2e612e87d6e93d49c7f5f489フォーク適応修正 (追加1コミット)
Codex の事前レビューで 2 件の問題を検出したため、cherry-pick の上に
fix(fork): adapt upstream PR1 low-risk bundle for fork divergenceを積んでいます。1.
apps/desktop/src/main/index.tsupstream superset-sh#3461 は
getHostServiceCoordinator()を dev-reload 呼び出し箇所で直接使用しますが、フォークは同じシンボルを quit ライフサイクル互換のためgetHostServiceManagerとして alias import しています (apps/desktop/src/main/index.ts:41)。そのままではビルドが壊れるため、新規呼び出しも alias 名に書き換えました。2.
apps/desktop/src/renderer/routes/_authenticated/settings/layout.tsxupstream superset-sh#3466 は
[data-state=\"open\"]で Radix オーバーレイ検出をしていましたが、この selector は Radix Collapsible にもマッチします。Settings → Agents のAgentCardは Collapsible を使用しているため、カードを展開しただけで Escape による Settings クローズが silent に無効化される回帰がありました。selector を role ベースに狭めて修正:
[role=\"dialog\"][data-state=\"open\"](Dialog, Sheet)[role=\"alertdialog\"][data-state=\"open\"](AlertDialog)[role=\"menu\"][data-state=\"open\"](DropdownMenu, ContextMenu)[role=\"listbox\"][data-state=\"open\"](Select, Combobox)Tooltip は
data-state=\"delayed-open\"/\"instant-open\"を使うので Escape の対象外でも問題ありません。衝突・リグレッション調査結果 (Codex承認)
v1-terminal-cache.ts,useTerminalLifecycle.ts等) とは独立。isLocalWorkspace={hostType === \"local-device\"}ゲートはそのまま維持され、relay 導入後の local/remote 区別と整合。host-service-coordinatorrename と整合。v2Hostsjoin 前提と矛盾なし。テスト
bun install --frozen-lockfilebun run typecheckbun run lint次のステップ
このPRマージ後、PR2 (MCP OAuth + tray refactor) に進みます。
Summary by CodeRabbit
リリースノート
新機能
改善