feat(desktop): DnD / Open With でファイルを開けるようにする#412
Merged
Conversation
Finder / Explorer からのダブルクリック、Dock ドロップ、ウィンドウ への DnD に対応する。ドロップされたパスを main プロセス側で分類し: - 登録済みワークスペース配下のファイル → そのワークスペースに切替えて FileViewerPane として開く (Q5:A に対応し複数ファイル同時ドロップ可) - 未登録のファイル / フォルダ → 新設の `/scratch` ルートでエディタ だけの軽量モードで開く (Git / Agent / Chat / Terminal は無効) Scratch タブは永続化せず (Q1:B)、ルート URL にもパスを載せないので 再起動や localStorage に残るルータ履歴で復元されない。 実装: - `main/lib/file-intake`: パス分類・キューイング・dispatchPaths - `main/index.ts`: will-finish-launching + open-file, second-instance の argv 経由のファイル渡し、did-finish-load 後に cold-start を drain - `lib/trpc/routers/scratch`: ワークスペース外でのファイル read/write と DnD 取り込み。書き込みは /etc, ~/.ssh 等の deny list と symlink 拒否 - `renderer/lib/file-intake-client`: ウィンドウ DnD (bubble 相 + defaultPrevented ガードで既存ドロップゾーンを壊さない) と IPC バッチの受信 - `renderer/screens/scratch/ScratchView`: タブ + CodeEditor の最小UI - `renderer/routes/.../scratch/page.tsx`: 新規ルート - `_dashboard/layout.tsx`: scratch 時はワークスペースサイドバー非表示 - `electron-builder.ts`: fileAssociations を追加 (macOS は Alternate rank)
📝 WalkthroughWalkthroughElectron デスクトップアプリケーションに包括的なスクラッチファイル機能を追加します。ファイル関連付け登録、tRPC エンドポイント、OS レベルのファイルオープンイベント処理、ドラッグアンドドロップ対応、UI コンポーネント、ファイル状態管理ストアを実装します。 Changes
Sequence Diagram(s)sequenceDiagram
participant OS as OS / ユーザー
participant Main as メインプロセス
participant Router as ファイル取込ルーター
participant DB as ローカル DB
participant IPC as IPC チャネル
participant Renderer as レンダラー
participant Router2 as TanStack Router
OS->>Main: ファイルをダブルクリック / 開く (macOS)
Main->>Main: open-file イベントリスナー<br/>(will-finish-launching)
Main->>Main: パスをキューに追加
Main->>Main: appReady 時に dispatchFileIntakePaths 呼び出し
Main->>Router: classifyPaths(absolutePaths)
Router->>DB: ワークスペース根を取得 (worktree + branch)
DB-->>Router: 根リスト
Router->>Router: パス正規化 & シンボリックリンク解決
Router->>Router: ワークスペース or スクラッチ に分類
Router-->>Main: FileIntakeTarget[]
Main->>IPC: file-intake:open-workspace-batch<br/>or<br/>file-intake:open-scratch-batch
IPC->>Renderer: IPC メッセージ受信
Renderer->>Router2: navigate(/workspace/$id) or navigate(/scratch)
Renderer->>Renderer: useScratchTabsStore.openPaths() など
Renderer->>Renderer: UI 更新
sequenceDiagram
participant User as ユーザー
participant Browser as ブラウザ / DOM
participant FileIntakeClient as ファイル取込クライアント
participant TRPC as tRPC クライアント
participant Main as メインプロセス
participant ScratchRouter as スクラッチルーター
participant FS as ファイルシステム
User->>Browser: ファイルを window にドラッグ
Browser->>FileIntakeClient: dragover イベント
FileIntakeClient->>FileIntakeClient: Files を検出 → デフォルト防止
User->>Browser: ファイルをドロップ
Browser->>FileIntakeClient: drop イベント
FileIntakeClient->>FileIntakeClient: window.webUtils.getPathForFile()<br/>で絶対パス抽出
FileIntakeClient->>TRPC: scratch.ingestDroppedPaths.mutate()
TRPC->>Main: tRPC IPC 経由で実行
Main->>ScratchRouter: ingestDroppedPaths ハンドラー
ScratchRouter->>ScratchRouter: パス正規化 & サニタイズ
ScratchRouter->>Main: dispatchPaths() 呼び出し
Main->>FileIntakeClient: IPC file-intake:open-scratch-batch
FileIntakeClient->>FileIntakeClient: useScratchTabsStore.openPaths()
FileIntakeClient->>FileIntakeClient: navigate(/scratch)
sequenceDiagram
participant Editor as ScratchEditor
participant TRPC as tRPC クライアント
participant Main as メインプロセス
participant ScratchRouter as スクラッチルーター
participant FS as ファイルシステム
participant UI as UI 更新
Editor->>TRPC: readFile(absolutePath) クエリ
TRPC->>Main: メイン側で実行
Main->>ScratchRouter: readFile ハンドラー
ScratchRouter->>FS: fs.stat() 検証
ScratchRouter->>FS: fs.readFile() (max 5MB)
ScratchRouter-->>TRPC: { content, byteSize }
TRPC-->>Editor: キャッシュ済みデータ
Editor->>UI: 初期化 & コンテンツ表示
User->>Editor: テキスト編集
Editor->>UI: draft 更新
User->>Editor: 保存ボタンをクリック
Editor->>TRPC: writeFile(absolutePath, content) ミューテーション
TRPC->>Main: メイン側で実行
Main->>ScratchRouter: writeFile ハンドラー
ScratchRouter->>FS: 親ディレクトリを実パスに解決
ScratchRouter->>FS: システムディレクトリ deny-list チェック
ScratchRouter->>FS: fs.writeFile(UTF-8)
ScratchRouter-->>TRPC: { content, byteSize, mtime }
TRPC-->>Editor: 保存完了
Editor->>UI: savedAt 更新 & ステータス表示
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
electron-builder の Linux AppImage パーサーは FileAssociation.ext に 文字列しか受け付けないため、配列指定だと AppImage ビルドが失敗する (`ReadString: expects " or n, but found [`)。 拡張子ごとに 1 エントリ生成するよう変更。macOS / Windows は同じ形式でも 動くので、プラットフォーム別に分岐する必要はない。
![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: 6ea3a93cf9
ℹ️ 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
+126
to
+128
| let lstat: Awaited<ReturnType<typeof fs.lstat>> | null = null; | ||
| try { | ||
| lstat = await fs.lstat(abs); |
There was a problem hiding this comment.
The symlink safeguard only checks lstat(abs) on the final path string, so it misses cases where a parent directory is a symlink into a protected location. In that scenario (for example, writing through /tmp/link/... where link points into ~/.ssh or /etc), both the deny-list and symlink checks pass and writeFile will still modify the protected target. Resolve and validate the real path (or every path component) before enforcing deny rules.
Useful? React with 👍 / 👎.
Comment on lines
+167
to
+171
| if (t.kind === "workspace-file") { | ||
| const key = t.workspaceId; | ||
| const arr = byWorkspace.get(key) ?? []; | ||
| arr.push(t.absolutePath); | ||
| byWorkspace.set(key, arr); |
There was a problem hiding this comment.
classifyPath preserves isDirectory, but splitTargets drops that metadata and pushes all workspace targets as file paths. When a user drops a registered workspace folder, the renderer tries to open that directory via addFileViewerPane, which produces a directory-error tab instead of only switching to the workspace as intended for folder drops.
Useful? React with 👍 / 👎.
P1 — scratch writeFile の symlink ガードが親ディレクトリ経由の エスケープを見逃していた問題。`lstat(abs)` だけだと `/tmp/link/file` (link は ~/.ssh 等のシンボリックリンク) の場合に deny list も symlink チェックも素通りしてしまう。 fs.realpath で親ディレクトリを正規化してから basename を再結合し、 canonical パスに対して deny list / lstat を適用するよう変更。 P2 — 登録済みワークスペースフォルダを DnD したとき、splitTargets が isDirectory フラグを落として addFileViewerPane にディレクトリパスを 渡してしまい、エラータブが開く問題。 - splitTargets で directory は absolutePaths から除外 - renderer の handleWorkspaceBatch は paths.length === 0 でも必ず ワークスペースに navigate するよう変更 (ディレクトリのみドロップ時 の「ワークスペースを開く」挙動を維持)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (9)
apps/desktop/src/main/lib/file-intake/index.ts (2)
190-198:isDestroyed()ガードの追加を推奨
BrowserWindow.getAllWindows()が返すウィンドウは破棄済みの可能性があり、isMinimized()/show()/focus()呼び出しで例外が発生し得ます。防御的に先頭ウィンドウを破棄済みでない最初のインスタンスに絞ると安全です。🔧 Proposed fix
export function focusFirstWindow(): BrowserWindow | null { - const wins = BrowserWindow.getAllWindows(); - if (wins.length === 0) return null; - const main = wins[0]; + const main = BrowserWindow.getAllWindows().find((w) => !w.isDestroyed()); + if (!main) return null; if (main.isMinimized()) main.restore(); main.show(); main.focus(); return main; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/file-intake/index.ts` around lines 190 - 198, The focusFirstWindow function currently assumes the first window from BrowserWindow.getAllWindows() is valid; change it to pick the first non-destroyed window by filtering getAllWindows() with !win.isDestroyed() (or find the first where isDestroyed() returns false), then operate on that window: check isMinimized(), restore if needed, then show() and focus(), and return it; if no non-destroyed window exists return null. Ensure you reference focusFirstWindow and BrowserWindow.getAllWindows()/isDestroyed() when making the change.
252-268: tRPC の observable パターンでのリファクタリングを検討
win.webContents.send("file-intake:open-workspace-batch", ...)および"file-intake:open-scratch-batch"の生の IPC 送信は、コーディングガイドライン「Electron プロセス間通信にはsrc/lib/trpcで定義された tRPC を常に使用する」に違反しています。codebase 内では
apps/desktop/src/lib/trpc/routers/でobservable<T>((emit) => {...})パターンが既に確立されており、workspaces/init、vibrancy、service-status、terminalなど複数の router で使用されています。この file-intake module も同じ observable パターンで subscription を定義し、renderer 側から購読する形にリファクタすると、tRPC に統一できます。ただし
webContents.send()は codebase の他の箇所（window-manager、deep-link-navigate など）でも広く使用されているため、本 PR のスコープ外として別途で一括対応するのも現実的です。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/lib/file-intake/index.ts` around lines 252 - 268, The current direct IPC sends in the file-intake flow (inside the loop that uses splitTargets and the two win.webContents.send calls) must be replaced with tRPC observable events: create a new tRPC router (e.g., file-intake or fileIntakeRouter) exporting observable endpoints for the two event types (openWorkspaceBatch and openScratchBatch) that emit the same payloads (workspaceId + absolutePaths and absolutePaths for scratch), and from this module replace the win.webContents.send calls with invocations that emit to those observables (use the same emit API shape used by other routers like workspaces/init, vibrancy, service-status, terminal) so the renderer can subscribe via trpc client; keep splitTargets and the payload shapes unchanged and ensure the observable emits are called in the same places where win.webContents.send currently runs.apps/desktop/src/main/index.ts (2)
636-646:second-instanceのエラーハンドリングを追加推奨
filterFileIntakeArgs/dispatchFileIntakePathsが reject しても、second-instance ハンドラには catch がなく、Electron の unhandledRejection(同ファイル L518)にフォールバックして Sentry にのるだけになります。ユーザー操作起点の分岐なので、console.errorと明示 report をここでも行うと、did-finish-loadフロー(L873)と対称になって運用上追いやすくなります。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 636 - 646, The second-instance handler should guard against rejections from async calls: wrap the awaits for processDeepLink(url), filterFileIntakeArgs(argv), and dispatchFileIntakePaths(paths) inside a try/catch block in the app.on("second-instance", ...) callback (the block containing findDeepLinkInArgv, processDeepLink, filterFileIntakeArgs, dispatchFileIntakePaths); on error call console.error with a clear context message and invoke the same error reporting path you use elsewhere (e.g., Sentry/report function) so failures are handled locally rather than falling through to unhandledRejection, mirroring the did-finish-load flow's reporting behavior.
857-892:did-finish-load待機のフォールバック分岐が綺麗に設計されていますが、ウィンドウ破棄時の取り逃しに注意
firstWindow.webContents.isLoading()分岐とbrowser-window-createdフォールバックの 2 系統で readiness を待つのは適切です。ただし、まれに「ロード中に最初のウィンドウが閉じられる(=did-finish-loadが発火しない)」ケースではmarkFileIntakeReady()が永遠に呼ばれず、キューが滞留する可能性があります。新規ウィンドウ生成時に readiness 未設定であれば改めて待機する、あるいは一定時間でフォールバック発火させておくと安全です。通常フローでは問題にならないと思われるため、v1 以降の改善で構いません。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/main/index.ts` around lines 857 - 892, The current did-finish-load wait can hang if the first window is closed while still loading; update the firstWindow branch to detect that case and fallback: when webContents.isLoading() attach both a did-finish-load handler and a one-time 'closed'/'destroyed' handler on firstWindow (or a short timeout) that, if markFileIntakeReady hasn't been called, will either re-arm the existing app.once('browser-window-created') logic or invoke onRendererReady after a bounded delay; reference firstWindow, onRendererReady, markFileIntakeReady, webContents.isLoading(), and the app.once('browser-window-created') fallback when implementing this change.apps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEmpty/ScratchEmpty.tsx (1)
17-24: コメントの「synchronously on mount」はやや不正確です
useEffectは commit 後(= 初回ペイント後)に実行されるため、厳密には「同期的に」ではありません。実害はaria-hiddenな空 div で隠蔽されているため問題ありませんが、より確実に初回ペイント前にリダイレクトしたい場合はuseLayoutEffectへの置換も検討できます(ただし本件は実挙動として十分軽量なので任意)。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEmpty/ScratchEmpty.tsx` around lines 17 - 24, The comment saying the redirect runs "synchronously on mount" is inaccurate because the current useEffect runs after commit (post-paint); update the comment text to reflect that (e.g., "runs after commit / after initial paint") or, if you need the redirect to run before paint, replace the useEffect with useLayoutEffect in the ScratchEmpty component so navigate({ to: "/workspace", replace: true }) executes prior to the first paint (keep the same dependencies: [navigate, redirectToWorkspaceOnEmpty]); reference useEffect/useLayoutEffect, navigate, and redirectToWorkspaceOnEmpty when making the change.apps/desktop/src/renderer/screens/scratch/ScratchView/utils/path.ts (1)
1-5: 末尾にセパレータを含むパスで空文字が返る点に留意。
"foo/bar/"のような末尾セパレータ付きの入力ではpartsの末尾が""となり、""は nullish ではないため?? pのフォールバックが効かず空文字が返ります。v1 では OS 由来の絶対ファイルパスが渡る前提なので実害は小さいですが、将来ディレクトリパスなどが流入した場合にタブのラベルが空になり得ます。♻️ 末尾セパレータを除去してから分割する案
export function basename(p: string): string { if (!p) return ""; - const parts = p.split(/[\\/]/); - return parts[parts.length - 1] ?? p; + const trimmed = p.replace(/[\\/]+$/, ""); + const parts = trimmed.split(/[\\/]/); + return parts[parts.length - 1] || p; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/scratch/ScratchView/utils/path.ts` around lines 1 - 5, The basename function returns an empty string for paths ending with a separator (e.g., "foo/bar/"); update basename to ignore trailing slashes by trimming any trailing '/' or '\' before splitting (or split and pick the last non-empty segment) so that basename(p: string) returns the final path segment even for directory-like inputs; adjust the implementation in function basename to remove trailing separators or filter empty parts and then return the last non-empty part (still falling back to p if something unexpected occurs).apps/desktop/src/renderer/lib/file-intake-client/index.ts (3)
5-12:typeよりinterfaceがガイドライン準拠です
WorkspaceBatchPayload/ScratchBatchPayloadはオブジェクト型なので、IpcRendererAPI/NavRouterと同様にinterface宣言に揃えると一貫します（任意対応）。提案 diff
-type WorkspaceBatchPayload = { - workspaceId: string; - absolutePaths: string[]; -}; - -type ScratchBatchPayload = { - absolutePaths: string[]; -}; +interface WorkspaceBatchPayload { + workspaceId: string; + absolutePaths: string[]; +} + +interface ScratchBatchPayload { + absolutePaths: string[]; +}As per coding guidelines: "Prefer
interfacefor defining object shapes in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/file-intake-client/index.ts` around lines 5 - 12, Replace the two object type aliases WorkspaceBatchPayload and ScratchBatchPayload with interface declarations to follow project conventions (aligning with other interfaces like IpcRendererAPI and NavRouter); update the declarations for WorkspaceBatchPayload and ScratchBatchPayload to use the interface keyword and keep the same property names and types so callers and exports remain unchanged.
85-92: main→renderer 通知は tRPC subscription の方がガイドライン準拠です
file-intake:open-workspace-batch/file-intake:open-scratch-batchは main→renderer の一方向通知ですが、本プロジェクトのガイドライン・既存ラーニングでは Electron IPC はsrc/lib/trpc経由で統一する方針になっています。trpc-electronでは observable サブスクリプションとしてscratch.onIngestBatch: publicProcedure.subscription(() => observable<Batch>((emit) => { ... return unsubscribe; }) )のように main 側で
emit.next(payload)する形にまとめられるため、型情報とテアダウンがそのまま renderer に伝搬し、IpcRendererAPIのunknownキャストも不要になります。v1 スコープ次第では許容ですが、将来的に tRPC へ寄せるチケットだけでも残しておくことを推奨します。As per coding guidelines: "For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc" / "For tRPC subscriptions in trpc-electron, ALWAYS use the observable pattern withobservable<T>((emit) => {...})".Also applies to: 146-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/file-intake-client/index.ts` around lines 85 - 92, Replace the direct ipcRenderer listeners for "file-intake:open-workspace-batch" and "file-intake:open-scratch-batch" (the ipcRenderer?.on(...) registrations that call handleWorkspaceBatch and handleScratchBatch) with tRPC subscription-based notifications: implement corresponding publicProcedure.subscriptions on the main side (e.g., scratch.onIngestBatch) that use observable<Batch>((emit) => { emit.next(payload); return unsubscribe; }), and consume those subscriptions from the renderer via src/lib/trpc instead of ipcRenderer so you retain types, teardown, and remove the unknown casts around IpcRendererAPI; update the consumer code that currently calls handleWorkspaceBatch/handleScratchBatch to subscribe to the new trpc observable endpoints.
94-110: フォルダドロップ時の動作検証をテストチェックリストに追加しておくと安心ですFinder/Explorer からフォルダをドロップした場合、Chromium は自動的にディレクトリを展開し、
event.dataTransfer.filesには、ドロップされたフォルダ内の全ファイル（再帰的）の File オブジェクトが含まれます。フォルダ自体の単一パスが返されるのではなく、展開されたファイル群の個別パスがgetPathForFileで得られます。現在のコード実装（extractDroppedPaths）はこの展開後のファイル群を正しく処理しており、PR の分類ロジック（main 側でワークスペースかどうかを DB 照合）と合わせて適切に機能します。ただしディレクトリドロップ時の挙動はプラットフォーム固有の差異や API バージョンの変更が起きやすいため、手動テストチェックリストに以下を追加しておくと事故が減ります：
- macOS / Windows / Linux 各プラットフォームでフォルダドロップが正しく複数ファイルの
absolutePathsに展開されること- 空文字列がパス配列に含まれないこと
- ネストされたフォルダ構造も正しく処理されること
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/file-intake-client/index.ts` around lines 94 - 110, Add manual cross-platform tests and checklist for extractDroppedPaths to ensure folder drops expand to multiple file paths and no empty strings are produced: verify on macOS/Windows/Linux that dragging a folder into the renderer yields all nested file absolute paths via window.webUtils.getPathForFile, that extractDroppedPaths returns no empty entries, and that nested folder structures are handled; also add a test step to confirm the main-side classification logic still correctly checks workspace membership for the expanded paths.
🤖 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/scratch/index.ts`:
- Around line 55-108: The readFile procedure currently allows reading any
sanitized absolute path but should mirror writeFile's deny-list and symlink
protections: after computing abs via sanitizeAbsolutePath in readFile, resolve
the parent directory with realpath and run the same deny-list check used by
writeFile (deny paths like /etc, ~/.ssh, etc.), and also reject when the target
leaf is a symlink (use fs.lstat on abs and throw a TRPCError similar to
writeFile when isSymbolicLink). Ensure maxBytes handling
(MAX_SCRATCH_READ_BYTES) and the existing ENOENT/ directory/size checks remain
unchanged; reuse any existing denyList/ helper used by writeFile to keep
behavior consistent.
- Around line 18-28: The deny-list SCRATCH_WRITE_DENY_PATTERNS is POSIX-only and
misses Windows paths (backslashes and C:\ system dirs and user-dotfolders), so
update matching to normalize input paths and broaden patterns: when evaluating
paths normalize separators (e.g., replace backslashes with slashes OR use
path.relative against os.homedir()) before testing against
SCRATCH_WRITE_DENY_PATTERNS, add Windows-specific deny patterns for common
system dirs (e.g., C:\Windows\, C:\ProgramData\) and check user dot-folders by
comparing path segments or using path.join(os.homedir(), '.ssh') / '.aws' /
'.gnupg' rather than literal /^\/\.ssh\// regexes; ensure all matching is done
against the normalized path so existing RegExp entries in
SCRATCH_WRITE_DENY_PATTERNS cover both platforms.
In `@apps/desktop/src/main/lib/file-intake/index.ts`:
- Around line 282-303: The filterFilePathArgs function currently only skips
argv[0], causing dev-mode (process.defaultApp === true) to include argv[1] (the
app script) as a candidate; update filterFilePathArgs to also skip argv[1] when
process.defaultApp is true by adjusting the candidate filter (e.g., change the
early-return condition to return false if idx === 0 || (process.defaultApp &&
idx === 1)), keeping the rest of the path resolution and fs.access behavior
unchanged and preserving the function signature.
In `@apps/desktop/src/renderer/lib/file-intake-client/index.ts`:
- Around line 14-17: This file currently defines IpcRendererAPI and listens for
raw ipcRenderer events "file-intake:open-workspace-batch" and
"file-intake:open-scratch-batch"; replace that raw ipc usage by implementing a
tRPC router/ procedures for those two events and calling them from the renderer
via the existing trpc client. Concretely: add procedures named e.g.
fileIntake.openWorkspaceBatch and fileIntake.openScratchBatch in your trpc
router (server/preload side) that accept the same payload shape, wire the
preload/main process to expose those procedures (matching how other IPC routes
are exposed), and update this module to import the trpc client and
invoke/subscribe to those procedures instead of using
IpcRendererAPI/ipcRenderer.on/off so the app follows the "use src/lib/trpc for
Electron IPC" guideline.
In
`@apps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEditor/ScratchEditor.tsx`:
- Around line 22-26: writeMut の onSuccess ハンドラは現在 setSavedAt(res.mtimeMs)
しか更新しておらず、draft が残ったままなので hasChanges (draft !== null && data.content !== draft)
が true のままになります。修正は writeMut.onSuccess の中（関数 writeMut.useMutation の onSuccess
ハンドラ、現在 setSavedAt を呼んでいる箇所）で保存成功時に setDraft(null) を呼び出して draft をクリアして
hasChanges を false に戻すこと；代替案として utils.scratch.readFile.setData(...) や
queryClient.invalidate() でキャッシュ（data.content）を更新してもよいですが、このコンポーネントでは
setDraft(null) が最も簡潔です。
In
`@apps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchTabBar/ScratchTabBar.tsx`:
- Around line 58-69: The close button in ScratchTabBar is not keyboard-focusable
because tabIndex={-1}; remove the tabIndex prop from the button (so it uses the
native focusable behavior of a <button>) and ensure the onClick handler calls
onClose(tab.id) as it already does; optionally keep or remove the
e.stopPropagation() call (it’s unnecessary here). This touches the close button
in ScratchTabBar (the element rendering <X className="size-3" />) and preserves
the existing onClose(tab.id) call so Enter/Space will invoke the close action.
---
Nitpick comments:
In `@apps/desktop/src/main/index.ts`:
- Around line 636-646: The second-instance handler should guard against
rejections from async calls: wrap the awaits for processDeepLink(url),
filterFileIntakeArgs(argv), and dispatchFileIntakePaths(paths) inside a
try/catch block in the app.on("second-instance", ...) callback (the block
containing findDeepLinkInArgv, processDeepLink, filterFileIntakeArgs,
dispatchFileIntakePaths); on error call console.error with a clear context
message and invoke the same error reporting path you use elsewhere (e.g.,
Sentry/report function) so failures are handled locally rather than falling
through to unhandledRejection, mirroring the did-finish-load flow's reporting
behavior.
- Around line 857-892: The current did-finish-load wait can hang if the first
window is closed while still loading; update the firstWindow branch to detect
that case and fallback: when webContents.isLoading() attach both a
did-finish-load handler and a one-time 'closed'/'destroyed' handler on
firstWindow (or a short timeout) that, if markFileIntakeReady hasn't been
called, will either re-arm the existing app.once('browser-window-created') logic
or invoke onRendererReady after a bounded delay; reference firstWindow,
onRendererReady, markFileIntakeReady, webContents.isLoading(), and the
app.once('browser-window-created') fallback when implementing this change.
In `@apps/desktop/src/main/lib/file-intake/index.ts`:
- Around line 190-198: The focusFirstWindow function currently assumes the first
window from BrowserWindow.getAllWindows() is valid; change it to pick the first
non-destroyed window by filtering getAllWindows() with !win.isDestroyed() (or
find the first where isDestroyed() returns false), then operate on that window:
check isMinimized(), restore if needed, then show() and focus(), and return it;
if no non-destroyed window exists return null. Ensure you reference
focusFirstWindow and BrowserWindow.getAllWindows()/isDestroyed() when making the
change.
- Around line 252-268: The current direct IPC sends in the file-intake flow
(inside the loop that uses splitTargets and the two win.webContents.send calls)
must be replaced with tRPC observable events: create a new tRPC router (e.g.,
file-intake or fileIntakeRouter) exporting observable endpoints for the two
event types (openWorkspaceBatch and openScratchBatch) that emit the same
payloads (workspaceId + absolutePaths and absolutePaths for scratch), and from
this module replace the win.webContents.send calls with invocations that emit to
those observables (use the same emit API shape used by other routers like
workspaces/init, vibrancy, service-status, terminal) so the renderer can
subscribe via trpc client; keep splitTargets and the payload shapes unchanged
and ensure the observable emits are called in the same places where
win.webContents.send currently runs.
In `@apps/desktop/src/renderer/lib/file-intake-client/index.ts`:
- Around line 5-12: Replace the two object type aliases WorkspaceBatchPayload
and ScratchBatchPayload with interface declarations to follow project
conventions (aligning with other interfaces like IpcRendererAPI and NavRouter);
update the declarations for WorkspaceBatchPayload and ScratchBatchPayload to use
the interface keyword and keep the same property names and types so callers and
exports remain unchanged.
- Around line 85-92: Replace the direct ipcRenderer listeners for
"file-intake:open-workspace-batch" and "file-intake:open-scratch-batch" (the
ipcRenderer?.on(...) registrations that call handleWorkspaceBatch and
handleScratchBatch) with tRPC subscription-based notifications: implement
corresponding publicProcedure.subscriptions on the main side (e.g.,
scratch.onIngestBatch) that use observable<Batch>((emit) => {
emit.next(payload); return unsubscribe; }), and consume those subscriptions from
the renderer via src/lib/trpc instead of ipcRenderer so you retain types,
teardown, and remove the unknown casts around IpcRendererAPI; update the
consumer code that currently calls handleWorkspaceBatch/handleScratchBatch to
subscribe to the new trpc observable endpoints.
- Around line 94-110: Add manual cross-platform tests and checklist for
extractDroppedPaths to ensure folder drops expand to multiple file paths and no
empty strings are produced: verify on macOS/Windows/Linux that dragging a folder
into the renderer yields all nested file absolute paths via
window.webUtils.getPathForFile, that extractDroppedPaths returns no empty
entries, and that nested folder structures are handled; also add a test step to
confirm the main-side classification logic still correctly checks workspace
membership for the expanded paths.
In
`@apps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEmpty/ScratchEmpty.tsx`:
- Around line 17-24: The comment saying the redirect runs "synchronously on
mount" is inaccurate because the current useEffect runs after commit
(post-paint); update the comment text to reflect that (e.g., "runs after commit
/ after initial paint") or, if you need the redirect to run before paint,
replace the useEffect with useLayoutEffect in the ScratchEmpty component so
navigate({ to: "/workspace", replace: true }) executes prior to the first paint
(keep the same dependencies: [navigate, redirectToWorkspaceOnEmpty]); reference
useEffect/useLayoutEffect, navigate, and redirectToWorkspaceOnEmpty when making
the change.
In `@apps/desktop/src/renderer/screens/scratch/ScratchView/utils/path.ts`:
- Around line 1-5: The basename function returns an empty string for paths
ending with a separator (e.g., "foo/bar/"); update basename to ignore trailing
slashes by trimming any trailing '/' or '\' before splitting (or split and pick
the last non-empty segment) so that basename(p: string) returns the final path
segment even for directory-like inputs; adjust the implementation in function
basename to remove trailing separators or filter empty parts and then return the
last non-empty part (still falling back to p if something unexpected occurs).
🪄 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: ecbac79f-8305-4004-8702-1285ca1ece8e
📒 Files selected for processing (20)
apps/desktop/electron-builder.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/lib/trpc/routers/scratch/index.tsapps/desktop/src/main/index.tsapps/desktop/src/main/lib/file-intake/index.tsapps/desktop/src/preload/index.tsapps/desktop/src/renderer/index.tsxapps/desktop/src/renderer/lib/file-intake-client/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/scratch/page.tsxapps/desktop/src/renderer/screens/scratch/ScratchView/ScratchView.tsxapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEditor/ScratchEditor.tsxapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEditor/index.tsapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEmpty/ScratchEmpty.tsxapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchEmpty/index.tsapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchTabBar/ScratchTabBar.tsxapps/desktop/src/renderer/screens/scratch/ScratchView/components/ScratchTabBar/index.tsapps/desktop/src/renderer/screens/scratch/ScratchView/index.tsapps/desktop/src/renderer/screens/scratch/ScratchView/store.tsapps/desktop/src/renderer/screens/scratch/ScratchView/utils/path.ts
- **tRPC subscription に置き換え**: main→renderer の file-intake バッチ
送信を生 ipcRenderer から `fileIntakeEmitter` + scratch.onOpen*Batch
subscription に変更。AGENTS.md の「IPC は必ず tRPC」ルール準拠。
- **deny-list を Windows 対応**: パスを forward slash に正規化してから
パターン照合。`C:\Users\x\.ssh\...` や `C:\Windows\...` も拒否。
- **readFile にも deny-list + realpath + symlink leaf 拒否**: `writeFile`
と挙動を揃える。`~/.ssh/config` を誤ってドロップした時に開く段階で
FORBIDDEN になるので、編集してから保存で初めて知るUX を回避。
- **dev モードの argv[1] スキップ**: `process.defaultApp` 時は
`argv[1] = main.js のパス` なので idx<2 を除外 (コールドスタートで
自身のビルド成果物を scratch でディスパッチする事故を防止)。
- **保存後 readFile キャッシュを更新**: `utils.scratch.readFile.setData`
で書き込んだ内容を反映。draft クリア単独だと value が元の
data.content にフォールバックしてエディタが巻き戻る UX バグを防止。
- **close ボタンの `tabIndex={-1}` を削除**: キーボードのみユーザーが
scratch タブを閉じられるように。
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2fb06ccec
ℹ️ 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".
macOS で全ウィンドウを閉じた状態で Finder から "Open With" 経由で ファイルが来た場合、dispatchPaths の focusFirstWindow() が null を返して パスが queue されるだけで、後から Dock クリックで新ウィンドウを作っても drain されず、アプリを再起動するまで無反応になる問題。 `browser-window-created` を常時監視し、新ウィンドウの did-finish-load で queue を drain するよう変更。cold-start 用の drain は専用フラグで一度だけ 動くよう守り、以降は追加ドロップ用の re-drain だけが走る。
Owner
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dddfd4d6ed
ℹ️ 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".
P2 (tearoff window): renderer バンドルは tearoff ウィンドウでも共有される ため、installFileIntakeClient を無条件に呼ぶと tearoff も scratch.onOpen*Batch を subscribe して OS ドロップのたびに勝手に ナビゲートしてしまう。`isTearoffWindow()` 時は install をスキップ。 P1 (mutation variables): writeMut.onSuccess で live `draft` 状態から キャッシュを合成していたが、保存中にユーザーがタイピングを続けると draft は在空で進んだ値になり、実際に書き込んだ内容と食い違う。 mutation variables.content (実際に writeFile に渡したペイロード) から キャッシュを作るよう変更、in-flight な追加キーストロークを dirty 扱いで 保持する。 なお Codex の「queued paths drain」P1 は false positive で、既に dddfd4d の `browser-window-created` + `drainOnWindowReady` で対応済み。
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
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.
概要
VS Code や Antigravity と同じように、Finder / Explorer からファイルをダブルクリックしたり Superset のウィンドウに DnD したりしてファイルを開けるようにする v1 実装。
ドロップされたパスを main プロセス側で分類し、2 つの経路に振り分ける:
/scratchルートでエディタのみの軽量モードで開く。Git / Agent / Chat / Terminal は無効設計判断 (ユーザーとの対話で確定した仕様)
useTabsStore.addFileViewerPaneを直接呼ぶ主な追加
apps/desktop/src/main/lib/file-intake/— パス分類 / キューイング / dispatchPaths / EventEmitterapps/desktop/src/lib/trpc/routers/scratch/— ワークスペース外の read/write、DnD 取り込み mutation、main→renderer バッチ配信用の 2 つの subscriptionapps/desktop/src/renderer/lib/file-intake-client/— ウィンドウ DnD (bubble 相 +defaultPreventedガード) と tRPC subscription の受信apps/desktop/src/renderer/screens/scratch/ScratchView/— タブ + CodeEditor の最小 UIapps/desktop/src/renderer/routes/_authenticated/_dashboard/scratch/page.tsx— 新ルート変更
main/index.ts:will-finish-launching+open-file、second-instanceの argv 経由ファイル渡し、browser-window-createdで毎回 drain (全ウィンドウ閉→再アクティブ時も queue を drain)preload/index.ts:typeof webUtilsAPIパターンに変更 (TS2687 の pre-existing エラーを解消)electron-builder.ts:fileAssociations追加 (macOS はrank: Alternate、Linux AppImage 互換のため各拡張子を単独エントリに)安全面
/etc,/System,/usr,~/.ssh,~/.aws,~/.gnupg,C:\Windows,C:\ProgramData)/tmp/link/fooでlinkが~/.sshを指すケース)defaultPreventedをガード → 既存の Chat 添付 / Terminal パスドロップ / Sidebar プロジェクト追加 / ScriptsEditor 等のドロップゾーンを上書きしないレビュー対応履歴
6ea3a93cff9e9e4a4e2fb06ccedddfd4d6e35229b082合計 17 件 のレビュー指摘に対応済み。Codex の "queued paths drain" 再提起は
dddfd4d6e時点で既に対応済みの false positive。Test plan
手動確認 (macOS):
~/Downloads/foo.md) を DnD →/scratchで開く、サイドバーが消える、Git/Agent/Chat/Terminal のステータスが消える/workspaceにリダイレクトPromptInput/ Terminal / SidebarDropZone の既存ドロップ機能が壊れていない/etc/passwdを DnD → read / save の両方でFORBIDDENtoo-large表示未確認:
second-instance経路)v1 スコープ外 (意図的に先送り)
.gitignore,Dockerfile) の fileAssociations 登録 (electron-builder のextが拡張子のみ対応)