merge: integrate upstream/main (2026-04-03)#53
Merged
Conversation
…set-sh#3108) * height * lifecycle * Deslop * Refactor * Separate terminal runtime from WebSocket transport in renderer Split terminal-runtime-registry.ts into three focused modules: - terminal-runtime.ts: XTerm instance, addons, DOM wrapper, buffer/dimension persistence, fit/resize - terminal-ws-transport.ts: WebSocket connection, message protocol, connection state - terminal-runtime-registry.ts: orchestrator mapping paneId to runtime + transport * Fix stale WebSocket handlers and eager runtime creation - Guard onClose/onError/onMessage with socket reference comparison to prevent displaced connections from corrupting active session state - Close old socket explicitly on reattach (code 4000) - Create registry entry eagerly in onStateChange so useSyncExternalStore listeners are registered before the attach effect runs * Co-locate useGlobalTerminalLifecycle under its component Move hook from routes/_authenticated/hooks/ into components/GlobalTerminalLifecycle/hooks/ since it's only used by that component. * Keep WebSocket alive across terminal attach/detach cycles detach() now only removes the DOM wrapper, resize observer, and focus — the WebSocket and xterm data flow stay alive so output written while the pane is hidden (tab switch, workspace switch) is not lost. attach() checks whether the transport is already open for the same URL and skips reconnection on simple re-shows, only reconnecting when the socket dropped or the endpoint changed. A full terminal.refresh() is added on re-attach to repaint rows that were written while the canvas was offscreen. * Lint * Move connect idempotency into transport layer connect() now early-returns when already open or connecting to the same URL, covering rapid tab-switch during an in-flight handshake. The registry no longer branches — it just calls connect() unconditionally.
…ctor (superset-sh#3122) * feat(desktop): file tree sidebar, file pane, and alert refactor - Add RightSidebar with file tree, toggled via Cmd+L, state persisted in workspace local collection - Create FilePane with CodeRenderer (Monaco), MarkdownRenderer (TipTap), and ImageRenderer — replaces WorkspaceFilePreview - File-type icons, italic title when unpinned, dirty indicator dot - onBeforeClose on PaneDefinition — registry-level close guards with Save/Don't Save/Cancel dialog - onHeaderClick on PaneDefinition — click header to pin, middle-click to close - selectedFilePath derived from store, sidebar highlights active file - Click file in sidebar: pin if already active, else open unpinned - Cmd+O wired up in OpenInMenuButton for both old and v2 workspace - setPanePinned: removed unnecessary tabId param - useFileTree: added reveal() method, fixed stale cache (staleTime: 0) - Alert component: refactored from onConfirm/onCancel to actions array API, migrated all 8 call sites * fix: scroll revealed file to center of sidebar * fix: remove unused filePath param from MarkdownRenderer
…ion (superset-sh#3121) These per-tool hooks add overhead without providing value for Codex. Only SessionStart, UserPromptSubmit, and Stop are needed.
📝 WalkthroughWalkthroughCodexフック管理から Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer as Renderer (UI)
participant Registry as TerminalRuntimeRegistry
participant Runtime as TerminalRuntime
participant Transport as TerminalTransport
participant Host as Host Service
Renderer->>Registry: attach(paneId, container, wsUrl)
Registry->>Runtime: getOrCreate(paneId)
Runtime-->>Registry: TerminalRuntime
Registry->>Transport: getOrCreate()
Transport-->>Registry: TerminalTransport
Registry->>Runtime: attachToContainer(runtime, container)
Registry->>Transport: connect(transport, runtime.terminal, wsUrl)
Transport->>Host: WebSocket connect
Host-->>Transport: open / messages (data/replay/error/exit)
Renderer->>Registry: detach(paneId)
Registry->>Runtime: detachFromContainer(runtime)
Note over Renderer,Registry: 500ms 後に paneId 未検出なら dispose を呼ぶ
Registry->>Transport: sendDispose(transport)
Registry->>Transport: disposeTransport(transport)
Registry->>Runtime: disposeRuntime(runtime)
sequenceDiagram
participant User as User
participant FilePane as FilePane
participant Document as useFileDocument
participant Renderer as Markdown/Code/Image Renderers
participant Storage as File Storage
User->>FilePane: select file
FilePane->>Document: open/use document
Document->>Storage: fetch content
Storage-->>Document: content
Document-->>FilePane: document state (type/content)
FilePane->>Renderer: render appropriate renderer
User->>Renderer: edit
Renderer->>FilePane: onDirtyChange(true)
FilePane->>Storage: save (onSave)
Storage-->>FilePane: save success
FilePane->>Renderer: clear dirty
External Change->>FilePane: notify hasExternalChange
FilePane->>Renderer: show ExternalChangeBar
User->>ExternalChangeBar: click Reload
ExternalChangeBar->>Document: reload()
Document->>Storage: re-fetch
Storage-->>Document: updated content
Document-->>Renderer: refreshed content
Estimated code review effort🎯 5 (Critical) | ⏱️ ~105 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 |
Add a cloud download button (IoCloudDownloadOutline) to the left of the Refresh Changes button in the right sidebar's Git tab. Clicking it runs git fetch --prune via a new fetchRemote tRPC mutation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (6)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx-83-85 (1)
83-85:⚠️ Potential issue | 🟡 Minorホットキー実行条件をボタン無効条件と揃えてください。
Line 95でボタンを!currentApp時に無効化していますが、Line 83のホットキーは常時登録されるため、キーボード経由だけ実行できる状態が発生します。ホットキー側にも同等のガードを入れてください。差分案
-useAppHotkey("OPEN_IN_APP", handleOpenInEditor, undefined, [ - handleOpenInEditor, -]); +useAppHotkey( + "OPEN_IN_APP", + () => { + if (!currentApp || isLoading) return; + handleOpenInEditor(); + }, + undefined, + [currentApp, isLoading, handleOpenInEditor], +);🤖 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/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx` around lines 83 - 85, The hotkey registered via useAppHotkey("OPEN_IN_APP", handleOpenInEditor, ...) is always active while the button is disabled when !currentApp; make the hotkey behavior consistent by adding the same guard—either only register the hotkey when currentApp is truthy or have handleOpenInEditor early-return when currentApp is falsy. Update the OpenInMenuButton component to reference the currentApp guard (same condition used to disable the button) when calling useAppHotkey so keyboard invocation cannot run if currentApp is missing.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx-24-24 (1)
24-24:⚠️ Potential issue | 🟡 MinorWindows パスだと
altがファイル名になりません。
split("/")だとC:\foo\bar.pngを分解できないので、区切り文字は/と\の両方を扱った方がよいです。修正例
- alt={filePath.split("/").pop() ?? ""} + alt={filePath.split(/[/\\]/).pop() ?? ""}🤖 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/renderers/ImageRenderer/ImageRenderer.tsx at line 24, The alt value in ImageRenderer uses filePath.split("/") which fails for Windows paths; update the logic in ImageRenderer to derive the filename from filePath that handles both "/" and "\" (e.g., split on a regex like /[\/\\]/ or use a cross-platform basename helper) so alt={...} gets the correct file name even for Windows paths; locate the alt assignment in the ImageRenderer component and replace the split("/") usage on filePath with a split or helper that handles both separators.packages/workspace-client/src/hooks/useFileTree/useFileTree.ts-483-486 (1)
483-486:⚠️ Potential issue | 🟡 Minor
revealのパス境界チェックはstartsWithでは不十分です。
/repoが/repo-backupにも一致してしまうので、ルート外のディレクトリを expand し得ます。ここは同ファイルの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 `@packages/workspace-client/src/hooks/useFileTree/useFileTree.ts` around lines 483 - 486, reveal 内のパス境界チェックで startsWith を使うと `/repo` が `/repo-backup` にマッチしてしまうため、ルート外ディレクトリを誤展開する可能性があります；代わりに同ファイルの isWithinPath 関数を使って rootPath と absolutePath の包含関係を正確に判定し、条件分岐を startsWith ベースのチェックから isWithinPath(rootPath, absolutePath) を使う形に置き換えてください（関数名: reveal, ユーティリティ: isWithinPath）。apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx-94-104 (1)
94-104:⚠️ Potential issue | 🟡 Minor削除操作にエラーハンドリングがありません。
authClient.apiKey.deleteが失敗した場合、ユーザーにフィードバックがありません。Line 69-87のhandleGenerateKeyと同様に、try/catchでエラーを処理することを推奨します。🛡️ 修正案
{ label: "Revoke", variant: "destructive", onClick: async () => { - await authClient.apiKey.delete({ keyId: id }); - toast.success("API key revoked"); + try { + await authClient.apiKey.delete({ keyId: id }); + toast.success("API key revoked"); + } catch (error) { + console.error("[api-keys] Failed to revoke API key:", error); + toast.error("Failed to revoke API key"); + } }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx` around lines 94 - 104, The revoke action's onClick handler calls authClient.apiKey.delete({ keyId: id }) without error handling; wrap that async call inside a try/catch in the onClick for the "Revoke" action (the handler that currently awaits authClient.apiKey.delete and then calls toast.success("API key revoked")), log or surface the caught error and call toast.error(...) on failure so the user receives feedback, and keep the success toast on the happy path.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx-23-23 (1)
23-23:⚠️ Potential issue | 🟡 Minor
viewModeの状態が変更される手段がない
_setViewModeがコンポーネント内で使用されておらず、MarkdownViewModeToggleコンポーネントは外部エクスポート用に存在しますが、viewMode状態はMarkdownRenderer内部に閉じているため、ビューモードを切り替える方法がありません。
viewModeとonViewModeChangeをpropsとして受け取るか、MarkdownViewModeToggleを内部でレンダリングするか検討してください。🤖 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/renderers/MarkdownRenderer/MarkdownRenderer.tsx at line 23, MarkdownRenderer defines local state const [viewMode, _setViewMode] = useState<MarkdownViewMode>("rendered") but never uses _setViewMode, so there is no way to toggle modes; update MarkdownRenderer to either accept viewMode and onViewModeChange as props (e.g. add props viewMode: MarkdownViewMode and onViewModeChange: (m: MarkdownViewMode)=>void and use them instead of the internal state) or render the existing MarkdownViewModeToggle inside MarkdownRenderer and hook its change handler to call _setViewMode so the viewMode state can actually change; modify the component signature and usages accordingly to keep identifiers viewMode, _setViewMode, MarkdownViewModeToggle, and onViewModeChange consistent.apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx-23-25 (1)
23-25:⚠️ Potential issue | 🟡 Minor外部からの
content変更時にsavedContentとcurrentContentRefが同期されない問題
contentpropが外部から変更された場合（例：onReload後）、savedContentとcurrentContentRefが古い値のままになり、dirty状態の判定が不正確になる可能性があります。🛠️ 修正案: contentの変更を検知して状態をリセット
export function MarkdownRenderer({ content, hasExternalChange, onDirtyChange, onReload, onSave, }: MarkdownRendererProps) { const [viewMode, _setViewMode] = useState<MarkdownViewMode>("rendered"); const currentContentRef = useRef(content); const [savedContent, setSavedContent] = useState(content); + + // 外部からcontentが変更された場合、状態をリセット + useEffect(() => { + currentContentRef.current = content; + setSavedContent(content); + onDirtyChange(false); + }, [content, onDirtyChange]);🤖 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/renderers/MarkdownRenderer/MarkdownRenderer.tsx around lines 23 - 25, 外部から渡される content が更新されたときに savedContent と currentContentRef が同期されていないためダーティ判定が壊れます。MarkdownRenderer 内で content を監視する useEffect を追加し、content が変化したら setSavedContent(content) を呼び出して savedContent をリセットし、currentContentRef.current = content で ref も更新してください（参照シンボル: content prop, savedContent, setSavedContent, currentContentRef）。これにより onReload 等で外部更新されたときに状態と ref が一致します。
🧹 Nitpick comments (3)
apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts (1)
52-70: 空のcatchブロックについて
persistBuffer、restoreBuffer、clearPersistedBufferで空のcatchブロックが使用されています。localStorageはプライベートブラウジングモードやクォータ超過時に例外をスローする可能性があるため、これは意図的な設計と理解できますが、デバッグ時に問題を追跡しにくくなる可能性があります。♻️ 開発時のデバッグを助けるためのログ追加案
function persistBuffer(paneId: string, serializeAddon: SerializeAddon) { try { const data = serializeAddon.serialize({ scrollback: SERIALIZE_SCROLLBACK }); localStorage.setItem(`${STORAGE_KEY_PREFIX}${paneId}`, data); - } catch {} + } catch (e) { + // localStorage may be unavailable in private browsing or quota exceeded + if (import.meta.env.DEV) { + console.warn("[terminal-runtime] failed to persist buffer:", e); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts` around lines 52 - 70, The empty catch blocks in persistBuffer, restoreBuffer, and clearPersistedBuffer hide errors from localStorage/serialization; update each catch to capture the error (e) and log a concise debug/warn message including the function name, STORAGE_KEY_PREFIX/paneId, and the error details (e.g. "persistBuffer failed for {paneId}: {error}"), keeping behavior unchanged in production (log only in development or use console.debug/console.warn) so failures are visible when debugging but do not crash the app.apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts (1)
67-75: サーバーメッセージの型安全性を強化する検討
JSON.parseの結果をTerminalServerMessageとして直接アサートしていますが、不正なサーバーペイロードの場合、後続のmessage.typeチェックで未定義の動作が発生する可能性があります。現在の実装ではパースエラーのみを捕捉していますが、スキーマ検証がないため、予期しない型のメッセージが処理される可能性があります。♻️ 基本的な型チェックを追加する案
socket.addEventListener("message", (event) => { if (transport.socket !== socket) return; let message: TerminalServerMessage; try { - message = JSON.parse(String(event.data)) as TerminalServerMessage; + const parsed = JSON.parse(String(event.data)); + if (!parsed || typeof parsed.type !== "string") { + terminal.writeln("\r\n[terminal] invalid server payload"); + return; + } + message = parsed as TerminalServerMessage; } catch { terminal.writeln("\r\n[terminal] invalid server payload"); return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts` around lines 67 - 75, Parsed JSON is being asserted directly to TerminalServerMessage and may lack required fields; after JSON.parse in the socket.addEventListener callback, validate the parsed value before treating it as TerminalServerMessage: ensure it's a non-null object, has a 'type' property (and other required fields depending on message.type) and that message.type is a string, or implement a small type guard/isValidTerminalServerMessage function to perform these checks; if validation fails, write the invalid payload to terminal and return instead of proceeding to use message.type. Reference symbols: socket.addEventListener callback, variable message, and the TerminalServerMessage type.apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts (1)
90-96:onStateChangeがエントリを副作用として作成する可能性
onStateChangeはgetOrCreateを呼び出すため、存在しないpaneIdでリスナーを登録しようとすると、意図せずruntime/transportのエントリが作成されます。これはuseSyncExternalStoreなどで使用される場合、不要なリソース割り当ての原因になる可能性があります。♻️ エントリが存在しない場合はno-opを返す実装案
onStateChange(paneId: string, listener: () => void): () => void { - const { transport } = this.getOrCreate(paneId); + const entry = this.entries.get(paneId); + if (!entry) { + // エントリが存在しない場合は何もしないunsubscribeを返す + return () => {}; + } + const { transport } = entry; transport.stateListeners.add(listener); return () => { transport.stateListeners.delete(listener); }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts` around lines 90 - 96, onStateChange currently calls getOrCreate which creates a runtime/transport entry as a side effect; change it to check for an existing entry and no-op if none: call the non-creating lookup (e.g., this.get(paneId) or an equivalent method) instead of this.getOrCreate(paneId), only add the listener to transport.stateListeners when a transport exists, and return a stable no-op unsubscribe when there is no entry; keep references to onStateChange, getOrCreate, and transport.stateListeners to locate and update the code.
🤖 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/lib/trpc/routers/changes/git-operations.ts`:
- Around line 346-352: The fetchRemote mutation (in function fetchRemote)
performs git.fetch but doesn't clear the status cache; add a call to
clearStatusCacheForWorktree(input.worktreePath) immediately after the await
git.fetch([...]) and before returning { success: true } so the ahead/behind
status is refreshed; keep the existing assertRegisteredWorktree and
getGitWithShellPath usage and only add the cache-clear call in the same
mutation.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/RightSidebar/RightSidebar.tsx:
- Around line 135-143: The WorkspaceFilesToolbar is passing empty handlers for
onNewFile and onNewFolder (in RightSidebar/RightSidebar.tsx) so the toolbar
shows controls that do nothing; either wire these to the existing creation flows
(e.g. call the workspace's createFile/createFolder functions or open the
create-file/create-folder modal handlers) by replacing the empty arrow functions
with the actual handlers, or disable/hide those toolbar buttons by passing a
disable prop or switching to onNewFile={undefined}/onNewFolder={undefined} (or a
showNewButtons=false flag) until the implementation exists. Locate the
WorkspaceFilesToolbar usage and update the onNewFile and onNewFolder props to
point to the appropriate functions (or disable the UI) rather than empty
handlers.
- Around line 65-84: The effect that calls fileTree.reveal never runs on first
render because prevSelectedRef is initialized to the current selectedFilePath;
change prevSelectedRef so it starts empty (e.g., useRef<string |
undefined>(undefined) or null) instead of useRef(selectedFilePath) so the
condition in the useEffect (selectedFilePath !== prevSelectedRef.current) will
be true on initial mount and trigger fileTree.reveal + the scrollIntoView logic
in the effect; update the declaration of prevSelectedRef and keep the rest of
the useEffect (and use of scrollContainerRef, selectedFilePath, rootPath,
fileTree) unchanged.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx:
- Around line 1-3: ExternalChangeBarProps の onReload が呼ばれる箇所（例:
ExternalChangeBar の onClick ハンドラ）で onReload() の返す Promise
を放置しており拒否が未処理になっているため、onReload() 呼び出しを明示的に await し try/catch で囲んで拒否を処理してください;
具体的には ExternalChangeBar コンポーネント内の onClick / リロードトリガーで await onReload() を実行し
catch でエラーログ出力またはユーザー向けのエラーフィードバックを行うように修正してください。
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/CodeRenderer/CodeRenderer.tsx:
- Around line 24-43: The code fails to keep currentContentRef and savedContent
in sync with prop content and updates the saved snapshot unconditionally; add a
useEffect that watches content and sets currentContentRef.current = content and
setSavedContent(content) so both follow external replacements, change
handleChange to guard onDirtyChange (if present) before calling it, and update
handleSave to only setSavedContent(currentContentRef.current) and call
onDirtyChange(false) after a successful onSave (wrap await onSave in try/catch
and only mutate savedContent/onDirtyChange on success) — reference symbols:
currentContentRef, savedContent, handleChange, handleSave.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx:
- Around line 10-18: The current useMemo in ImageRenderer (producing dataUrl
from content and filePath via btoa(Array.from(...))) copies the entire file into
strings and hurts memory/renderer performance for large images; replace this
with creating a Blob from the Uint8Array and using URL.createObjectURL(blob)
instead, update the hook to produce an objectUrl (based on content and
filePath), and ensure you revoke the URL on cleanup (useEffect cleanup or return
function) to avoid leaks; keep getImageMimeType(filePath) for the Blob type and
update any consumers expecting dataUrl to accept the object URL.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/usePaneRegistry.tsx:
- Around line 56-65: The Save button in the alert callback currently
short-circuits with resolve(true) inside usePaneRegistry's confirmation flow
(the TODOed Save handler in the alert call), so choosing "Save" does not perform
or wait for an actual save; change the handler to invoke the real save routine
(e.g., call the editor save method via the editor ref or the existing save
function used elsewhere), await its completion and only call resolve(true) after
the save succeeds (on failure, show an error and resolve(false) or keep the pane
open); remove the immediate resolve(true) placeholder and ensure the Save branch
only resolves true on confirmed success.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/page.tsx:
- Around line 326-337: The "Save All" action in the alert currently calls
resolve(true) immediately without performing saves (the onClick handler at the
alert actions uses resolve(true)), which closes dirty file panes without
persisting changes; modify that onClick to invoke the actual save routine (use
the editor refs or the existing saveAll/saveFile function used elsewhere in this
component), await the save completion, and only call resolve(true) after all
saves succeed; on save failure or cancellation resolve(false) and surface an
error/toast so the user stays on the page.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`:
- Around line 1136-1148: The FetchRemoteButton's handleClick currently swallows
errors and doesn't refresh data after a successful fetch, so update handleClick
in FetchRemoteButton to (1) on success call the appropriate TRPC
refetch/invalidate methods (e.g., invalidate or refetch the changes/branches/PR
queries used elsewhere) so remote updates are reflected immediately, and (2)
replace the empty catch block with error handling that at minimum logs the error
(console.error or processLogger) and optionally surface a UI error, while
preserving the finally block that sets isFetching(false); apply the same fixes
to the other similar handler at the referenced location.
In `@apps/marketing/package.json`:
- Line 28: The upgrade to next-mdx-remote v6.0.0 disables MDX JavaScript
expressions by default (blockJS: true, blockDangerousJS: true), which strips
expressions used in files like parallel-agents-guide.mdx and causes your RSC
page components (apps/marketing/src/app/*/page.tsx) that serialize raw MDX on
the server to lose content; fix by either removing JS expressions from the MDX
files or, if the MDX is trusted and needs expressions, modify the server-side
serialize call used by those page components to pass explicit options { blockJS:
false, blockDangerousJS: true } so expressions are preserved while still
blocking dangerous JS.
In `@packages/host-service/src/terminal/terminal.ts`:
- Around line 205-217: The pty.onExit handler currently marks session.exited but
never removes the session from sessions, causing zombie sessions if dispose
isn't called; update the handler (pty.onExit) to schedule TTL-based eviction for
sessions that are both exited and not connected: set a per-session eviction
timer (e.g., session.evictionTimer) when exit occurs and
session.socket?.readyState !== 1 that removes the session from the sessions
collection after a configurable TTL; ensure any existing evictionTimer is
cleared if the socket reconnects or when dispose() runs (dispose should clear
session.evictionTimer and immediately delete from sessions), and keep
sendMessage behavior unchanged for connected sockets.
- Around line 103-107: The session key currently uses only paneId (from
c.req.param("paneId")), which allows cross-workspace reconnection; change the
session lookup/creation to include workspaceId (from
c.req.query("workspaceId"))—e.g., form a composite key like
`${workspaceId}:${paneId}` when indexing sessions OR store workspaceId on the
session object when creating it and in the existing-branch verify
session.workspaceId === workspaceId before reattaching; update all places that
read/write the session map (the upgradeWebSocket handler around the existing
branch and the reconnect/attach logic referenced at lines ~119-143 and ~185-196)
to use the composite key or the workspaceId equality check so reconnections
cannot attach to a PTY from a different workspace/lifetime.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx`:
- Around line 64-70: Header-level click handlers (onClick and onAuxClick) are
firing for clicks inside child interactive areas like actionsContent and toolbar
due to event bubbling; update the PaneHeader's handlers to only invoke
onClick/onMiddleClick when the event originates on the header element itself
(e.g., check event.target === event.currentTarget) so clicks on children are
ignored, and keep the existing e.preventDefault() behavior for middle-clicks
when calling onMiddleClick; locate the handlers on the PaneHeader component (the
onClick and onAuxClick props) and add this event-origin guard to both.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx`:
- Around line 84-90: The close action passes through an unhandled promise from
definition.onBeforeClose, which can cause unhandled rejections when
actions.close is called (e.g., via PaneHeader.onMiddleClick). Wrap the call to
definition.onBeforeClose(pane) inside a try/catch in the close async function
(and the duplicate at the other occurrence) so any rejection is caught; on error
log or swallow the error and treat it as a denied close (return early),
otherwise continue to call store.getState().closePane({ tabId: tab.id, paneId:
pane.id }); thereby ensuring onBeforeClose exceptions are handled synchronously
by the action.
In `@packages/ui/src/atoms/Alert/Alert.tsx`:
- Around line 25-29: The alert dialog allows dismiss via Esc/outside click but
the dismiss path only closes the dialog and does not call any action or resolve
the Promise returned by alert(), leaving callers of
onBeforeClose/onBeforeCloseTab pending; update the Alert component/alert() flow
so that dismiss explicitly notifies the caller: either (preferred) add and
invoke an onDismiss (or include a special AlertAction with type "dismiss") so
alert() resolves (e.g., resolves false or a distinct dismiss value) when user
dismisses, OR make the dialog non-dismissible by disabling escape/outside-close
in the Alert/dialog implementation; locate the Alert component, the alert()
wrapper and the onBeforeClose/onBeforeCloseTab callsites and implement one of
these fixes so Promises always settle on dismiss.
---
Minor comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx`:
- Around line 83-85: The hotkey registered via useAppHotkey("OPEN_IN_APP",
handleOpenInEditor, ...) is always active while the button is disabled when
!currentApp; make the hotkey behavior consistent by adding the same guard—either
only register the hotkey when currentApp is truthy or have handleOpenInEditor
early-return when currentApp is falsy. Update the OpenInMenuButton component to
reference the currentApp guard (same condition used to disable the button) when
calling useAppHotkey so keyboard invocation cannot run if currentApp is missing.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/ImageRenderer/ImageRenderer.tsx:
- Line 24: The alt value in ImageRenderer uses filePath.split("/") which fails
for Windows paths; update the logic in ImageRenderer to derive the filename from
filePath that handles both "/" and "\" (e.g., split on a regex like /[\/\\]/ or
use a cross-platform basename helper) so alt={...} gets the correct file name
even for Windows paths; locate the alt assignment in the ImageRenderer component
and replace the split("/") usage on filePath with a split or helper that handles
both separators.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/FilePane/renderers/MarkdownRenderer/MarkdownRenderer.tsx:
- Line 23: MarkdownRenderer defines local state const [viewMode, _setViewMode] =
useState<MarkdownViewMode>("rendered") but never uses _setViewMode, so there is
no way to toggle modes; update MarkdownRenderer to either accept viewMode and
onViewModeChange as props (e.g. add props viewMode: MarkdownViewMode and
onViewModeChange: (m: MarkdownViewMode)=>void and use them instead of the
internal state) or render the existing MarkdownViewModeToggle inside
MarkdownRenderer and hook its change handler to call _setViewMode so the
viewMode state can actually change; modify the component signature and usages
accordingly to keep identifiers viewMode, _setViewMode, MarkdownViewModeToggle,
and onViewModeChange consistent.
- Around line 23-25: 外部から渡される content が更新されたときに savedContent と currentContentRef
が同期されていないためダーティ判定が壊れます。MarkdownRenderer 内で content を監視する useEffect を追加し、content
が変化したら setSavedContent(content) を呼び出して savedContent
をリセットし、currentContentRef.current = content で ref も更新してください（参照シンボル: content prop,
savedContent, setSavedContent, currentContentRef）。これにより onReload 等で外部更新されたときに状態と
ref が一致します。
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsx`:
- Around line 94-104: The revoke action's onClick handler calls
authClient.apiKey.delete({ keyId: id }) without error handling; wrap that async
call inside a try/catch in the onClick for the "Revoke" action (the handler that
currently awaits authClient.apiKey.delete and then calls toast.success("API key
revoked")), log or surface the caught error and call toast.error(...) on failure
so the user receives feedback, and keep the success toast on the happy path.
In `@packages/workspace-client/src/hooks/useFileTree/useFileTree.ts`:
- Around line 483-486: reveal 内のパス境界チェックで startsWith を使うと `/repo` が
`/repo-backup` にマッチしてしまうため、ルート外ディレクトリを誤展開する可能性があります；代わりに同ファイルの isWithinPath
関数を使って rootPath と absolutePath の包含関係を正確に判定し、条件分岐を startsWith ベースのチェックから
isWithinPath(rootPath, absolutePath) を使う形に置き換えてください（関数名: reveal, ユーティリティ:
isWithinPath）。
---
Nitpick comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.ts`:
- Around line 90-96: onStateChange currently calls getOrCreate which creates a
runtime/transport entry as a side effect; change it to check for an existing
entry and no-op if none: call the non-creating lookup (e.g., this.get(paneId) or
an equivalent method) instead of this.getOrCreate(paneId), only add the listener
to transport.stateListeners when a transport exists, and return a stable no-op
unsubscribe when there is no entry; keep references to onStateChange,
getOrCreate, and transport.stateListeners to locate and update the code.
In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts`:
- Around line 52-70: The empty catch blocks in persistBuffer, restoreBuffer, and
clearPersistedBuffer hide errors from localStorage/serialization; update each
catch to capture the error (e) and log a concise debug/warn message including
the function name, STORAGE_KEY_PREFIX/paneId, and the error details (e.g.
"persistBuffer failed for {paneId}: {error}"), keeping behavior unchanged in
production (log only in development or use console.debug/console.warn) so
failures are visible when debugging but do not crash the app.
In `@apps/desktop/src/renderer/lib/terminal/terminal-ws-transport.ts`:
- Around line 67-75: Parsed JSON is being asserted directly to
TerminalServerMessage and may lack required fields; after JSON.parse in the
socket.addEventListener callback, validate the parsed value before treating it
as TerminalServerMessage: ensure it's a non-null object, has a 'type' property
(and other required fields depending on message.type) and that message.type is a
string, or implement a small type guard/isValidTerminalServerMessage function to
perform these checks; if validation fails, write the invalid payload to terminal
and return instead of proceeding to use message.type. Reference symbols:
socket.addEventListener callback, variable message, and the
TerminalServerMessage type.
🪄 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: b930dc99-887d-4532-8fa1-07c561129aa5
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
apps/desktop/docs/EXTERNAL_FILES.mdapps/desktop/src/lib/trpc/routers/changes/git-operations.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers-claude-codex-opencode.tsapps/desktop/src/main/lib/agent-setup/agent-wrappers.test.tsapps/desktop/src/renderer/components/MarkdownRenderer/styles/default/default.cssapps/desktop/src/renderer/components/MarkdownRenderer/styles/tufte/tufte.cssapps/desktop/src/renderer/lib/terminal/terminal-runtime-registry.tsapps/desktop/src/renderer/lib/terminal/terminal-runtime.tsapps/desktop/src/renderer/lib/terminal/terminal-ws-transport.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/components/DashboardSidebarProjectSection/hooks/useDashboardSidebarProjectSectionActions/useDashboardSidebarProjectSectionActions.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/RightSidebar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/WorkspaceFilesSearchResultItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesSearchResultItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/WorkspaceFilesToolbar.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesToolbar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/components/WorkspaceFilesTreeItem/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/hooks/useWorkspaceFileSearch/useWorkspaceFileSearch.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/RightSidebar/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.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/index.tsapps/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/WorkspaceFilePreview.tsxapps/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/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/index.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/usePaneRegistry.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/components/GlobalTerminalLifecycle/GlobalTerminalLifecycle.tsxapps/desktop/src/renderer/routes/_authenticated/components/GlobalTerminalLifecycle/hooks/useGlobalTerminalLifecycle/index.tsapps/desktop/src/renderer/routes/_authenticated/components/GlobalTerminalLifecycle/hooks/useGlobalTerminalLifecycle/useGlobalTerminalLifecycle.tsapps/desktop/src/renderer/routes/_authenticated/components/GlobalTerminalLifecycle/index.tsapps/desktop/src/renderer/routes/_authenticated/layout.tsxapps/desktop/src/renderer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.tsapps/desktop/src/renderer/routes/_authenticated/settings/api-keys/components/ApiKeysSettings/ApiKeysSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/InviteMemberButton/InviteMemberButton.tsxapps/desktop/src/renderer/routes/_authenticated/settings/members/components/MembersSettings/components/MemberActions/MemberActions.tsxapps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/cloud/secrets/components/SecretsSettings/components/AddSecretSheet/AddSecretSheet.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/components/SessionSelectorItem/SessionSelectorItem.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsxapps/marketing/package.jsonpackage.jsonpackages/host-service/src/terminal/terminal.tspackages/panes/src/core/store/store.test.tspackages/panes/src/core/store/store.tspackages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsxpackages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsxpackages/panes/src/react/types.tspackages/ui/src/atoms/Alert/Alert.tsxpackages/ui/src/atoms/Alert/index.tspackages/workspace-client/src/hooks/useFileTree/useFileTree.ts
💤 Files with no reviewable changes (6)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/index.ts
- packages/panes/src/core/store/store.test.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/WorkspaceFilePreview.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilesPane/components/WorkspaceFilePreview/components/WorkspaceFilePreviewContent/WorkspaceFilePreviewContent.tsx
Comment on lines
+65
to
+84
| const scrollContainerRef = useRef<HTMLDivElement>(null); | ||
| const prevSelectedRef = useRef(selectedFilePath); | ||
|
|
||
| useEffect(() => { | ||
| if ( | ||
| selectedFilePath && | ||
| selectedFilePath !== prevSelectedRef.current && | ||
| rootPath | ||
| ) { | ||
| void fileTree.reveal(selectedFilePath).then(() => { | ||
| requestAnimationFrame(() => { | ||
| const el = scrollContainerRef.current?.querySelector( | ||
| `[data-filepath="${CSS.escape(selectedFilePath)}"]`, | ||
| ); | ||
| el?.scrollIntoView({ block: "center" }); | ||
| }); | ||
| }); | ||
| } | ||
| prevSelectedRef.current = selectedFilePath; | ||
| }, [selectedFilePath, rootPath, fileTree]); |
There was a problem hiding this comment.
初回表示で選択中ファイルを展開・スクロールできません。
prevSelectedRef を selectedFilePath で初期化しているため、sidebar を開いた時点で既に選択済みの file は Line 69-73 の条件が false になり、最初の reveal() が走りません。現在選択中の file を見せる用途なので、初回表示も同期したいです。
🛠️ 最小修正案
- const prevSelectedRef = useRef(selectedFilePath);
+ const prevSelectedRef = useRef<string | undefined>();🤖 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/RightSidebar/RightSidebar.tsx
around lines 65 - 84, The effect that calls fileTree.reveal never runs on first
render because prevSelectedRef is initialized to the current selectedFilePath;
change prevSelectedRef so it starts empty (e.g., useRef<string |
undefined>(undefined) or null) instead of useRef(selectedFilePath) so the
condition in the useEffect (selectedFilePath !== prevSelectedRef.current) will
be true on initial mount and trigger fileTree.reveal + the scrollIntoView logic
in the effect; update the declaration of prevSelectedRef and keep the rest of
the useEffect (and use of scrollContainerRef, selectedFilePath, rootPath,
fileTree) unchanged.
Comment on lines
+135
to
+143
| <WorkspaceFilesToolbar | ||
| isRefreshing={isRefreshing} | ||
| onCollapseAll={fileTree.collapseAll} | ||
| onNewFile={() => {}} | ||
| onNewFolder={() => {}} | ||
| onRefresh={() => void handleRefresh()} | ||
| onSearchChange={setSearchTerm} | ||
| searchTerm={searchTerm} | ||
| /> |
There was a problem hiding this comment.
新規ファイル/フォルダ操作が未実装のまま露出しています。
Line 138-139 が空ハンドラのままなので、toolbar 側がこれらの操作を表示しているとクリックしても何も起きません。配線できるまで無効化・非表示にするか、実処理を渡してください。
🤖 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/RightSidebar/RightSidebar.tsx
around lines 135 - 143, The WorkspaceFilesToolbar is passing empty handlers for
onNewFile and onNewFolder (in RightSidebar/RightSidebar.tsx) so the toolbar
shows controls that do nothing; either wire these to the existing creation flows
(e.g. call the workspace's createFile/createFolder functions or open the
create-file/create-folder modal handlers) by replacing the empty arrow functions
with the actual handlers, or disable/hide those toolbar buttons by passing a
disable prop or switching to onNewFile={undefined}/onNewFolder={undefined} (or a
showNewButtons=false flag) until the implementation exists. Locate the
WorkspaceFilesToolbar usage and update the onNewFile and onNewFolder props to
point to the appropriate functions (or disable the UI) rather than empty
handlers.
Comment on lines
+1
to
+3
| interface ExternalChangeBarProps { | ||
| onReload: () => Promise<void>; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and read its content
cat "apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/hooks/usePaneRegistry/components/FilePane/components/ExternalChangeBar/ExternalChangeBar.tsx"Repository: MocA-Love/superset
Length of output: 532
🏁 Script executed:
# Let's also check for similar event handler patterns in the codebase
rg "onClick.*void.*\(\)" apps/desktop --type ts --type tsx -A 2 -B 2 | head -50Repository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
# Check for error handling patterns in similar files
rg "onReload|handleReload" apps/desktop --type ts --type tsx -B 2 -A 5 | head -80Repository: MocA-Love/superset
Length of output: 89
🏁 Script executed:
# Search for event handler patterns with async/await in the codebase
rg "onClick.*async" apps/desktop --max-count 20 -B 1 -A 2Repository: MocA-Love/superset
Length of output: 663
🏁 Script executed:
# Search for event handlers with .catch() pattern
rg "onClick.*\.catch" apps/desktop --max-count 20 -B 1 -A 2Repository: MocA-Love/superset
Length of output: 44
🏁 Script executed:
# Check if there's a global promise rejection handler setup
rg "unhandledrejection|onunhandledrejection" apps/desktop --max-count 10 -B 2 -A 2Repository: MocA-Love/superset
Length of output: 839
🏁 Script executed:
# Look for similar patterns with void in event handlers
rg "void.*\(\)" apps/desktop --max-count 30 -B 1 -A 1Repository: MocA-Love/superset
Length of output: 50374
🏁 Script executed:
# Search for similar file reload or refresh patterns with error handling
rg "reload|onReload" apps/desktop --type ts -A 5 -B 2 | grep -A 5 "catch\|try"Repository: MocA-Love/superset
Length of output: 20530
🏁 Script executed:
# Check parent component and how onReload is passed
rg "onReload" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace --type ts -B 3 -A 1Repository: MocA-Love/superset
Length of output: 9973
🏁 Script executed:
# Check what the global unhandled rejection handler actually does
cat apps/desktop/src/renderer/lib/boot-errors.tsRepository: MocA-Love/superset
Length of output: 2933
onReload() の reject が未処理です。
void onReload() は Promise を待たないだけで reject を吸収しないため、リロード失敗時に unhandled rejection になります。アプリマウント後のユーザーアクション（onClick）では global rejection handler の保護外となるため、ここで明示的に try/catch で処理してください。
修正案
export function ExternalChangeBar({ onReload }: ExternalChangeBarProps) {
+ const handleReload = async () => {
+ try {
+ await onReload();
+ } catch (error) {
+ console.error("[ExternalChangeBar] Failed to reload file", error);
+ }
+ };
+
return (
<div className="flex items-center gap-2 border-b border-border bg-warning/10 px-3 py-1.5 text-xs text-warning-foreground">
<span>File changed on disk.</span>
<button
type="button"
className="underline hover:no-underline"
- onClick={() => void onReload()}
+ onClick={() => void handleReload()}
>
Reload
</button>
</div>
);
}🤖 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/ExternalChangeBar/ExternalChangeBar.tsx
around lines 1 - 3, ExternalChangeBarProps の onReload が呼ばれる箇所（例:
ExternalChangeBar の onClick ハンドラ）で onReload() の返す Promise
を放置しており拒否が未処理になっているため、onReload() 呼び出しを明示的に await し try/catch で囲んで拒否を処理してください;
具体的には ExternalChangeBar コンポーネント内の onClick / リロードトリガーで await onReload() を実行し
catch でエラーログ出力またはユーザー向けのエラーフィードバックを行うように修正してください。
Comment on lines
+24
to
+43
| const currentContentRef = useRef(content); | ||
| const [savedContent, setSavedContent] = useState(content); | ||
|
|
||
| // Track the initial/saved content to detect dirty state | ||
| if (content !== savedContent && !onDirtyChange) { | ||
| setSavedContent(content); | ||
| } | ||
|
|
||
| const handleChange = useCallback( | ||
| (value: string) => { | ||
| currentContentRef.current = value; | ||
| onDirtyChange(value !== savedContent); | ||
| }, | ||
| [onDirtyChange, savedContent], | ||
| ); | ||
|
|
||
| const handleSave = useCallback(async () => { | ||
| await onSave(currentContentRef.current); | ||
| setSavedContent(currentContentRef.current); | ||
| }, [onSave]); |
There was a problem hiding this comment.
保存スナップショットが content と保存結果に追従していません。
Line 28 の !onDirtyChange は必須 props なので実質到達せず、currentContentRef / savedContent は reload や content 差し替え後も古いまま残ります。その状態で Line 40-43 が無条件に snapshot を進めるため、古い内容を再保存したり、保存失敗後に dirty 判定を崩したりします。
🛠️ 修正イメージ
-import { useCallback, useRef, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
@@
- // Track the initial/saved content to detect dirty state
- if (content !== savedContent && !onDirtyChange) {
- setSavedContent(content);
- }
+ useEffect(() => {
+ currentContentRef.current = content;
+ setSavedContent(content);
+ }, [content]);あわせて、Line 40-43 の snapshot 更新は onSave の成功時だけに限定したいです。
🤖 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/renderers/CodeRenderer/CodeRenderer.tsx
around lines 24 - 43, The code fails to keep currentContentRef and savedContent
in sync with prop content and updates the saved snapshot unconditionally; add a
useEffect that watches content and sets currentContentRef.current = content and
setSavedContent(content) so both follow external replacements, change
handleChange to guard onDirtyChange (if present) before calling it, and update
handleSave to only setSavedContent(currentContentRef.current) and call
onDirtyChange(false) after a successful onSave (wrap await onSave in try/catch
and only mutate savedContent/onDirtyChange on success) — reference symbols:
currentContentRef, savedContent, handleChange, handleSave.
Comment on lines
+103
to
107
| "/terminal/:paneId", | ||
| upgradeWebSocket((c) => { | ||
| const workspaceId = c.req.param("workspaceId"); | ||
| const workspace = workspaceId | ||
| ? db.query.workspaces | ||
| .findFirst({ where: eq(workspaces.id, workspaceId) }) | ||
| .sync() | ||
| : null; | ||
|
|
||
| let terminal: IPty | null = null; | ||
| let disposed = false; | ||
|
|
||
| const disposeTerminal = () => { | ||
| if (disposed) { | ||
| return; | ||
| } | ||
| disposed = true; | ||
| terminal?.kill(); | ||
| terminal = null; | ||
| }; | ||
| const paneId = c.req.param("paneId"); | ||
| const workspaceId = c.req.query("workspaceId") ?? null; | ||
|
|
There was a problem hiding this comment.
セッションキーを paneId だけにすると誤再接続の余地が残ります。
existing 分岐は workspaceId を見ないため、paneId がワークスペースや別ライフタイムで再利用されると別 PTY にアタッチしてしまいます。セッションに workspaceId を保持して再接続時に一致確認するか、キー自体を workspaceId + paneId にした方が安全です。
Also applies to: 119-143, 185-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/terminal/terminal.ts` around lines 103 - 107, The
session key currently uses only paneId (from c.req.param("paneId")), which
allows cross-workspace reconnection; change the session lookup/creation to
include workspaceId (from c.req.query("workspaceId"))—e.g., form a composite key
like `${workspaceId}:${paneId}` when indexing sessions OR store workspaceId on
the session object when creating it and in the existing-branch verify
session.workspaceId === workspaceId before reattaching; update all places that
read/write the session map (the upgradeWebSocket handler around the existing
branch and the reconnect/attach logic referenced at lines ~119-143 and ~185-196)
to use the composite key or the workspaceId equality check so reconnections
cannot attach to a PTY from a different workspace/lifetime.
Comment on lines
+205
to
217
| pty.onExit(({ exitCode, signal }) => { | ||
| session.exited = true; | ||
| session.exitCode = exitCode ?? 0; | ||
| session.exitSignal = signal ?? 0; | ||
|
|
||
| if (session.socket?.readyState === 1) { | ||
| sendMessage(session.socket, { | ||
| type: "exit", | ||
| exitCode: session.exitCode, | ||
| signal: session.exitSignal, | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
終了済みセッションの回収が dispose 依存です。
onExit では sessions から消していないので、renderer クラッシュや未送信の dispose があると終了済みセッションが無期限に残ります。未接続かつ終了済みのセッションは TTL 付きで回収する経路を入れておきたいです。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/host-service/src/terminal/terminal.ts` around lines 205 - 217, The
pty.onExit handler currently marks session.exited but never removes the session
from sessions, causing zombie sessions if dispose isn't called; update the
handler (pty.onExit) to schedule TTL-based eviction for sessions that are both
exited and not connected: set a per-session eviction timer (e.g.,
session.evictionTimer) when exit occurs and session.socket?.readyState !== 1
that removes the session from the sessions collection after a configurable TTL;
ensure any existing evictionTimer is cleared if the socket reconnects or when
dispose() runs (dispose should clear session.evictionTimer and immediately
delete from sessions), and keep sendMessage behavior unchanged for connected
sockets.
Comment on lines
+64
to
+70
| onClick={onClick} | ||
| onAuxClick={(e) => { | ||
| if (e.button === 1 && onMiddleClick) { | ||
| e.preventDefault(); | ||
| onMiddleClick(); | ||
| } | ||
| }} |
There was a problem hiding this comment.
ヘッダー配下のボタン操作でも onClick / onMiddleClick が発火します。
このままだと actionsContent や toolbar 内のクリックがバブリングして、閉じる・メニュー操作とヘッダー操作が二重実行されます。ヘッダー本体だけを反応させるか、子のインタラクティブ要素を除外した方が安全です。
修正例
- onClick={onClick}
- onAuxClick={(e) => {
- if (e.button === 1 && onMiddleClick) {
- e.preventDefault();
- onMiddleClick();
- }
- }}
+ onClick={(e) => {
+ const target = e.target as HTMLElement;
+ if (
+ e.target !== e.currentTarget &&
+ target.closest("button,a,input,select,textarea,[role='button']")
+ ) {
+ return;
+ }
+ onClick?.();
+ }}
+ onAuxClick={(e) => {
+ const target = e.target as HTMLElement;
+ if (
+ e.target !== e.currentTarget &&
+ target.closest("button,a,input,select,textarea,[role='button']")
+ ) {
+ return;
+ }
+ if (e.button === 1 && onMiddleClick) {
+ e.preventDefault();
+ onMiddleClick();
+ }
+ }}📝 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
| onClick={onClick} | |
| onAuxClick={(e) => { | |
| if (e.button === 1 && onMiddleClick) { | |
| e.preventDefault(); | |
| onMiddleClick(); | |
| } | |
| }} | |
| onClick={(e) => { | |
| const target = e.target as HTMLElement; | |
| if ( | |
| e.target !== e.currentTarget && | |
| target.closest("button,a,input,select,textarea,[role='button']") | |
| ) { | |
| return; | |
| } | |
| onClick?.(); | |
| }} | |
| onAuxClick={(e) => { | |
| const target = e.target as HTMLElement; | |
| if ( | |
| e.target !== e.currentTarget && | |
| target.closest("button,a,input,select,textarea,[role='button']") | |
| ) { | |
| return; | |
| } | |
| if (e.button === 1 && onMiddleClick) { | |
| e.preventDefault(); | |
| onMiddleClick(); | |
| } | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsx`
around lines 64 - 70, Header-level click handlers (onClick and onAuxClick) are
firing for clicks inside child interactive areas like actionsContent and toolbar
due to event bubbling; update the PaneHeader's handlers to only invoke
onClick/onMiddleClick when the event originates on the header element itself
(e.g., check event.target === event.currentTarget) so clicks on children are
ignored, and keep the existing e.preventDefault() behavior for middle-clicks
when calling onMiddleClick; locate the handlers on the PaneHeader component (the
onClick and onAuxClick props) and add this event-origin guard to both.
Comment on lines
+84
to
+90
| close: async () => { | ||
| if (definition?.onBeforeClose) { | ||
| const allowed = await definition.onBeforeClose(pane); | ||
| if (!allowed) return; | ||
| } | ||
| store.getState().closePane({ tabId: tab.id, paneId: pane.id }); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx | sed -n '75,100p'Repository: MocA-Love/superset
Length of output: 1057
🏁 Script executed:
cat -n packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx | sed -n '220,235p'Repository: MocA-Love/superset
Length of output: 668
🏁 Script executed:
grep -n "PaneHeader" packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx -A 5 -B 5Repository: MocA-Love/superset
Length of output: 1759
🏁 Script executed:
fd "PaneHeader" packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Repository: MocA-Love/superset
Length of output: 275
🏁 Script executed:
wc -l packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsxRepository: MocA-Love/superset
Length of output: 178
🏁 Script executed:
cat packages/panes/src/react/components/Workspace/components/Tab/components/Pane/components/PaneHeader/PaneHeader.tsxRepository: MocA-Love/superset
Length of output: 2026
onBeforeClose の例外がハンドルされていません。
actions.close は async 関数なのに PaneHeader の onMiddleClick ハンドラに直接渡され、イベント発火時に await なしで呼び出されます（line 68）。definition.onBeforeClose() が reject すると unhandled rejection になります。例外を吸収するか、呼び出し側で async/await 対応にしてください。
修正案
- close: async () => {
- if (definition?.onBeforeClose) {
- const allowed = await definition.onBeforeClose(pane);
- if (!allowed) return;
- }
- store.getState().closePane({ tabId: tab.id, paneId: pane.id });
- },
+ close: () => {
+ void (async () => {
+ try {
+ if (definition?.onBeforeClose) {
+ const allowed = await definition.onBeforeClose(pane);
+ if (!allowed) return;
+ }
+ store.getState().closePane({ tabId: tab.id, paneId: pane.id });
+ } catch (error) {
+ console.error("[Pane] Failed to close pane", error);
+ }
+ })();
+ },Also applies to: 228-228
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/panes/src/react/components/Workspace/components/Tab/components/Pane/Pane.tsx`
around lines 84 - 90, The close action passes through an unhandled promise from
definition.onBeforeClose, which can cause unhandled rejections when
actions.close is called (e.g., via PaneHeader.onMiddleClick). Wrap the call to
definition.onBeforeClose(pane) inside a try/catch in the close async function
(and the duplicate at the other occurrence) so any rejection is caught; on error
log or swallow the error and treat it as a denied close (return early),
otherwise continue to call store.getState().closePane({ tabId: tab.id, paneId:
pane.id }); thereby ensuring onBeforeClose exceptions are handled synchronously
by the action.
Comment on lines
25
to
29
| type AlertOptions = { | ||
| title: string; | ||
| description: string; | ||
| confirmText?: string; | ||
| cancelText?: string; | ||
| onConfirm: () => void | Promise<void>; | ||
| onCancel?: () => void; | ||
| }; | ||
|
|
||
| type InternalAlertOptions = AlertOptions & { | ||
| variant: "default" | "destructive"; | ||
| actions: AlertAction[]; | ||
| }; |
There was a problem hiding this comment.
dismiss 時に呼び出し元の待機が解けません。
Line 56-58 は dialog を閉じるだけで action も callback も実行しないので、今回の onBeforeClose / onBeforeCloseTab のように alert() を Promise<boolean> で包んでいる呼び出し元は Esc・外側クリックで永久に pending のままになります。dismiss を明示的に通知する API を追加するか、この dialog は dismiss 不可にした方が安全です。
🛠️ 修正イメージ
type AlertOptions = {
title: string;
description: string;
actions: AlertAction[];
+ onDismiss?: () => void;
};
@@
const handleClose = () => {
setIsOpen(false);
+ alertOptions?.onDismiss?.();
};
@@
- onOpenChange={(open) => !open && handleClose()}
+ onOpenChange={(open) => {
+ if (!open && loadingIndex === null) {
+ handleClose();
+ }
+ }}Also applies to: 56-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui/src/atoms/Alert/Alert.tsx` around lines 25 - 29, The alert dialog
allows dismiss via Esc/outside click but the dismiss path only closes the dialog
and does not call any action or resolve the Promise returned by alert(), leaving
callers of onBeforeClose/onBeforeCloseTab pending; update the Alert
component/alert() flow so that dismiss explicitly notifies the caller: either
(preferred) add and invoke an onDismiss (or include a special AlertAction with
type "dismiss") so alert() resolves (e.g., resolves false or a distinct dismiss
value) when user dismisses, OR make the dialog non-dismissible by disabling
escape/outside-close in the Alert/dialog implementation; locate the Alert
component, the alert() wrapper and the onBeforeClose/onBeforeCloseTab callsites
and implement one of these fixes so Promises always settle on dismiss.
- Clear status cache after git fetch --prune so ahead/behind reflects immediately - Call onRefresh() after successful fetch to sync branch/PR display - Show toast error on fetch failure instead of silently ignoring
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx (1)
1151-1152: エラーメッセージに具体的な内容を含めることを検討してください。このファイル内の他の toast エラー（例: Line 483 の
Failed to switch branch: ${msg}、Line 516 のFailed to create branch: ${message}）では具体的なエラー内容を表示していますが、FetchRemoteButtonでは汎用メッセージのみを表示しています。ネットワークエラーや認証エラーなどの原因をユーザーに伝えることで、トラブルシューティングが容易になります。♻️ 修正案
- } catch { - toast.error("Failed to fetch remote"); + } catch (error) { + toast.error( + error instanceof Error ? error.message : "Failed to fetch remote", + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx` around lines 1151 - 1152, The catch block in the FetchRemoteButton handler currently shows a generic toast ("Failed to fetch remote"); update the try/catch to capture the thrown error (e.g., catch (err) or catch (error)), extract a human-readable message (err.message or a fallback), and pass it into toast.error (e.g., "Failed to fetch remote: ${message}") so the UI shows the specific error like the other handlers (see the existing patterns at the branch switch/create handlers in ChangesHeader).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx`:
- Around line 1151-1152: The catch block in the FetchRemoteButton handler
currently shows a generic toast ("Failed to fetch remote"); update the try/catch
to capture the thrown error (e.g., catch (err) or catch (error)), extract a
human-readable message (err.message or a fallback), and pass it into toast.error
(e.g., "Failed to fetch remote: ${message}") so the UI shows the specific error
like the other handlers (see the existing patterns at the branch switch/create
handlers in ChangesHeader).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48a8465a-9cc7-4112-bd9e-65640c25b8b4
📒 Files selected for processing (2)
apps/desktop/src/lib/trpc/routers/changes/git-operations.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/ChangesView/components/ChangesHeader/ChangesHeader.tsx
This was referenced Apr 12, 2026
This was referenced Apr 23, 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.
Summary
merge/upstream-main-2026-04-03fstreampatch/override) while adopting upstream trusted dependenciesNotes
package.json,bun.lock, and the v2 workspace file preview pathWorkspaceFilePreviewContentto the newFilePaneSummary by CodeRabbit
リリースノート
新機能
改善