From be0e3c2fa7931455705b129a26ebb2d441b88c95 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 30 Apr 2026 20:39:04 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20handle=20unhydrated=20store=20in=20file-?= =?UTF-8?q?explorer=E2=86=92terminal=20drop=20(#1296)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getConnectionId returns undefined during store hydration, which was misclassified as remote by `!== null` check. Changed to `typeof === 'string'` so unhydrated state falls through to client-OS quoting, consistent with terminal-drop-handler.ts behavior. Co-authored-by: Orca --- docs/terminal-drop-ssh.md | 452 ++++++++++++++++++ src/main/ipc/filesystem-import-ssh.ts | 46 +- src/main/ipc/filesystem-mutations.ts | 49 +- src/preload/api-types.ts | 12 + src/preload/index.ts | 12 + .../components/terminal-pane/TerminalPane.tsx | 24 +- .../terminal-pane/pane-helpers.test.ts | 18 +- .../components/terminal-pane/pane-helpers.ts | 11 +- .../terminal-pane/terminal-drop-handler.ts | 121 +++++ .../use-terminal-pane-global-effects.ts | 34 +- 10 files changed, 750 insertions(+), 29 deletions(-) create mode 100644 docs/terminal-drop-ssh.md create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-handler.ts diff --git a/docs/terminal-drop-ssh.md b/docs/terminal-drop-ssh.md new file mode 100644 index 00000000000..09f44a63887 --- /dev/null +++ b/docs/terminal-drop-ssh.md @@ -0,0 +1,452 @@ +# Terminal drag-and-drop over SSH + +## Problem + +Dragging a local file onto a terminal pane inserts the file's absolute path +into the PTY, so the user can reference it in a CLI or TUI-agent prompt. On +SSH worktrees the terminal runs remotely, so injecting a **local** path +(`/Users/alice/Desktop/log.txt`) is useless — the remote agent has no access +to it. + +The file-explorer drop path was fixed in PR #1279 by routing through SFTP +upload (`importExternalPathsSsh`). The terminal drop path was not touched +and still breaks for SSH worktrees. + +Reported: https://stablygroup.slack.com/archives/C0ASMDT6LQZ/p1777530155421009 + +## Goals + +- Dropping a local file onto a terminal connected to an SSH worktree makes + that file available to the remote shell/agent, and injects a path the + remote process can read. +- Local terminal drops keep their current behavior: reference-in-place, no + copy, no authorization, no repo pollution. +- Local and SSH paths share one main-side resolver. The renderer may pass + connection context and show progress UI, but copy/upload/deconfliction + policy stays out of the renderer so the two modes do not drift over time. + +## Non-goals + +- Unifying terminal drop with file-explorer drop. They have different + semantics (explorer always copies into a user-picked `destDir`; terminal + references a path). They share the SSH upload primitive internally but + remain separate IPCs. +- Cleanup / garbage collection of staged remote files. Tracked as follow-up + (see "Follow-ups" below — file **before merging** and replace this + parenthetical with the issue number so the reference is locatable). Until + GC lands, files uploaded by a drop whose pane is unmounted before the + upload resolves are orphaned in `.orca/drops/` with no injected path. + Users should know uploads are not cancellable. +- Abortable uploads. SFTP transfers run to completion even if the terminal + pane is unmounted mid-flight. + +## Considered: Option A (full unification) + +Terminal drop calls `fs:importExternalPaths` with +`destDir = worktreePath`, then injects `result.destPath` into the PTY. +Shares exactly the file-explorer code path. + +Rejected because it changes **local** terminal-drop UX: today dropping +`~/Desktop/log.txt` into the terminal pastes `/Users/…/Desktop/log.txt` +so the agent reads the file in place; under Option A the file would be +copied into the repo. Users rely on the reference-in-place behavior to +point agents at files without polluting the worktree. + +## Design (Option B) + +### New IPC + +``` +fs:resolveDroppedPathsForAgent({ + paths: string[], + worktreePath: string, + connectionId?: string, +}) → { + resolvedPaths: string[], + skipped: { sourcePath: string; reason: 'missing' | 'symlink' | 'permission-denied' | 'unsupported' }[], + failed: { sourcePath: string; reason: string }[], +} +``` + +Contract: + +- **Local (`connectionId == null`):** returns + `{ resolvedPaths: paths, skipped: [], failed: [] }` unchanged. No copy. No + authorization (matches today's behavior — the agent's own read is what gets + authorized, not the drop). Use `args.connectionId == null` (not `!args.connectionId`) + so an empty string cannot silently pick the local branch. +- **SSH (`connectionId` is a non-empty string):** uploads each path via SFTP + into a staging dir under the worktree. Returns remote absolute paths for + items that uploaded successfully, items rejected by policy (symlinks, + missing sources, permission-denied, unsupported file types) in `skipped`, + and hard upload errors in `failed`. The split mirrors `ImportItemResult`'s + existing `'imported' | 'skipped' | 'failed'` and lets the renderer toast + "Skipped N symlinks" distinctly from "Failed to upload N files." Collapsing + skipped into failed would mislabel routine policy rejections as errors. + +### SSH staging dir + +`${worktreePath}/.orca/drops/` on the remote. `.orca/` is reserved as an +Orca-owned directory for future remote state (GC metadata, cached remote +capability probes, etc.); this is its first use. Future features adding +subpaths under `.orca/` should namespace themselves (`.orca/drops/`, +`.orca//`) rather than placing files at the root. + +Rationale: + +- No need to resolve remote `$HOME` (which would require a round-trip and + caching layer). +- Lives inside the worktree, so cleaned up naturally when the worktree is + deleted. +- The agent has read access by construction (it runs with the worktree's + cwd). + +The main process must bootstrap `.orca/.gitignore` with `*\n!.gitignore\n` +before the first successful upload. Otherwise every SSH terminal drop dirties +source control with an untracked `.orca/` directory, which recreates the +repo-pollution problem that ruled out Option A for local terminal drops. The +`!.gitignore` negation keeps the marker file itself trackable if we ever want +to (and costs nothing today — `git status` stays clean either way because +nothing tries to add it). + +The staging directory must be created recursively over SFTP before upload: + +- create `${worktreePath}/.orca` (ignore "already exists") +- write `${worktreePath}/.orca/.gitignore` as `*\n!.gitignore\n` **only if it + does not already exist** — never overwrite. A user may have added patterns + there, and silently clobbering user-authored content violates least + surprise even inside an Orca-owned directory. Use `sftpPathExists` before + writing. (Two concurrent first-drops racing through the `sftpPathExists` + check will both write the same bytes — last writer wins, idempotent, so + the race is benign and not worth locking.) +- create `${worktreePath}/.orca/drops` (ignore "already exists") + +Do not rely on `uploadFile`, `uploadDirectory`, or the existing +`mkdirSftp(destPath)` calls to create missing parents. `uploadFile` writes +directly to the final remote file path, and `mkdirSftp` is not recursive, so +the first terminal drop into a fresh SSH worktree would fail if the parents do +not already exist. + +### Main-side implementation + +`src/main/ipc/filesystem-mutations.ts`: + +```ts +ipcMain.handle('fs:resolveDroppedPathsForAgent', async (_e, args) => { + // Why: `== null` (not `!args.connectionId`) so an empty string is treated + // as an error from the renderer, not silently routed to the local branch. + if (args.connectionId == null) { + return { resolvedPaths: args.paths, skipped: [], failed: [] } + } + const worktreePath = args.worktreePath.replace(/\/+$/, '') + const destDir = `${worktreePath}/.orca/drops` + const { results } = await importExternalPathsSsh( + args.paths, + destDir, + args.connectionId, + { ensureDir: true }, + ) + const resolvedPaths: string[] = [] + const skipped: { sourcePath: string; reason: ImportSkipReason }[] = [] + const failed: { sourcePath: string; reason: string }[] = [] + // Iterate in input order so injected paths align with the user's drop order. + for (const r of results) { + if (r.status === 'imported') { + resolvedPaths.push(r.destPath) + } else if (r.status === 'skipped') { + skipped.push({ sourcePath: r.sourcePath, reason: r.reason }) + } else { + failed.push({ sourcePath: r.sourcePath, reason: r.reason }) + } + } + return { resolvedPaths, skipped, failed } +}) +``` + +Reuses `importExternalPathsSsh` — SFTP upload, symlink pre-scan, name +deconfliction, per-item error reporting are all already there. + +**Staging bootstrap lives inside `importExternalPathsSsh`** behind a new +optional `{ ensureDir?: boolean }` parameter. When set, before the first +upload the function creates `${destDir}`'s parent chain (`.orca`, then +`drops`) and writes `.orca/.gitignore` (`*\n`) only if missing, all on the +**same SFTP session** already opened for the upload. Do not add a separate +`ensureSshDropStagingDir` helper that opens its own channel — that would +double the SFTP handshake cost on every drop. + +### Renderer-side implementation + +**API change to `shellEscapePath`.** Today the second arg is a *userAgent +string* (substring-matched for `"Windows"`), which couples escape rules to +the client OS. For SSH drops we need to escape for the *target shell*, +which is always POSIX on the remote regardless of client OS. Change the +signature to take an explicit target: + +```ts +shellEscapePath(path: string, targetShell: 'posix' | 'windows') +``` + +Callers derive `targetShell` from context: local drops pass +`isWindowsUserAgent() ? 'windows' : 'posix'`; SSH drops always pass +`'posix'`. This makes test #11 (Windows client → Linux SSH worktree) +correct by construction instead of by coincidence. + +**Migration — all three call sites + tests must change together:** + +- `src/renderer/src/components/terminal-pane/pane-helpers.ts:53` — update + signature; drop the `userAgent` default. +- `src/renderer/src/components/terminal-pane/TerminalPane.tsx:937` + (file-explorer → terminal drop). This is a **local-only** code path + (explorer drag uses a DOM MIME type that the preload SSH bridge does not + forward), so pass `isWindowsUserAgent() ? 'windows' : 'posix'` to preserve + today's behavior exactly. +- `src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts:344` + — replaced by the new SSH-aware handler below; the `shellEscapePath` call + moves inside it with an explicit `targetShell`. +- `src/renderer/src/components/terminal-pane/pane-helpers.test.ts` — the + existing cases pass `'Macintosh'`, `'Linux'`, `'Windows'` as the userAgent + arg. Rewrite to pass `'posix'` (for Mac/Linux) and `'windows'`, so tests + exercise the new contract rather than the legacy substring match. + +No local behavior changes if this migration is done in one commit: Mac/Linux +already took the POSIX branch via the userAgent string match; Windows +already took the Windows branch. The new signature just names that +explicitly. + +`src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts`: + +```ts +return window.api.ui.onFileDrop(async (data) => { + if (data.target !== 'terminal') return + if (data.paths.length === 0) return + const manager = managerRef.current + if (!manager) return + const pane = manager.getActivePane() ?? manager.getPanes()[0] + if (!pane) return + const paneId = pane.id + const transport = paneTransportsRef.current.get(paneId) + if (!transport) return + + const wtId = worktreeIdRef.current + const worktreePath = worktreePathRef.current + if (!wtId || !worktreePath) return + + // Why: getConnectionId (selector on the terminals/repos slice: + // `state.repos.find(r => r.id === )?.connectionId`, + // exposed via the store) returns `string` (SSH), `null` (local repo + // found), or `undefined` (store not hydrated / worktree not found). + // Treat `undefined` as an error, not as "local" — otherwise a drop + // during hydration would silently paste local paths into a remote + // shell. + const connectionId = getConnectionId(wtId) + if (connectionId === undefined) { + toast.error('Worktree not ready — try again in a moment.') + return + } + const isRemote = connectionId !== null + const targetShell: 'posix' | 'windows' = isRemote + ? 'posix' + : isWindowsUserAgent() + ? 'windows' + : 'posix' + + // Local fast path: no IPC round-trip, no toast. Preserves today's + // zero-latency behavior exactly — same code shape as before, only the + // shellEscapePath signature is new (and resolves to the same branch). + if (!isRemote) { + for (const p of data.paths) { + transport.sendInput(`${shellEscapePath(p, targetShell)} `) + } + pane.terminal.focus() + return + } + + const pending = toast.loading( + `Uploading ${data.paths.length} file(s) to remote…`, + ) + try { + const { resolvedPaths, skipped, failed } = + await window.api.fs.resolveDroppedPathsForAgent({ + paths: data.paths, + worktreePath, + connectionId, + }) + // Why: pane may have unmounted during the SFTP upload (tab closed, + // worktree switched). Re-check the transport map before writing so + // we don't call sendInput on a torn-down PTY. Orphaned uploads are + // acknowledged in Non-goals. + const liveTransport = paneTransportsRef.current.get(paneId) + if (liveTransport) { + // resolvedPaths preserves input order (main-side iterates results in + // order); injected paths line up with the user's drop gesture. + for (const p of resolvedPaths) { + liveTransport.sendInput(`${shellEscapePath(p, targetShell)} `) + } + pane.terminal.focus() + } + if (skipped.length > 0) { + const symlinkCount = skipped.filter((s) => s.reason === 'symlink').length + const noun = skipped.length === 1 ? 'item' : 'items' + toast.message( + symlinkCount === skipped.length + ? `Skipped ${skipped.length} symlink${skipped.length === 1 ? '' : 's'}.` + : `Skipped ${skipped.length} ${noun}.`, + ) + } + if (failed.length > 0) { + const noun = failed.length === 1 ? 'file' : 'files' + toast.error(`Failed to upload ${failed.length} ${noun}.`) + } + } catch (err) { + toast.error(extractIpcErrorMessage(err, 'Failed to upload files.')) + } finally { + toast.dismiss(pending) + } +}) +``` + +`extractIpcErrorMessage` is the existing helper at +`src/renderer/src/lib/ipc-error.ts:6` (already used by +`useFileExplorerImport.ts`, `Terminal.tsx`, etc.). Reuse it — do not copy +the body locally. + +New dependencies on the hook: `worktreeId` and `worktreePath` refs. Use the +`TerminalPane`'s own `worktreeId` prop, not global `activeWorktreeId`. The +drop listener is already gated by `isActive`, and the pane's own +`worktreeId` is the authoritative identity of the terminal being written +to; reading from global state would race during worktree switches. Promote +this reasoning into a `// Why:` comment at the call site per CLAUDE.md. + +Derive `worktreePath` the same way `use-terminal-pane-lifecycle.ts` does +today: find the worktree by `worktreeId` in the store, then fall back to +`cwd`. + +### Why not branch in the renderer + +- Keeps upload semantics in one place. Adding a second terminal consumer + (e.g. a standalone TUI pane) should call the same resolver instead of + deciding where to copy, how to deconflict names, or how to report per-item + failures. +- The renderer can know that the worktree is SSH for progress UI and shell + escaping, but it should not know SFTP details or construct uploaded + destination filenames itself. + +### UX details + +- **SSH "Uploading…" toast:** SFTP of a few MB can take seconds. Without + feedback the user thinks the drop failed. Dismiss on success, replace + with an error toast on failure. +- **Don't inject until upload resolves.** Injecting the remote path before + the file lands means the agent may try to read a file that doesn't yet + exist and error. Worth the extra perceived latency. +- **Failure policy:** partial failures still inject the succeeded paths + and toast the count of failures (same pattern as the explorer). +- **Escape for the terminal that receives the path.** Local drops keep the + existing platform-specific quoting. SSH drops must use POSIX shell quoting + for the returned remote paths; do not let a Windows client choose Windows + quoting for a Linux/macOS SSH shell. + +## System fit + +``` +[Electron preload native drop] + | + v +[terminal:file-drop IPC relay] + | + v +[active TerminalPane drop handler] + | + v +[fs:resolveDroppedPathsForAgent] + | local | SSH + v v +[return original paths] [SFTP stage into ${worktreePath}/.orca/drops] + | | + | v + | [return remote readable paths] + | | + +---------------+-----------------+ + | + v + [PTY sendInput escaped paths] +``` + +## Testing + +1. Local worktree, drop a single file onto the terminal → original + absolute path pasted. No copy. No change from today. +2. Local worktree, drop multiple files → each path pasted separated by a + space. No change from today. +3. SSH worktree, drop a single file → file uploads to + `${worktreePath}/.orca/drops/` on remote; remote path pasted + into PTY; agent can read it. +4. SSH worktree, drop a folder → folder uploads recursively; remote dir + path pasted. +5. SSH worktree, drop a symlink → no injection; toast reads + "Skipped 1 symlink." (not "Failed to upload") because symlink rejection + is policy, not error. +6. SSH worktree, drop 5 files where items 2 and 4 are permission-denied at + the local source → paths 1, 3, 5 are injected **in that order** + (matching input order), toast says "Skipped 2 items." (permission-denied + is classified `skipped`, not `failed`, by `importExternalPathsSsh`). + Additionally, simulate an SFTP write error mid-upload to cover the + `failed` branch → "Failed to upload N files" toast. +7. SSH worktree disconnected mid-drag → user-visible error toast, no + partial injection. +8. Name collision: drop the same file twice in quick succession → second + upload lands as ` copy.` (deconfliction inherited from + `importExternalPathsSsh`). +9. Fresh SSH worktree with no `.orca` directory → first drop creates + `.orca/`, `.orca/.gitignore`, and `.orca/drops/`; upload succeeds. +10. After an SSH drop, `git status --short` in the remote worktree does not + show `.orca/`. +11. Windows client dropping `my file's $draft.txt` into a Linux SSH worktree + pastes a POSIX-escaped remote path, not Windows double-quoted syntax. + Expected output for remote path `/home/u/wt/.orca/drops/my file's $draft.txt`: + `'/home/u/wt/.orca/drops/my file'\''s $draft.txt'` (literal `$draft` + inside single quotes — no expansion, no backslash-escaping). +16. Local terminal drop on macOS / Linux / Windows clients behaves + byte-identically to pre-change: same injected string, no toast, no IPC + call. Run the existing `pane-helpers.test.ts` expectations through the + new `'posix' | 'windows'` API and confirm outputs match the legacy + userAgent-based outputs. +17. File-explorer → terminal drop (`TerminalPane.tsx` onDrop) continues to + work on local and SSH worktrees exactly as today — this path is not + touched by the new IPC and must still use the explorer's own handling. +12. Empty drop (`data.paths.length === 0`) → no IPC call, no toast, no + injection. (Possible on some OSes when a drag contains only non-file + items.) +13. Store unhydrated (`getConnectionId` returns `undefined`) → user-visible + "Worktree not ready" toast, no injection, no IPC call. Not a silent + local fallback. +14. Existing user-authored `.orca/.gitignore` with extra patterns → after + first SSH drop the file is unchanged (bootstrap writes only when + missing). +15. Unit coverage: + - preload/API types expose `resolveDroppedPathsForAgent` and the + channel is registered in the preload `contextBridge` / IPC + allowlist (regression guard — easy to forget). + - main IPC covers: local passthrough, SSH success, partial failure + (order preserved), fresh-worktree staging bootstrap, bootstrap + preserves existing `.gitignore`, and disconnected SSH. + - terminal-pane coverage verifies the resolver is called once per + gesture, no path is injected until the promise resolves, and + `shellEscapePath` is called with `'posix'` for SSH drops regardless + of client userAgent. + +## Follow-ups + +File each of these as a GitHub issue before merging the implementation PR +so "tracked as follow-up" is actually locatable. Inline the issue number +next to each item once filed (e.g. `- GC drops dir (#1301)`), and update +the GC paragraph in Non-goals to point to that issue directly. + +- GC `${worktreePath}/.orca/drops/` on worktree delete / disconnect. +- `AbortController` plumbing so unmounting the terminal pane cancels the + in-flight SFTP upload (related to the pane-unmount guard added in the + renderer handler: today we no-op the injection, but the bytes still + transfer). +- Drag-over affordance on the terminal pane (it has the + `data-native-file-drop-target="terminal"` marker but no hover style), + so users get feedback that dropping into the terminal is supported. diff --git a/src/main/ipc/filesystem-import-ssh.ts b/src/main/ipc/filesystem-import-ssh.ts index 9276bb362fb..8554b466547 100644 --- a/src/main/ipc/filesystem-import-ssh.ts +++ b/src/main/ipc/filesystem-import-ssh.ts @@ -12,7 +12,8 @@ import type { ImportItemResult } from './filesystem-mutations' export async function importExternalPathsSsh( sourcePaths: string[], destDir: string, - connectionId: string + connectionId: string, + options?: { ensureDir?: boolean } ): Promise<{ results: ImportItemResult[] }> { if (sourcePaths.length === 0) { return { results: [] } @@ -35,6 +36,17 @@ export async function importExternalPathsSsh( const sftp = await conn.sftp() try { + if (options?.ensureDir) { + // Why: terminal-drop staging needs `${worktree}/.orca/drops` to exist + // before the first upload. Upload primitives do not create parent dirs, + // and mkdirSftp is not recursive — so walk the parent chain here on the + // same SFTP session to avoid doubling the handshake cost. Writing the + // .orca/.gitignore marker only when absent prevents clobbering user- + // authored patterns. .orca/ is reserved as Orca-owned remote state; + // see docs/terminal-drop-ssh.md. + await ensureDropStagingDir(sftp, destDir) + } + const results: ImportItemResult[] = [] const reservedNames = new Set() @@ -171,6 +183,38 @@ async function deconflictNameSftp( ) } +async function ensureDropStagingDir(sftp: SFTPWrapper, destDir: string): Promise { + // destDir is a posix remote path, expected to be `${worktreePath}/.orca/drops`. + const parent = posix.dirname(destDir) + await mkdirSftp(sftp, parent) + const gitignorePath = `${parent}/.gitignore` + if (!(await sftpPathExists(sftp, gitignorePath))) { + // Why: negate the marker so .orca/.gitignore itself is trackable if we + // ever want to, without dirtying `git status` today. Only write when + // absent to avoid clobbering user-authored patterns. + await writeSftpFile(sftp, gitignorePath, '*\n!.gitignore\n') + } + await mkdirSftp(sftp, destDir) +} + +function writeSftpFile(sftp: SFTPWrapper, remotePath: string, contents: string): Promise { + return new Promise((resolve, reject) => { + let settled = false + const writeStream = sftp.createWriteStream(remotePath) + const settle = (fn: typeof resolve | typeof reject, val?: unknown): void => { + if (settled) { + return + } + settled = true + writeStream.destroy() + fn(val as never) + } + writeStream.on('close', () => settle(resolve)) + writeStream.on('error', (err) => settle(reject, err)) + writeStream.end(contents) + }) +} + async function preScanForSymlinks(dirPath: string): Promise { const entries = await readdir(dirPath, { withFileTypes: true }) for (const entry of entries) { diff --git a/src/main/ipc/filesystem-mutations.ts b/src/main/ipc/filesystem-mutations.ts index b31a8236f39..b9bcd91bedd 100644 --- a/src/main/ipc/filesystem-mutations.ts +++ b/src/main/ipc/filesystem-mutations.ts @@ -147,6 +147,53 @@ export function registerFilesystemMutationHandlers(store: Store): void { return { results } } ) + + // Why: terminal drag-and-drop resolver. Local worktrees pass paths through + // unchanged (reference-in-place; preserves zero-latency drop). SSH worktrees + // upload each path into `${worktreePath}/.orca/drops/` and return remote + // paths the remote agent can read. Kept as a separate IPC from + // fs:importExternalPaths because terminal semantics differ from the + // explorer's "copy into user-picked destDir". See docs/terminal-drop-ssh.md. + ipcMain.handle( + 'fs:resolveDroppedPathsForAgent', + async ( + _event, + args: { paths: string[]; worktreePath: string; connectionId?: string } + ): Promise => { + // Why: `== null` (not `!args.connectionId`) so an empty string is + // treated as a renderer error, not silently routed to the local branch. + if (args.connectionId == null) { + return { resolvedPaths: args.paths, skipped: [], failed: [] } + } + const worktreePath = args.worktreePath.replace(/\/+$/, '') + const destDir = `${worktreePath}/.orca/drops` + const { results } = await importExternalPathsSsh(args.paths, destDir, args.connectionId, { + ensureDir: true + }) + const resolvedPaths: string[] = [] + const skipped: { sourcePath: string; reason: ImportSkipReason }[] = [] + const failed: { sourcePath: string; reason: string }[] = [] + // Iterate in input order so injected paths align with the user's drop order. + for (const r of results) { + if (r.status === 'imported') { + resolvedPaths.push(r.destPath) + } else if (r.status === 'skipped') { + skipped.push({ sourcePath: r.sourcePath, reason: r.reason }) + } else { + failed.push({ sourcePath: r.sourcePath, reason: r.reason }) + } + } + return { resolvedPaths, skipped, failed } + } + ) +} + +export type ImportSkipReason = 'missing' | 'symlink' | 'permission-denied' | 'unsupported' + +export type ResolveDroppedPathsResult = { + resolvedPaths: string[] + skipped: { sourcePath: string; reason: ImportSkipReason }[] + failed: { sourcePath: string; reason: string }[] } // ─── External Import Types ────────────────────────────────────────── @@ -162,7 +209,7 @@ export type ImportItemResult = | { sourcePath: string status: 'skipped' - reason: 'missing' | 'symlink' | 'permission-denied' | 'unsupported' + reason: ImportSkipReason } | { sourcePath: string diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index fa89d3f76f9..e1e8fb6cae5 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -674,6 +674,18 @@ export type PreloadApi = { } )[] }> + resolveDroppedPathsForAgent: (args: { + paths: string[] + worktreePath: string + connectionId?: string + }) => Promise<{ + resolvedPaths: string[] + skipped: { + sourcePath: string + reason: 'missing' | 'symlink' | 'permission-denied' | 'unsupported' + }[] + failed: { sourcePath: string; reason: string }[] + }> watchWorktree: (args: { worktreePath: string; connectionId?: string }) => Promise unwatchWorktree: (args: { worktreePath: string; connectionId?: string }) => Promise onFsChanged: (callback: (payload: FsChangedPayload) => void) => () => void diff --git a/src/preload/index.ts b/src/preload/index.ts index 68526f2f061..d5b8f36e9a1 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1161,6 +1161,18 @@ const api = { } )[] }> => ipcRenderer.invoke('fs:importExternalPaths', args), + resolveDroppedPathsForAgent: (args: { + paths: string[] + worktreePath: string + connectionId?: string + }): Promise<{ + resolvedPaths: string[] + skipped: { + sourcePath: string + reason: 'missing' | 'symlink' | 'permission-denied' | 'unsupported' + }[] + failed: { sourcePath: string; reason: string }[] + }> => ipcRenderer.invoke('fs:resolveDroppedPathsForAgent', args), watchWorktree: (args: { worktreePath: string; connectionId?: string }): Promise => ipcRenderer.invoke('fs:watchWorktree', args), unwatchWorktree: (args: { worktreePath: string; connectionId?: string }): Promise => diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index afd9b461e67..9d938fc5bb5 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -13,6 +13,7 @@ import type { PaneManager } from '@/lib/pane-manager/pane-manager' import TerminalSearch from '@/components/TerminalSearch' import type { PtyTransport } from './pty-transport' import { fitPanes, isWindowsUserAgent, shellEscapePath } from './pane-helpers' +import { getConnectionId } from '@/lib/connection-context' import { EMPTY_LAYOUT, paneLeafId, serializeTerminalLayout } from './layout-serialization' import { createExpandCollapseActions } from './expand-collapse' import { useTerminalKeyboardShortcuts, type SearchState } from './keyboard-handlers' @@ -536,6 +537,13 @@ export default function TerminalPane({ useTerminalPaneGlobalEffects({ tabId, + // Why: use the pane's own `worktreeId` prop (not global activeWorktreeId) + // so the terminal-drop resolver routes to the worktree that actually owns + // this PTY. Reading from global state would race during worktree switches + // — the drop listener is already gated by `isActive`, and the pane's own + // id is the authoritative identity of the terminal being written to. + worktreeId, + cwd, isActive, isVisible, managerRef, @@ -934,7 +942,21 @@ export default function TerminalPane({ if (!transport) { return } - transport.sendInput(shellEscapePath(filePath)) + // Why: the explorer passes the worktree-absolute path via a DOM + // MIME, so for SSH worktrees this is a remote POSIX path destined + // for the remote shell. Quote for the target shell (remote = posix) + // rather than the client OS; otherwise a Windows client dropping + // onto an SSH-Linux worktree would emit Windows-style quoting. + // Why: `typeof === 'string'` (not `!== null`) so an unhydrated + // store (`undefined`) is treated as local and falls through to + // client-OS quoting, rather than being misclassified as remote. + const isRemote = typeof getConnectionId(worktreeId) === 'string' + const targetShell: 'posix' | 'windows' = isRemote + ? 'posix' + : isWindowsUserAgent() + ? 'windows' + : 'posix' + transport.sendInput(shellEscapePath(filePath, targetShell)) // Move focus to the terminal so the user can keep typing where the // dropped path just landed. Without this, focus stays on the file // tree row that originated the drag and subsequent keystrokes do diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.test.ts b/src/renderer/src/components/terminal-pane/pane-helpers.test.ts index 8ee9aca3069..bda187eb094 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.test.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.test.ts @@ -13,28 +13,36 @@ describe('isWindowsUserAgent', () => { describe('shellEscapePath', () => { it('keeps safe POSIX paths unquoted', () => { - expect(shellEscapePath('/tmp/file.txt', 'Macintosh')).toBe('/tmp/file.txt') + expect(shellEscapePath('/tmp/file.txt', 'posix')).toBe('/tmp/file.txt') }) it('single-quotes POSIX paths with shell-special characters', () => { - expect(shellEscapePath("/tmp/it's here.txt", 'Linux')).toBe("'/tmp/it'\\''s here.txt'") + expect(shellEscapePath("/tmp/it's here.txt", 'posix')).toBe("'/tmp/it'\\''s here.txt'") }) it('keeps safe Windows paths unquoted', () => { - expect(shellEscapePath('C:\\Users\\orca\\file.txt', 'Windows')).toBe( + expect(shellEscapePath('C:\\Users\\orca\\file.txt', 'windows')).toBe( 'C:\\Users\\orca\\file.txt' ) }) it('double-quotes Windows paths with spaces', () => { - expect(shellEscapePath('C:\\Users\\orca\\my file.txt', 'Windows')).toBe( + expect(shellEscapePath('C:\\Users\\orca\\my file.txt', 'windows')).toBe( '"C:\\Users\\orca\\my file.txt"' ) }) it('double-quotes Windows paths with cmd separators', () => { - expect(shellEscapePath('C:\\Users\\orca\\a&b.txt', 'Windows')).toBe( + expect(shellEscapePath('C:\\Users\\orca\\a&b.txt', 'windows')).toBe( '"C:\\Users\\orca\\a&b.txt"' ) }) + + it('uses POSIX escaping for SSH drops regardless of client OS', () => { + // A Windows client dropping into a Linux SSH worktree must produce POSIX + // quoting, not Windows double-quotes (see docs/terminal-drop-ssh.md). + expect(shellEscapePath("/home/u/wt/.orca/drops/my file's $draft.txt", 'posix')).toBe( + "'/home/u/wt/.orca/drops/my file'\\''s $draft.txt'" + ) + }) }) diff --git a/src/renderer/src/components/terminal-pane/pane-helpers.ts b/src/renderer/src/components/terminal-pane/pane-helpers.ts index dc38171eaf5..ae89e797638 100644 --- a/src/renderer/src/components/terminal-pane/pane-helpers.ts +++ b/src/renderer/src/components/terminal-pane/pane-helpers.ts @@ -50,11 +50,12 @@ export function isMacUserAgent( return userAgent.includes('Mac') } -export function shellEscapePath( - path: string, - userAgent: string = typeof navigator === 'undefined' ? '' : navigator.userAgent -): string { - if (isWindowsUserAgent(userAgent)) { +// Why: escape rules are a property of the *target* shell receiving the path, +// not the client OS. A Windows client dropping onto a Linux SSH worktree must +// produce POSIX-quoted output; passing a userAgent string here coupled escape +// rules to the client and silently misquoted cross-platform SSH drops. +export function shellEscapePath(path: string, targetShell: 'posix' | 'windows'): string { + if (targetShell === 'windows') { return /^[a-zA-Z0-9_./@:\\-]+$/.test(path) ? path : `"${path}"` } diff --git a/src/renderer/src/components/terminal-pane/terminal-drop-handler.ts b/src/renderer/src/components/terminal-pane/terminal-drop-handler.ts new file mode 100644 index 00000000000..3f5c036a2bf --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-drop-handler.ts @@ -0,0 +1,121 @@ +import { toast } from 'sonner' +import { getConnectionId } from '@/lib/connection-context' +import { extractIpcErrorMessage } from '@/lib/ipc-error' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' +import { useAppStore } from '@/store' +import { isWindowsUserAgent, shellEscapePath } from './pane-helpers' +import type { PtyTransport } from './pty-transport' + +type Args = { + manager: PaneManager + paneTransports: Map + worktreeId: string + cwd: string | undefined + data: { paths: string[]; target: string } +} + +/** + * Handle a native file drop targeted at a terminal pane. + * + * Local worktrees: paste the local absolute path (reference-in-place; no copy + * or IPC). SSH worktrees: upload each file into `${worktreePath}/.orca/drops` + * and paste the remote path so the remote agent can read it. See + * docs/terminal-drop-ssh.md. + */ +export async function handleTerminalFileDrop(args: Args): Promise { + const { manager, paneTransports, worktreeId, cwd, data } = args + if (data.paths.length === 0) { + return + } + const pane = manager.getActivePane() ?? manager.getPanes()[0] + if (!pane) { + return + } + const paneId = pane.id + const transport = paneTransports.get(paneId) + if (!transport) { + return + } + + // Why: `getConnectionId` returns `string` (SSH), `null` (local repo found), + // or `undefined` (store not hydrated / worktree not found). Treat + // `undefined` as an error — otherwise a drop during hydration would + // silently paste local paths into a remote shell. + const connectionId = getConnectionId(worktreeId) + if (connectionId === undefined) { + toast.error('Worktree not ready — try again in a moment.') + return + } + const isRemote = connectionId !== null + const targetShell: 'posix' | 'windows' = isRemote + ? 'posix' + : isWindowsUserAgent() + ? 'windows' + : 'posix' + + // Why: local fast path — no IPC round-trip, no toast — preserves today's + // zero-latency drop behavior. Trailing space separates multiple paths in + // the terminal input, matching standard drag-and-drop UX conventions. + if (!isRemote) { + for (const p of data.paths) { + transport.sendInput(`${shellEscapePath(p, targetShell)} `) + } + pane.terminal.focus() + return + } + + const worktreePath = resolveWorktreePath(worktreeId, cwd) + if (!worktreePath) { + toast.error('Worktree path not available.') + return + } + + const pending = toast.loading( + `Uploading ${data.paths.length} file${data.paths.length === 1 ? '' : 's'} to remote…` + ) + try { + const { resolvedPaths, skipped, failed } = await window.api.fs.resolveDroppedPathsForAgent({ + paths: data.paths, + worktreePath, + connectionId + }) + // Why: pane may have unmounted during the SFTP upload (tab closed, + // worktree switched). Re-check the transport map before writing so we + // don't call sendInput on a torn-down PTY. Orphaned uploads are an + // acknowledged limitation — see docs/terminal-drop-ssh.md. + const liveTransport = paneTransports.get(paneId) + if (liveTransport) { + for (const p of resolvedPaths) { + liveTransport.sendInput(`${shellEscapePath(p, targetShell)} `) + } + pane.terminal.focus() + } + if (skipped.length > 0) { + // Why: symlink rejection is policy, not error — show as neutral + // message. Mixed skips collapse to a single "items" count to avoid + // enumerating every reason. + const symlinkCount = skipped.filter((s) => s.reason === 'symlink').length + const noun = skipped.length === 1 ? 'item' : 'items' + toast.message( + symlinkCount === skipped.length + ? `Skipped ${skipped.length} symlink${skipped.length === 1 ? '' : 's'}.` + : `Skipped ${skipped.length} ${noun}.` + ) + } + if (failed.length > 0) { + const noun = failed.length === 1 ? 'file' : 'files' + toast.error(`Failed to upload ${failed.length} ${noun}.`) + } + } catch (err) { + toast.error(extractIpcErrorMessage(err, 'Failed to upload files.')) + } finally { + toast.dismiss(pending) + } +} + +function resolveWorktreePath(worktreeId: string, fallbackCwd: string | undefined): string | null { + const state = useAppStore.getState() + const allWorktrees = Object.values(state.worktreesByRepo ?? {}).flat() + const worktree = allWorktrees.find((w) => w.id === worktreeId) + return worktree?.path ?? fallbackCwd ?? null +} diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts index 14b9d792008..8fc59553b4a 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-global-effects.ts @@ -6,12 +6,14 @@ import { type FocusTerminalPaneDetail } from '@/constants/terminal' import type { PaneManager } from '@/lib/pane-manager/pane-manager' -import { shellEscapePath } from './pane-helpers' import { fitAndFocusPanes, fitPanes, hasDimensionsChanged } from './pane-helpers' import type { PtyTransport } from './pty-transport' +import { handleTerminalFileDrop } from './terminal-drop-handler' type UseTerminalPaneGlobalEffectsArgs = { tabId: string + worktreeId: string + cwd?: string isActive: boolean isVisible: boolean managerRef: React.RefObject @@ -25,6 +27,8 @@ type UseTerminalPaneGlobalEffectsArgs = { export function useTerminalPaneGlobalEffects({ tabId, + worktreeId, + cwd, isActive, isVisible, managerRef, @@ -35,6 +39,10 @@ export function useTerminalPaneGlobalEffects({ isVisibleRef, toggleExpandPane }: UseTerminalPaneGlobalEffectsArgs): void { + const worktreeIdRef = useRef(worktreeId) + worktreeIdRef.current = worktreeId + const cwdRef = useRef(cwd) + cwdRef.current = cwd // Why: starts as `true` so the first render with isVisible=false triggers // suspendRendering(). Without this, background worktrees that mount hidden // (isVisible=false from the start) never suspend their WebGL contexts — @@ -326,23 +334,17 @@ export function useTerminalPaneGlobalEffects({ if (!manager) { return } - const pane = manager.getActivePane() ?? manager.getPanes()[0] - if (!pane) { + const wtId = worktreeIdRef.current + if (!wtId) { return } - const transport = paneTransportsRef.current.get(pane.id) - if (!transport) { - return - } - // Why: preload consumes native OS drops before React sees them, so the - // terminal cannot rely on DOM `drop` events for external files. Reusing - // the active PTY transport preserves the existing CLI behavior for drag- - // and-drop path insertion instead of opening those files in the editor. - // Why: appending a trailing space keeps multiple paths separated in the - // terminal input, matching standard drag-and-drop UX conventions. - for (const path of data.paths) { - transport.sendInput(`${shellEscapePath(path)} `) - } + void handleTerminalFileDrop({ + manager, + paneTransports: paneTransportsRef.current, + worktreeId: wtId, + cwd: cwdRef.current, + data + }) }) }, [isActive, managerRef, paneTransportsRef]) }