Conversation
…-sh#3526, part 2) Complete the v2 file editor migration started in PR4. Wire the new fileDocumentStore + view registry into FilePane, usePaneRegistry, and page.tsx, then retire the old useFileDocument hook and the legacy FilePane renderers. Fork-specific features (SpreadsheetViewer, memo displayName, addMemoTab, SEARCH_IN_FILES / BROWSER_RELOAD hotkeys, right-sidebar width auto-adjust on file open) are preserved. Changes: * `v2-workspace/$workspaceId/page.tsx` — wrap with FileDocumentStoreProvider, switch onBeforeCloseTab to read dirty via getDocument, update useWorkspaceHotkeys() to pass paneRegistry (for onBeforeClose), relabel close-pane pane action with CLOSE_PANE. Fork features kept: openFilePane(filePath, displayName?) variant, openSidebarFilePane with rightSidebarOpenViewWidth auto-resize, addMemoTab, useCommandPalette hook, SEARCH_IN_FILES hotkey, BROWSER_RELOAD / BROWSER_HARD_RELOAD + browser-shortcut-event listener, presets bar toggle. * `v2-workspace/$workspaceId/types.ts` — drop `hasChanges`, keep fork's `displayName` and BrowserPaneData fields (`mode`, `reloadToken`, `hardReloadToken`). * `v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx` — new FilePaneTabTitle component reads dirty via useSharedFileDocument (with displayName fork fallback), renderHeaderExtras added, onBeforeClose rewritten to Save / Don't Save / Cancel flow using getDocument. * `FilePane/FilePane.tsx` — rewrite on top of useSharedFileDocument + view registry. Preserve fork SpreadsheetViewer fast-path (still bypasses the store). Memo displayName auto-derive effect kept in a fork-only useEffect that writes into FilePaneData.displayName. * `useWorkspaceHotkeys.ts` — CLOSE_TERMINAL → CLOSE_PANE with paneRegistry onBeforeClose await (upstream). * `useDefaultContextMenuActions.tsx` — close pane shortcut label now reads CLOSE_PANE. * `hotkeys/registry.ts` — SHOW_HOTKEYS moved off Cmd+/ onto Cmd+Shift+/ so CodeMirror can take Cmd+/ for the comment toggle. Fork-specific hotkeys (SEARCH_IN_FILES, BROWSER_RELOAD, BROWSER_HARD_RELOAD) kept. * `shared/themes/editor-theme.ts` — apply upstream's activeLine token change (accent at 0.5 alpha); KEEP fork's extended syntax tokens (docComment, controlKeyword, storageKeyword, escape, variableProperty, annotation, operator, punctuation, markdown*, meta) because fork-only shiki-theme.ts reads them. * `useCopyToClipboard.ts` / `PathActionsMenuItems.tsx` — switch to navigator.clipboard (upstream). * `.superset/lib/setup/steps.sh` — electric container restart policy: unless-stopped → on-failure:5 (upstream). * `WorkspaceFilePreviewContent.tsx` (fork-only FilesPane preview) — ported to useSharedFileDocument so it keeps working after the old useFileDocument is removed. * Deleted: `hooks/host-service/useFileDocument/**`, legacy `FilePane/components/ExternalChangeBar/**`, `FilePane/renderers/{CodeRenderer,ImageRenderer,MarkdownRenderer}/**`.
…ows basename - Spreadsheet files bypass the shared document store in FilePane but the tab title and header-extras still called useSharedFileDocument for them. Split FilePaneTabTitle into static vs with-document variants, and early-return FilePaneHeaderExtras before acquiring a document that would never be consumed. - getFileName() regressed to single-separator split when it took the upstream version; restore [/\\] so Windows paths are parsed the same way the classic v1 workspace has always done. Leaves the pre-existing BROWSER_RELOAD / BROWSER_HARD_RELOAD wiring gap alone: v2 page.tsx writes reloadToken onto BrowserPaneData but usePersistentWebview never reads it. That has been true on main since the fork added those hotkeys (commit 87d87d5) and is outside the scope of this PR.
…heet preview, fix remaining Windows paths
- useCopyToClipboard: revert to Electron IPC (electronTrpc.external.copyPath).
Upstream switched to navigator.clipboard for remote-host scenarios, but
that reintroduces the terminal/webview-focus copy failure this hook was
added to work around. Fork does not need the remote-host property here.
- WorkspaceFilePreviewContent: add a top-level fast-path so spreadsheets
go to SpreadsheetViewer before the shared store acquires the document.
The store treats non-image binary as text (isBinary flag only), so
xlsx etc. would otherwise fall into the raw-text preview.
- Replace split('/') → split(/[/\\]/) in the remaining three basename
sites (page.tsx onBeforeCloseTab dirty-name list, usePaneRegistry
onBeforeClose dialog title, FilePane conflict dialog title). Windows
full paths were leaking into warning copy otherwise.
📝 Walkthrough概要このプルリクエストは、ファイル文書の状態管理をカスタムフックベースのアプローチから共有ストアベースのアプローチに移行する大規模なリファクタリングです。レガシーレンダラコンポーネント（CodeRenderer、ImageRenderer、MarkdownRenderer、ExternalChangeBar）を削除し、FilePane UI、ホットキーレジストリ、ワークスペースのクローズ動作を更新して、新しい共有ドキュメント管理パターンに対応させています。 変更内容
シーケンス図sequenceDiagram
participant FilePane as FilePane Component
participant SharedDoc as useSharedFileDocument<br/>(Shared Store)
participant TrpcQuery as TRPC Query<br/>(Read/Save)
participant FsEvents as Filesystem<br/>Events
participant Store as FileDocumentStore
Note over FilePane,Store: 旧: useFileDocument (個別フック)
FilePane->>FilePane: レンダラ選択<br/>(CodeRenderer など)
FilePane->>FilePane: ローカルstate: dirty, content
Note over FilePane,Store: 新: useSharedFileDocument (共有ストア)
FilePane->>SharedDoc: document 取得
SharedDoc->>Store: 共有ドキュメント照会
Store-->>SharedDoc: document 返却
SharedDoc-->>FilePane: absolutePath, content,<br/>dirty, save, reload
FilePane->>FilePane: useEffect: filePath 同期
FilePane->>FilePane: useEffect: dirty時に自動ピン
FilePane->>SharedDoc: save(content)
SharedDoc->>TrpcQuery: 保存リクエスト
TrpcQuery-->>SharedDoc: 成功 or コンフリクト
alt コンフリクト検出
SharedDoc->>SharedDoc: conflict state セット
SharedDoc-->>FilePane: conflict 通知
FilePane->>FilePane: alert で解決アクション表示
FilePane->>SharedDoc: resolveConflict(action)
else 成功
SharedDoc->>Store: キャッシュ無効化
SharedDoc-->>FilePane: 完了
end
FsEvents->>SharedDoc: ファイル変更イベント
SharedDoc->>SharedDoc: external change マーク
SharedDoc-->>FilePane: 変更通知
FilePane->>FilePane: UI に反映
推定コード審査工数🎯 4 (複雑) | ⏱️ ~45 分 このプルリクエストは複数のシステムに跨る広範な変更を含んでいます。個別には多くの変更が単純な削除やリファクタリングですが、新しい共有ドキュメントストアパターンの導入、FilePane 内の複雑な useEffect ロジック、ワークスペース全体のホットキー・クローズ動作の更新、および複数のコンポーネント間の相互依存性により、理解と検証に相当な労力が必要です。 関連する可能性のあるプルリクエスト
詩
🚥 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 |
![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: 249d6deb59
ℹ️ 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
243
to
245
| return new Promise<boolean>((resolve) => { | ||
| alert({ | ||
| title: `Do you want to save the changes you made to ${name}?`, |
There was a problem hiding this comment.
onBeforeClose now returns a Promise<boolean> that is only resolved by action buttons, but this alert(...) call no longer provides onDismiss. In packages/ui/src/atoms/Alert/Alert.tsx, ESC/overlay dismissal triggers onDismiss only; without it, the promise can stay pending forever. In the close-pane hotkey path this leaves the await unresolved and can keep isClosingPaneRef stuck, so subsequent close hotkeys stop working until reload.
Useful? React with 👍 / 👎.
Comment on lines
598
to
600
| return new Promise<boolean>((resolve) => { | ||
| alert({ | ||
| title, |
There was a problem hiding this comment.
This tab-level onBeforeCloseTab promise also has no onDismiss handler on the alert, so pressing ESC or clicking outside the modal can leave the returned promise unresolved. Because tab closing awaits that promise, the close request can hang with no decision applied even though the dialog disappeared.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/hotkeys/registry.ts (1)
309-313:⚠️ Potential issue | 🟡 Minor
CLOSE_TERMINALとCLOSE_PANEが同じキーバインディングを使用しており、ショートカット衝突が発生します。両ホットキーが同一のキーバインディング（mac:
meta+w, windows/linux:ctrl+shift+w）を使用しているため、ショートカット一覧やキーバインディングの競合が発生します。CLOSE_TERMINALは ChatPane、BrowserPane、TabPane、DevToolsPane で現在も活発に使用されているため、単純な削除ではなく以下の対応が必要です：
- 異なるキーバインディングを割り当てるか、または
CLOSE_PANEに統一してすべての使用箇所を移行するかどちらのアプローチを採用するか明確にしてください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/hotkeys/registry.ts` around lines 309 - 313, CLOSE_TERMINAL and CLOSE_PANE share the same binding causing a conflict; decide and implement one of two fixes: (A) assign a distinct keybinding to CLOSE_TERMINAL (change its key object for mac/windows/linux) and keep existing usages in ChatPane, BrowserPane, TabPane, DevToolsPane, or (B) consolidate by removing CLOSE_TERMINAL, replace all references to CLOSE_TERMINAL in ChatPane, BrowserPane, TabPane, DevToolsPane with CLOSE_PANE and ensure CLOSE_PANE’s label/category reflect terminal semantics; update any docs/tests that reference the old symbol accordingly.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx (1)
44-195: 🛠️ Refactor suggestion | 🟠 Major
FilePaneContentは別ファイルに分離してください。このファイルは
components配下でFilePaneとFilePaneContentの 2 コンポーネントを持っています。FilePaneContent/FilePaneContent.tsxなどへ分けると、プロジェクトのコンポーネント配置ルールに合います。As per coding guidelines,
**/components/**/*.{ts,tsx}: No multi-component files - one component per file.🤖 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/FilePane/FilePane.tsx around lines 44 - 195, The file contains two components; extract the FilePaneContent component into its own file (e.g., components/FilePane/FilePaneContent.tsx) and export it so FilePane imports it; specifically move the function FilePaneContent (including its props type usage of FilePaneProps, and any helper imports it depends on like useSharedFileDocument, deriveMemoDisplayName, resolveActivePaneView, LoadingState/ErrorState/OrphanedBanner/SaveErrorBanner, alert) to the new module, update the original FilePane file to import FilePaneContent and keep only the FilePane component, and ensure all named/types (FilePaneProps, FilePaneData, PaneViewerData) remain exported/imported as needed so TypeScript builds.
🧹 Nitpick comments (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/FilePaneHeaderExtras/FilePaneHeaderExtras.tsx (1)
38-75:FilePaneHeaderExtrasInnerは別ファイルへ抽出してください。このファイル内に
FilePaneHeaderExtrasとFilePaneHeaderExtrasInnerの 2 component が同居しています。hook 分離自体は妥当なので、inner component だけFilePaneHeaderExtrasInner.tsxなどへ移すのが安全です。As per coding guidelines,
**/components/**/*.{ts,tsx}: No multi-component files - one component per file🤖 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/FilePane/components/FilePaneHeaderExtras/FilePaneHeaderExtras.tsx around lines 38 - 75, Move the inner component FilePaneHeaderExtrasInner into its own file (e.g., FilePaneHeaderExtrasInner.tsx): extract the function and its imports (useSharedFileDocument, resolveActivePaneView, orderForToggle, FileViewToggle) into the new file, export it as the default or named export, and update the original file to import that component and keep only the outer FilePaneHeaderExtras wrapper; ensure props types (FilePaneHeaderExtrasProps, FilePaneData) are imported or duplicated as necessary and that the handleChangeView logic and return behavior remain unchanged so consumers of FilePaneHeaderExtras continue to work without other code changes.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsx (1)
34-135:WorkspaceFilePreviewContentInnerは別ファイルへ分離してください。hook 呼び出しを分離する構造は良いですが、この component file に 2 component が定義されています。
WorkspaceFilePreviewContentInner.tsxへ抽出してください。As per coding guidelines,
**/components/**/*.{ts,tsx}: No multi-component files - one component per file🤖 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/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsx around lines 34 - 135, The file defines two components in one file — extract the WorkspaceFilePreviewContentInner function into its own file named WorkspaceFilePreviewContentInner.tsx: move the WorkspaceFilePreviewContentInner definition and any types it needs (e.g. WorkspaceFilePreviewContentProps) into the new file, keep necessary imports such as useSharedFileDocument, isSpreadsheetFile and SpreadsheetViewer, export the component (default or named) from WorkspaceFilePreviewContentInner.tsx, then replace the moved implementation in the original WorkspaceFilePreviewContent.tsx with an import of WorkspaceFilePreviewContentInner and render it (pass selectedFilePath and workspaceId through), ensuring reload/document typing and any helper functions remain imported where used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.superset/lib/setup/steps.sh:
- Line 262: The restart policy for the Electric container was changed to
"on-failure:5" which prevents automatic recovery after Docker/daemon restarts;
revert the restart flag in the setup script (the line emitting "--restart
on-failure:5 \") back to a persistent policy such as "--restart unless-stopped
\", or alternatively add explicit restart logic in the development startup flow
that ensures Electric is started after daemon/reboot (update the invocation in
.superset/lib/setup/steps.sh where the restart option is emitted).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx:
- Around line 88-112: The effect in FilePane is re-running when the document
object reference changes causing duplicate alerts; add a useRef (e.g.,
lastPromptedConflictRef) to track the currently-prompted conflict and only call
alert() when document.conflict exists and lastPromptedConflictRef.current !==
document.conflict (or its unique id), then set the ref to that conflict after
showing the prompt; also reset lastPromptedConflictRef.current to null when
conflict becomes null (or when resolveConflict is invoked) so future conflicts
can prompt again; keep the effect dependencies to [hasConflict, filePath]
(remove document) and reference document.resolveConflict directly inside the
handlers.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 249-267: The onClick handlers inside the onBeforeClose flow call
getDocument(...).save() and .reload() without error handling, so if those
promises reject the onBeforeClose Promise never resolves; wrap the await calls
in try/catch (in the Save and Don't Save handlers referenced by label "Save" and
label "Don't Save") and in the catch always call resolve(false) (and optionally
log the error) so the close guard is released and close is aborted on failure;
ensure every code path in those onClick handlers calls resolve(...) exactly
once.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.ts:
- Around line 75-92: The close handler may get an unhandled rejection if
definition.onBeforeClose throws; update the useHotkey("CLOSE_PANE") callback
(the isClosingPaneRef guarded block) to catch errors from
definition.onBeforeClose: call it inside a try/catch (or await with .catch) and
treat any thrown/rejected result the same as a false/denied value (i.e., safely
return without closing the pane), ensuring isClosingPaneRef is still reset in
the existing finally block; reference: isClosingPaneRef, useHotkey callback,
paneRegistry, definition.onBeforeClose, and state.closePane.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 584-632: The onBeforeCloseTab handler (the onBeforeCloseTab
callback where dirtyPanes are iterated) can hang if doc.save() or doc.reload()
rejects; wrap each await doc.save() and await doc.reload() in try/catch, and on
any caught error call resolve(false) (to prevent the Promise from never
resolving) and surface a user notification via the existing alert mechanism (or
an error alert) so the user is informed; ensure you reference the same
dirtyPanes loop and getDocument(workspaceId, filePath) lookups, and do not let
exceptions propagate out of the onBeforeCloseTab Promise.
---
Outside diff comments:
In `@apps/desktop/src/renderer/hotkeys/registry.ts`:
- Around line 309-313: CLOSE_TERMINAL and CLOSE_PANE share the same binding
causing a conflict; decide and implement one of two fixes: (A) assign a distinct
keybinding to CLOSE_TERMINAL (change its key object for mac/windows/linux) and
keep existing usages in ChatPane, BrowserPane, TabPane, DevToolsPane, or (B)
consolidate by removing CLOSE_TERMINAL, replace all references to CLOSE_TERMINAL
in ChatPane, BrowserPane, TabPane, DevToolsPane with CLOSE_PANE and ensure
CLOSE_PANE’s label/category reflect terminal semantics; update any docs/tests
that reference the old symbol accordingly.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx:
- Around line 44-195: The file contains two components; extract the
FilePaneContent component into its own file (e.g.,
components/FilePane/FilePaneContent.tsx) and export it so FilePane imports it;
specifically move the function FilePaneContent (including its props type usage
of FilePaneProps, and any helper imports it depends on like
useSharedFileDocument, deriveMemoDisplayName, resolveActivePaneView,
LoadingState/ErrorState/OrphanedBanner/SaveErrorBanner, alert) to the new
module, update the original FilePane file to import FilePaneContent and keep
only the FilePane component, and ensure all named/types (FilePaneProps,
FilePaneData, PaneViewerData) remain exported/imported as needed so TypeScript
builds.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/components/FilePaneHeaderExtras/FilePaneHeaderExtras.tsx:
- Around line 38-75: Move the inner component FilePaneHeaderExtrasInner into its
own file (e.g., FilePaneHeaderExtrasInner.tsx): extract the function and its
imports (useSharedFileDocument, resolveActivePaneView, orderForToggle,
FileViewToggle) into the new file, export it as the default or named export, and
update the original file to import that component and keep only the outer
FilePaneHeaderExtras wrapper; ensure props types (FilePaneHeaderExtrasProps,
FilePaneData) are imported or duplicated as necessary and that the
handleChangeView logic and return behavior remain unchanged so consumers of
FilePaneHeaderExtras continue to work without other code changes.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsx:
- Around line 34-135: The file defines two components in one file — extract the
WorkspaceFilePreviewContentInner function into its own file named
WorkspaceFilePreviewContentInner.tsx: move the WorkspaceFilePreviewContentInner
definition and any types it needs (e.g. WorkspaceFilePreviewContentProps) into
the new file, keep necessary imports such as useSharedFileDocument,
isSpreadsheetFile and SpreadsheetViewer, export the component (default or named)
from WorkspaceFilePreviewContentInner.tsx, then replace the moved implementation
in the original WorkspaceFilePreviewContent.tsx with an import of
WorkspaceFilePreviewContentInner and render it (pass selectedFilePath and
workspaceId through), ensuring reload/document typing and any helper functions
remain imported where used.
🪄 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: 413c8987-06bd-44ff-80fc-fa46df432515
📒 Files selected for processing (22)
.superset/lib/setup/steps.shapps/desktop/src/renderer/hooks/host-service/useFileDocument/index.tsapps/desktop/src/renderer/hooks/host-service/useFileDocument/useFileDocument.tsapps/desktop/src/renderer/hotkeys/registry.tsapps/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/hooks/useDefaultContextMenuActions/useDefaultContextMenuActions.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/FilePaneHeaderExtras/FilePaneHeaderExtras.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useWorkspaceHotkeys/useWorkspaceHotkeys.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types.tsapps/desktop/src/shared/themes/editor-theme.ts
💤 Files with no reviewable changes (10)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx
- apps/desktop/src/renderer/hooks/host-service/useFileDocument/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx
- apps/desktop/src/renderer/hooks/host-service/useFileDocument/useFileDocument.ts
| if ! docker run -d \ | ||
| --name "$ELECTRIC_CONTAINER" \ | ||
| --restart unless-stopped \ | ||
| --restart on-failure:5 \ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Docker restart policy on-failure vs unless-stopped behavior after Docker daemon restart official documentation
💡 Result:
After a Docker daemon restart: - on-failure: Does not restart the container. It only restarts if the container exits due to an error (non-zero exit code) while running. Daemon restart does not trigger a restart under this policy. - unless-stopped: Restarts the container if it was running at the time of daemon restart (unless it was explicitly stopped beforehand, in which case it remains stopped). Official Docker documentation explicitly states for on-failure: "The on-failure policy only prompts a restart if the container exits with a failure. It doesn't restart the container if the daemon restarts." For unless-stopped: "Similar to always, except that when the container is stopped (manually or otherwise), it isn't restarted even after Docker daemon restarts." This implies it does restart on daemon restart if not previously stopped.
Citations:
- 1: https://docs.docker.com/config/containers/start-containers-automatically
- 2: https://docs.docker.com/engine/containers/start-containers-automatically/
- 3: https://docs.docker.com/engine/admin/start-containers-automatically/
Docker 再起動後の Electric サービス復旧動作の変更を確認してください。
on-failure:5 に変更すると、Docker daemon/Desktop 再起動後に Electric コンテナが自動復旧されなくなります。公式ドキュメント上、on-failure はコンテナのエラー終了時にのみ再起動し、daemon 再起動時には起動しません。生成済み .env の ELECTRIC_URL が停止中のコンテナを指す状態になる可能性があります。
常駐サービスとして保証したい場合は、unless-stopped のまま据え置くか、開発起動フロー側で Electric の再起動を明示的に保証してください。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superset/lib/setup/steps.sh at line 262, The restart policy for the
Electric container was changed to "on-failure:5" which prevents automatic
recovery after Docker/daemon restarts; revert the restart flag in the setup
script (the line emitting "--restart on-failure:5 \") back to a persistent
policy such as "--restart unless-stopped \", or alternatively add explicit
restart logic in the development startup flow that ensures Electric is started
after daemon/reboot (update the invocation in .superset/lib/setup/steps.sh where
the restart option is emitted).
Comment on lines
+88
to
+112
| const hasConflict = document.conflict !== null; | ||
| useEffect(() => { | ||
| if (!hasConflict) return; | ||
| const name = filePath.split(/[/\\]/).pop(); | ||
| alert({ | ||
| title: `Do you want to save the changes you made to ${name}?`, | ||
| description: "Your changes will be lost if you don't save them.", | ||
| actions: [ | ||
| { | ||
| label: "Save", | ||
| onClick: () => document.resolveConflict("overwrite"), | ||
| }, | ||
| { | ||
| label: "Don't Save", | ||
| variant: "secondary", | ||
| onClick: () => document.resolveConflict("reload"), | ||
| }, | ||
| { | ||
| label: "Cancel", | ||
| variant: "ghost", | ||
| onClick: () => document.resolveConflict("keep"), | ||
| }, | ||
| ], | ||
| }); | ||
| }, [hasConflict, document, filePath]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n "apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx" | sed -n '1,120p'Repository: MocA-Love/superset
Length of output: 221
🏁 Script executed:
fd -u "FilePane.tsx" --type fRepository: MocA-Love/superset
Length of output: 201
🏁 Script executed:
cat -n 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx' | sed -n '80,120p'Repository: MocA-Love/superset
Length of output: 1377
🏁 Script executed:
cat -n 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx' | head -50Repository: MocA-Love/superset
Length of output: 2361
🏁 Script executed:
fd -u "fileDocumentStore" -type fRepository: MocA-Love/superset
Length of output: 232
🏁 Script executed:
rg -l "useSharedFileDocument" --type ts --type tsxRepository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
fd "fileDocumentStore" -type fRepository: MocA-Love/superset
Length of output: 232
🏁 Script executed:
rg "useSharedFileDocument" -g "*.ts" -g "*.tsx" -A 10Repository: MocA-Love/superset
Length of output: 21763
🏁 Script executed:
wc -l 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/state/fileDocumentStore/useSharedFileDocument.ts'Repository: MocA-Love/superset
Length of output: 198
🏁 Script executed:
cat -n 'apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/state/fileDocumentStore/useSharedFileDocument.ts'Repository: MocA-Love/superset
Length of output: 2465
🏁 Script executed:
rg "alert\(" -A 20 'apps/desktop/src/renderer' | head -100Repository: MocA-Love/superset
Length of output: 16431
🏁 Script executed:
fd "Alert" --extension ts --extension tsx | head -20Repository: MocA-Love/superset
Length of output: 99
🏁 Script executed:
cat -n 'packages/ui/src/atoms/Alert/Alert.tsx'Repository: MocA-Love/superset
Length of output: 3442
🏁 Script executed:
fd "Alert" -e ts -e tsx packages/ui/src/atoms | head -10Repository: MocA-Love/superset
Length of output: 99
競合ダイアログの多重表示を防いでください。
document が依存配列に含まれているため、オブジェクト参照が変わる度に effect が再実行されます。conflict が解決されていない状態で effect が再度実行されると、alert() が複数回呼ばれ、ダイアログが繰り返し開き直されるリスクがあります。useRef で「現在の conflict に対して既に prompt 済み」を管理し、同じ conflict に対しては alert() が一度だけ呼ばれるようにしてください。
修正案
-import { useCallback, useEffect } from "react";
+import { useCallback, useEffect, useRef } from "react"; const hasConflict = document.conflict !== null;
+ const conflictPromptOpenRef = useRef(false);
+
useEffect(() => {
- if (!hasConflict) return;
+ if (!hasConflict) {
+ conflictPromptOpenRef.current = false;
+ return;
+ }
+ if (conflictPromptOpenRef.current) return;
+ conflictPromptOpenRef.current = true;
+
const name = filePath.split(/[/\\]/).pop();
alert({
title: `Do you want to save the changes you made to ${name}?`,
description: "Your changes will be lost if you don't save them.",
actions: [📝 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
| const hasConflict = document.conflict !== null; | |
| useEffect(() => { | |
| if (!hasConflict) return; | |
| const name = filePath.split(/[/\\]/).pop(); | |
| alert({ | |
| title: `Do you want to save the changes you made to ${name}?`, | |
| description: "Your changes will be lost if you don't save them.", | |
| actions: [ | |
| { | |
| label: "Save", | |
| onClick: () => document.resolveConflict("overwrite"), | |
| }, | |
| { | |
| label: "Don't Save", | |
| variant: "secondary", | |
| onClick: () => document.resolveConflict("reload"), | |
| }, | |
| { | |
| label: "Cancel", | |
| variant: "ghost", | |
| onClick: () => document.resolveConflict("keep"), | |
| }, | |
| ], | |
| }); | |
| }, [hasConflict, document, filePath]); | |
| const hasConflict = document.conflict !== null; | |
| const conflictPromptOpenRef = useRef(false); | |
| useEffect(() => { | |
| if (!hasConflict) { | |
| conflictPromptOpenRef.current = false; | |
| return; | |
| } | |
| if (conflictPromptOpenRef.current) return; | |
| conflictPromptOpenRef.current = true; | |
| const name = filePath.split(/[/\\]/).pop(); | |
| alert({ | |
| title: `Do you want to save the changes you made to ${name}?`, | |
| description: "Your changes will be lost if you don't save them.", | |
| actions: [ | |
| { | |
| label: "Save", | |
| onClick: () => document.resolveConflict("overwrite"), | |
| }, | |
| { | |
| label: "Don't Save", | |
| variant: "secondary", | |
| onClick: () => document.resolveConflict("reload"), | |
| }, | |
| { | |
| label: "Cancel", | |
| variant: "ghost", | |
| onClick: () => document.resolveConflict("keep"), | |
| }, | |
| ], | |
| }); | |
| }, [hasConflict, document, filePath]); |
🤖 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/FilePane/FilePane.tsx
around lines 88 - 112, The effect in FilePane is re-running when the document
object reference changes causing duplicate alerts; add a useRef (e.g.,
lastPromptedConflictRef) to track the currently-prompted conflict and only call
alert() when document.conflict exists and lastPromptedConflictRef.current !==
document.conflict (or its unique id), then set the ref to that conflict after
showing the prompt; also reset lastPromptedConflictRef.current to null when
conflict becomes null (or when resolveConflict is invoked) so future conflicts
can prompt again; keep the effect dependencies to [hasConflict, filePath]
(remove document) and reference document.resolveConflict directly inside the
handlers.
Comment on lines
+249
to
+267
| label: "Save", | ||
| onClick: async () => { | ||
| const doc = getDocument(workspaceId, data.filePath); | ||
| if (!doc) { | ||
| resolve(true); | ||
| return; | ||
| } | ||
| const result = await doc.save(); | ||
| resolve(result.status === "saved"); | ||
| }, | ||
| }, | ||
| { | ||
| label: "Discard Changes", | ||
| variant: "destructive", | ||
| onClick: () => resolve(true), | ||
| label: "Don't Save", | ||
| variant: "secondary", | ||
| onClick: async () => { | ||
| const doc = getDocument(workspaceId, data.filePath); | ||
| if (doc) await doc.reload(); | ||
| resolve(true); | ||
| }, |
There was a problem hiding this comment.
Save/Don't Save の失敗時にも resolve してください。
Line 256 の doc.save() や Line 265 の doc.reload() が reject すると、この onBeforeClose Promise が未解決のままになり、hotkey 経由では close guard がロックされたままになります。失敗時は close を中断するなど、必ず resolve してください。
修正案
{
label: "Save",
onClick: async () => {
const doc = getDocument(workspaceId, data.filePath);
if (!doc) {
resolve(true);
return;
}
- const result = await doc.save();
- resolve(result.status === "saved");
+ try {
+ const result = await doc.save();
+ resolve(result.status === "saved");
+ } catch {
+ resolve(false);
+ }
},
},
{
label: "Don't Save",
variant: "secondary",
onClick: async () => {
- const doc = getDocument(workspaceId, data.filePath);
- if (doc) await doc.reload();
- resolve(true);
+ try {
+ const doc = getDocument(workspaceId, data.filePath);
+ if (doc) await doc.reload();
+ resolve(true);
+ } catch {
+ resolve(false);
+ }
},
},🤖 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/usePaneRegistry.tsx
around lines 249 - 267, The onClick handlers inside the onBeforeClose flow call
getDocument(...).save() and .reload() without error handling, so if those
promises reject the onBeforeClose Promise never resolves; wrap the await calls
in try/catch (in the Save and Don't Save handlers referenced by label "Save" and
label "Don't Save") and in the catch always call resolve(false) (and optionally
log the error) so the close guard is released and close is aborted on failure;
ensure every code path in those onClick handlers calls resolve(...) exactly
once.
Comment on lines
+75
to
92
| const isClosingPaneRef = useRef(false); | ||
| useHotkey("CLOSE_PANE", async () => { | ||
| if (isClosingPaneRef.current) return; | ||
| isClosingPaneRef.current = true; | ||
| try { | ||
| const state = store.getState(); | ||
| const active = state.getActivePane(); | ||
| if (!active) return; | ||
| const definition = paneRegistry[active.pane.kind]; | ||
| if (definition?.onBeforeClose) { | ||
| const allowed = await definition.onBeforeClose(active.pane); | ||
| if (!allowed) return; | ||
| } | ||
| state.closePane({ tabId: active.tabId, paneId: active.pane.id }); | ||
| } finally { | ||
| isClosingPaneRef.current = false; | ||
| } | ||
| }); |
There was a problem hiding this comment.
onBeforeClose の reject を close handler 側でも吸収してください。
Line 85 が throw/reject すると未処理 rejection になり、pane close は中断されます。close guard の失敗時は閉じないまま安全に return する形にしておくと、pane 定義側の例外に強くなります。
修正案
const definition = paneRegistry[active.pane.kind];
if (definition?.onBeforeClose) {
- const allowed = await definition.onBeforeClose(active.pane);
+ let allowed = false;
+ try {
+ allowed = await definition.onBeforeClose(active.pane);
+ } catch (error) {
+ console.error("Failed to run pane close guard", error);
+ return;
+ }
if (!allowed) 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
| const isClosingPaneRef = useRef(false); | |
| useHotkey("CLOSE_PANE", async () => { | |
| if (isClosingPaneRef.current) return; | |
| isClosingPaneRef.current = true; | |
| try { | |
| const state = store.getState(); | |
| const active = state.getActivePane(); | |
| if (!active) return; | |
| const definition = paneRegistry[active.pane.kind]; | |
| if (definition?.onBeforeClose) { | |
| const allowed = await definition.onBeforeClose(active.pane); | |
| if (!allowed) return; | |
| } | |
| state.closePane({ tabId: active.tabId, paneId: active.pane.id }); | |
| } finally { | |
| isClosingPaneRef.current = false; | |
| } | |
| }); | |
| const isClosingPaneRef = useRef(false); | |
| useHotkey("CLOSE_PANE", async () => { | |
| if (isClosingPaneRef.current) return; | |
| isClosingPaneRef.current = true; | |
| try { | |
| const state = store.getState(); | |
| const active = state.getActivePane(); | |
| if (!active) return; | |
| const definition = paneRegistry[active.pane.kind]; | |
| if (definition?.onBeforeClose) { | |
| let allowed = false; | |
| try { | |
| allowed = await definition.onBeforeClose(active.pane); | |
| } catch (error) { | |
| console.error("Failed to run pane close guard", error); | |
| return; | |
| } | |
| if (!allowed) return; | |
| } | |
| state.closePane({ tabId: active.tabId, paneId: active.pane.id }); | |
| } finally { | |
| isClosingPaneRef.current = false; | |
| } | |
| }); |
🤖 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/useWorkspaceHotkeys/useWorkspaceHotkeys.ts
around lines 75 - 92, The close handler may get an unhandled rejection if
definition.onBeforeClose throws; update the useHotkey("CLOSE_PANE") callback
(the isClosingPaneRef guarded block) to catch errors from
definition.onBeforeClose: call it inside a try/catch (or await with .catch) and
treat any thrown/rejected result the same as a false/denied value (i.e., safely
return without closing the pane), ensuring isClosingPaneRef is still reset in
the existing finally block; reference: isClosingPaneRef, useHotkey callback,
paneRegistry, definition.onBeforeClose, and state.closePane.
Comment on lines
584
to
+632
| onBeforeCloseTab={(tab) => { | ||
| const dirtyFiles = Object.values(tab.panes) | ||
| .filter( | ||
| (p) => | ||
| p.kind === "file" && (p.data as FilePaneData).hasChanges, | ||
| ) | ||
| .map((p) => | ||
| (p.data as FilePaneData).filePath.split("/").pop(), | ||
| ); | ||
| if (dirtyFiles.length === 0) return true; | ||
| const dirtyPanes = Object.values(tab.panes).filter((p) => { | ||
| if (p.kind !== "file") return false; | ||
| const filePath = (p.data as FilePaneData).filePath; | ||
| return getDocument(workspaceId, filePath)?.dirty === true; | ||
| }); | ||
| const dirtyFileNames = dirtyPanes.map((p) => | ||
| (p.data as FilePaneData).filePath.split(/[/\\]/).pop(), | ||
| ); | ||
| if (dirtyPanes.length === 0) return true; | ||
| const title = | ||
| dirtyFiles.length === 1 | ||
| ? `Do you want to save the changes you made to ${dirtyFiles[0]}?` | ||
| : `Do you want to save changes to ${dirtyFiles.length} files?`; | ||
| dirtyPanes.length === 1 | ||
| ? `Do you want to save the changes you made to ${dirtyFileNames[0]}?` | ||
| : `Do you want to save changes to ${dirtyPanes.length} files?`; | ||
| return new Promise<boolean>((resolve) => { | ||
| alert({ | ||
| title, | ||
| description: | ||
| "Your changes will be lost if you don't save them.", | ||
| actions: [ | ||
| { | ||
| label: "Cancel", | ||
| variant: "outline", | ||
| onClick: () => resolve(false), | ||
| label: "Save All", | ||
| onClick: async () => { | ||
| for (const pane of dirtyPanes) { | ||
| const filePath = (pane.data as FilePaneData) | ||
| .filePath; | ||
| const doc = getDocument(workspaceId, filePath); | ||
| if (!doc) continue; | ||
| const result = await doc.save(); | ||
| if (result.status !== "saved") { | ||
| resolve(false); | ||
| return; | ||
| } | ||
| } | ||
| resolve(true); | ||
| }, | ||
| }, | ||
| { | ||
| label: "Don't Save", | ||
| variant: "secondary", | ||
| onClick: async () => { | ||
| for (const pane of dirtyPanes) { | ||
| const filePath = (pane.data as FilePaneData) | ||
| .filePath; | ||
| const doc = getDocument(workspaceId, filePath); | ||
| if (doc) await doc.reload(); | ||
| } | ||
| resolve(true); | ||
| }, |
There was a problem hiding this comment.
save() / reload() の失敗時も必ず close 判定を resolve してください。
doc.save() または doc.reload() が reject すると、onBeforeCloseTab の Promise が解決されず、タブ close フローが固まる可能性があります。try/catch で resolve(false) し、ユーザーに失敗を通知するのが安全です。
🐛 修正案
{
label: "Save All",
onClick: async () => {
- for (const pane of dirtyPanes) {
- const filePath = (pane.data as FilePaneData)
- .filePath;
- const doc = getDocument(workspaceId, filePath);
- if (!doc) continue;
- const result = await doc.save();
- if (result.status !== "saved") {
- resolve(false);
- return;
- }
- }
- resolve(true);
+ try {
+ for (const pane of dirtyPanes) {
+ const filePath = (pane.data as FilePaneData)
+ .filePath;
+ const doc = getDocument(workspaceId, filePath);
+ if (!doc) {
+ resolve(false);
+ return;
+ }
+ const result = await doc.save();
+ if (result.status !== "saved") {
+ resolve(false);
+ return;
+ }
+ }
+ resolve(true);
+ } catch (error) {
+ const message =
+ error instanceof Error ? error.message : String(error);
+ toast.error(`Failed to save changes: ${message}`);
+ resolve(false);
+ }
},
},
{
label: "Don't Save",
variant: "secondary",
onClick: async () => {
- for (const pane of dirtyPanes) {
- const filePath = (pane.data as FilePaneData)
- .filePath;
- const doc = getDocument(workspaceId, filePath);
- if (doc) await doc.reload();
- }
- resolve(true);
+ try {
+ for (const pane of dirtyPanes) {
+ const filePath = (pane.data as FilePaneData)
+ .filePath;
+ const doc = getDocument(workspaceId, filePath);
+ if (doc) await doc.reload();
+ }
+ resolve(true);
+ } catch (error) {
+ const message =
+ error instanceof Error ? error.message : String(error);
+ toast.error(`Failed to discard changes: ${message}`);
+ resolve(false);
+ }
},
},📝 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
| onBeforeCloseTab={(tab) => { | |
| const dirtyFiles = Object.values(tab.panes) | |
| .filter( | |
| (p) => | |
| p.kind === "file" && (p.data as FilePaneData).hasChanges, | |
| ) | |
| .map((p) => | |
| (p.data as FilePaneData).filePath.split("/").pop(), | |
| ); | |
| if (dirtyFiles.length === 0) return true; | |
| const dirtyPanes = Object.values(tab.panes).filter((p) => { | |
| if (p.kind !== "file") return false; | |
| const filePath = (p.data as FilePaneData).filePath; | |
| return getDocument(workspaceId, filePath)?.dirty === true; | |
| }); | |
| const dirtyFileNames = dirtyPanes.map((p) => | |
| (p.data as FilePaneData).filePath.split(/[/\\]/).pop(), | |
| ); | |
| if (dirtyPanes.length === 0) return true; | |
| const title = | |
| dirtyFiles.length === 1 | |
| ? `Do you want to save the changes you made to ${dirtyFiles[0]}?` | |
| : `Do you want to save changes to ${dirtyFiles.length} files?`; | |
| dirtyPanes.length === 1 | |
| ? `Do you want to save the changes you made to ${dirtyFileNames[0]}?` | |
| : `Do you want to save changes to ${dirtyPanes.length} files?`; | |
| return new Promise<boolean>((resolve) => { | |
| alert({ | |
| title, | |
| description: | |
| "Your changes will be lost if you don't save them.", | |
| actions: [ | |
| { | |
| label: "Cancel", | |
| variant: "outline", | |
| onClick: () => resolve(false), | |
| label: "Save All", | |
| onClick: async () => { | |
| for (const pane of dirtyPanes) { | |
| const filePath = (pane.data as FilePaneData) | |
| .filePath; | |
| const doc = getDocument(workspaceId, filePath); | |
| if (!doc) continue; | |
| const result = await doc.save(); | |
| if (result.status !== "saved") { | |
| resolve(false); | |
| return; | |
| } | |
| } | |
| resolve(true); | |
| }, | |
| }, | |
| { | |
| label: "Don't Save", | |
| variant: "secondary", | |
| onClick: async () => { | |
| for (const pane of dirtyPanes) { | |
| const filePath = (pane.data as FilePaneData) | |
| .filePath; | |
| const doc = getDocument(workspaceId, filePath); | |
| if (doc) await doc.reload(); | |
| } | |
| resolve(true); | |
| }, | |
| onBeforeCloseTab={(tab) => { | |
| const dirtyPanes = Object.values(tab.panes).filter((p) => { | |
| if (p.kind !== "file") return false; | |
| const filePath = (p.data as FilePaneData).filePath; | |
| return getDocument(workspaceId, filePath)?.dirty === true; | |
| }); | |
| const dirtyFileNames = dirtyPanes.map((p) => | |
| (p.data as FilePaneData).filePath.split(/[/\\]/).pop(), | |
| ); | |
| if (dirtyPanes.length === 0) return true; | |
| const title = | |
| dirtyPanes.length === 1 | |
| ? `Do you want to save the changes you made to ${dirtyFileNames[0]}?` | |
| : `Do you want to save changes to ${dirtyPanes.length} files?`; | |
| return new Promise<boolean>((resolve) => { | |
| alert({ | |
| title, | |
| description: | |
| "Your changes will be lost if you don't save them.", | |
| actions: [ | |
| { | |
| label: "Save All", | |
| onClick: async () => { | |
| try { | |
| for (const pane of dirtyPanes) { | |
| const filePath = (pane.data as FilePaneData) | |
| .filePath; | |
| const doc = getDocument(workspaceId, filePath); | |
| if (!doc) { | |
| resolve(false); | |
| return; | |
| } | |
| const result = await doc.save(); | |
| if (result.status !== "saved") { | |
| resolve(false); | |
| return; | |
| } | |
| } | |
| resolve(true); | |
| } catch (error) { | |
| const message = | |
| error instanceof Error ? error.message : String(error); | |
| toast.error(`Failed to save changes: ${message}`); | |
| resolve(false); | |
| } | |
| }, | |
| }, | |
| { | |
| label: "Don't Save", | |
| variant: "secondary", | |
| onClick: async () => { | |
| try { | |
| for (const pane of dirtyPanes) { | |
| const filePath = (pane.data as FilePaneData) | |
| .filePath; | |
| const doc = getDocument(workspaceId, filePath); | |
| if (doc) await doc.reload(); | |
| } | |
| resolve(true); | |
| } catch (error) { | |
| const message = | |
| error instanceof Error ? error.message : String(error); | |
| toast.error(`Failed to discard changes: ${message}`); | |
| resolve(false); | |
| } | |
| }, |
🤖 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/page.tsx
around lines 584 - 632, The onBeforeCloseTab handler (the onBeforeCloseTab
callback where dirtyPanes are iterated) can hang if doc.save() or doc.reload()
rejects; wrap each await doc.save() and await doc.reload() in try/catch, and on
any caught error call resolve(false) (to prevent the Promise from never
resolving) and surface a user notification via the existing alert mechanism (or
an error alert) so the user is informed; ensure you reference the same
dirtyPanes loop and getDocument(workspaceId, filePath) lookups, and do not let
exceptions propagate out of the onBeforeCloseTab Promise.
MocA-Love
added a commit
that referenced
this pull request
Apr 17, 2026
Upstream commits processed (cherry-picked, then adapted where needed): - 07c1ee0 fix(desktop): Cmd+O firing open-in twice on v1 workspace route (superset-sh#3511) → PR #302 (with fork tearoff-window adaptation for Cmd+O) - 4a1f41a chore(deps): upgrade tanstack/db + electric, drop durable-streams patch (superset-sh#3509) → PR #303 (fork keeps fstream patch) - a3df489 feat(desktop): v2 PR checkout via widened checkout procedure (superset-sh#3525) → PR #304 (clean) - c504a50 feat(desktop): v2 file editor — foundation, views, and stability pass (superset-sh#3526) → PR #310 (foundation: 58 files path-checkout) → PR #311 (adaptation: 20 files manual port with SpreadsheetViewer/memo/fork-hotkeys preserved) - 78b7dc8 feat(desktop): promote "Create Section Below" to top-level on workspace menu (superset-sh#3537) → PR #308 Record merge so upstream/main..main shows 0 behind.
This was referenced Apr 22, 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
1 participant
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
c504a5036 feat(desktop): v2 file editor — foundation, views, and stability pass (#3526)の 後半 = fork 適応込みの手動移植 を取り込む PR です。PR4（foundation）で導入済みのfileDocumentStoreと view registry を、既存の v2 workspace ワイヤリングに接続します。behind は c504 最終 commit として一本化。取り込み範囲
c504 の 78 files のうち PR4 以降の 20 files:
page.tsx,types.ts,usePaneRegistry/usePaneRegistry.tsx,FilePane/FilePane.tsx,useWorkspaceHotkeys.ts,useDefaultContextMenuActions.tsx,hotkeys/registry.ts,editor-theme.ts,useCopyToClipboard.ts(→ 後述で revert),PathActionsMenuItems.tsx,.superset/lib/setup/steps.sh,apps/desktop/package.json,bun.lockhooks/host-service/useFileDocument/**(2),FilePane/components/ExternalChangeBar/**(2),FilePane/renderers/{CodeRenderer,ImageRenderer,MarkdownRenderer}/**(6)fork 適応の要点
維持 (fork-only)
FilePane,FilePaneHeaderExtras,usePaneRegistry.FilePaneTabTitle,WorkspaceFilePreviewContentすべてでisSpreadsheetFile(filePath)を先に見てSpreadsheetViewerへ直送し、shared document store を呼ばないFilePaneData.displayNameを残し、FilePaneの新useEffectでderiveMemoDisplayName(content.value)を計算してcontext.actions.updateData({displayName})。FilePaneTabTitleの name fallback はdata.displayName ?? getFileName(filePath)openFilePane(filePath, displayName?)variantrightSidebarOpenViewWidthに合わせてresizeSplitを呼ぶ fork 独自 width auto-correctionreloadToken/hardReloadToken書き込みmode/reloadToken/hardReloadTokenuseSharedFileDocumentに portshiki-theme.tsが参照）split(/[/\\]/).pop()に戻す（Windows cross-platform）取り込み (upstream)
FileDocumentStoreProviderwrap、getDocument経由の onBeforeCloseTab / onBeforeClose (Save / Don't Save / Cancel dialog)FilePaneHeaderExtras(view toggle in tab header)hasChanges削除（live dirty はdocument.dirtyで読む）CLOSE_TERMINAL→CLOSE_PANElabel 更新、useWorkspaceHotkeys({ paneRegistry })引数SHOW_HOTKEYSを Cmd+/ → Cmd+Shift+/ 移動（CodeMirror の comment toggle を Cmd+/ に解放）PathActionsMenuItems.tsxのtoast.promiserewriteeditor-theme.tsのactiveLinetoken 更新WorkspaceClientProvider→ fileDocumentStore がtrpcClientを受け取る（PR4 で済）検証
Codex pre-review
全 3 ラウンド。最終 Yes。
BROWSER_RELOADwiring gap, SpreadsheetViewer 側の store bypass 不完全, Windows path regression の 3 件を指摘useCopyToClipboard回帰,WorkspaceFilePreviewContentspreadsheet preview 回帰, 残りのsplit('/')3 箇所を指摘Deferred (pre-existing, not PR5-scope)
BROWSER_RELOAD/BROWSER_HARD_RELOADは v2 のusePersistentWebview側でreloadToken/hardReloadTokenを監視していないため実質 no-op。fork commit87d87d55c(2026-04-02) 時点から main にずっと存在する wiring 抜け。v2 webview で reload を発火するには別途usePersistentWebviewへの配線追加が必要 — 本 PR のスコープ外テストチェックリスト
SpreadsheetViewerで表示、タブ title に dirty dot が出ないSpreadsheetViewerSummary by CodeRabbit
リリースノート
New Features & Improvements
Bug Fixes