Repository navigation
fix(file-drop): explorer, project sidebar, tab strip and editor own OS file drops (STA-6940 PR5) - #26133
Merged
brennanb2025 merged 32 commits intoOct 8, 2026
Conversation
…2025/sta-6940-pr3-chat-composer-feedback
…lumbing' into brennanb2025/sta-6940-pr3-chat-composer-feedback
…-pr2-drop-plumbing
Preserve element-owned drops while integrating the upstream multi-file paperclip picker and all-or-nothing attachment limit. Resolve NativeChatComposer through its existing file-drop hook, keep the host-qualified attachment resolver with the current workspace state type, retain captured destinations and unmount checks through attachment reads and uploads, and keep attachment actions picker-only without a broadcast subscriber. Update picker fixtures and the upstream subscriber test. Validation: web and Node typechecks, full Oxlint on 39 PR3 files, and 372 tests across 45 explicitly named files pass.
…omposer-feedback' into brennanb2025/sta-6940-pr4-terminal-owners
Resolve NativeChatComposerField imports: keep PR3's flushSync (interrupted composition settlement) and drop the old ImageOff notice import that main's notice card replaced. Adapt PR3's IME file-drop test to main's notices and @-file suggestion props.
…or own OS drops STA-6940 PR5. The file explorer tree, the project sidebar, each editor group's tab strip and its editor area now take OS file drops on their own element through the shared owner hook, with identity from their own render (the explorer's shown workspace and target row, the tab strip's and editor group's worktree and group) instead of the active worktree. The explorer and sidebar broadcast subscribers and their legacy drop markers are deleted; editor-open moves out of useGlobalFileDrop into editor-dropped-file-open, which the legacy unmarked-chrome route still uses until PR6.
Contributor
Author
Review summaryThis PR went through an independent code review, two fix rounds each followed by a focused re-review, and a rendered check in a real Orca window. The reviewers and the rendered-check agent did not write the code. Their reports are summarized here because they were kept off GitHub while the work was in progress. First review (head
|
Contributor
Author
Ready for reviewPR 3 (#25781) and PR 4 (#26008) are now merged, so this PR no longer depends on anything open. It was brought up to date with
The review summary is in the comment above and the rendered-check screenshots are in the description. |
brennanb2025
marked this pull request as ready for review
October 8, 2026 00:50
Contributor
📝 WalkthroughWalkthroughNative file drops now use OS-drop owners in editor groups, the file explorer, and the project sidebar. Editor drops capture worktree and group ownership before opening local files or uploading remote files. Explorer drops capture a destination and operation guard before importing prepared paths. Floating-workspace operation routing is shared across file-operation ownership and runtime-owner checks. Priority: ➖ Normal Merge Risk: 🔵 Low · up to If a remote workspace changes owner during a file drop, the editor may open a tab for the previous host, or the explorer may select a stale import result. These bounded cases warrant fixes or explicit acceptance before merge. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
1b8a15a8-397d-4c54-ac7a-a77177b44aea
📒 Files selected for processing (41)
src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.tssrc/renderer/src/components/editor/editor-dropped-file-open.test.tssrc/renderer/src/components/editor/editor-dropped-file-open.tssrc/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsxsrc/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsxsrc/renderer/src/components/editor/use-editor-group-file-drop-owner.tssrc/renderer/src/components/right-sidebar/FileExplorer.tsxsrc/renderer/src/components/right-sidebar/FileExplorerFilesTreePane.tsxsrc/renderer/src/components/right-sidebar/FileExplorerRow.tsxsrc/renderer/src/components/right-sidebar/file-explorer-operation-generation.test.tssrc/renderer/src/components/right-sidebar/file-explorer-operation-owner.tssrc/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.tssrc/renderer/src/components/right-sidebar/useFileExplorerDragDrop.os-file-drop.test.tsxsrc/renderer/src/components/right-sidebar/useFileExplorerDragDrop.tssrc/renderer/src/components/right-sidebar/useFileExplorerImport.committed-scope.test.tsxsrc/renderer/src/components/right-sidebar/useFileExplorerImport.os-file-drop.test.tsxsrc/renderer/src/components/right-sidebar/useFileExplorerImport.remote.test.tsxsrc/renderer/src/components/right-sidebar/useFileExplorerImport.tssrc/renderer/src/components/right-sidebar/useFileExplorerRowDrag.tssrc/renderer/src/components/sidebar/Sidebar.test.tsxsrc/renderer/src/components/sidebar/index.tsxsrc/renderer/src/components/sidebar/useSidebarProjectDrop.os-file-drop.test.tsxsrc/renderer/src/components/sidebar/useSidebarProjectDrop.tssrc/renderer/src/components/tab-bar/TabBar.os-file-drop.test.tsxsrc/renderer/src/components/tab-bar/TabBar.tsxsrc/renderer/src/components/tab-bar/tab-bar-surface.tsxsrc/renderer/src/components/tab-group/TabGroupPanel.os-file-drop.test.tsxsrc/renderer/src/components/tab-group/TabGroupPanel.tsxsrc/renderer/src/hooks/use-os-file-drop-owner.test.tsxsrc/renderer/src/hooks/use-os-file-drop-owner.tssrc/renderer/src/hooks/useGlobalFileDrop.test.tssrc/renderer/src/hooks/useGlobalFileDrop.tssrc/renderer/src/lib/editor-file-operation-owner.tssrc/renderer/src/lib/local-file-access.test.tssrc/renderer/src/lib/open-document-in-floating-workspace.tssrc/renderer/src/lib/remote-workspace-drop-test-fixtures.tssrc/renderer/src/lib/ssh-mutation-expectation.test.tssrc/renderer/src/lib/ssh-mutation-expectation.tssrc/renderer/src/lib/worktree-operation-route.tssrc/renderer/src/lib/worktree-runtime-owner.test.tssrc/renderer/src/lib/worktree-runtime-owner.ts
💤 Files with no reviewable changes (1)
- src/renderer/src/components/right-sidebar/FileExplorer.tsx
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
Comment on lines
+177
to
+197
| for (const result of results) { | ||
| if (!editorGroupStillExists(destination)) { | ||
| return | ||
| } | ||
| if (result.status !== 'imported' || result.kind === 'directory') { | ||
| continue | ||
| } | ||
| const maybeRelative = toWorktreeRelativePath(result.destPath, worktreePath) | ||
| store.setActiveTabType('editor', worktreeId) | ||
| store.openFile( | ||
| { | ||
| filePath: result.destPath, | ||
| relativePath: maybeRelative ?? result.destPath, | ||
| worktreeId, | ||
| runtimeEnvironmentId: runtimeEnvironmentId ?? undefined, | ||
| language: detectLanguage(result.destPath), | ||
| mode: 'edit' | ||
| }, | ||
| { suppressActiveRuntimeFallback: runtimeEnvironmentId === null, ...groupOptions } | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '68,258p' src/renderer/src/components/editor/editor-dropped-file-open.ts
rg -n 'function importExternalPathsToRuntime|const importExternalPathsToRuntime|assertCurrent|function statUserOpenedPath' src/renderer/srcRepository: stablyai/orca
Length of output: 18681
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- target imports and declarations ---'
sed -n '1,120p' src/renderer/src/components/editor/editor-dropped-file-open.ts
sed -n '1,250p' src/renderer/src/runtime/runtime-file-import-client.ts
sed -n '1,120p' src/renderer/src/lib/user-opened-local-path.ts
printf '%s\n' '--- callers and owner/context references ---'
rg -n -F -- 'captureEditorFileDropOpen' src/renderer/src || true
rg -n -F -- 'captureWorktreeSshMutationExpectation' src/renderer/src || true
rg -n -F -- 'openEditorFileDropPaths' src/renderer/src || true
printf '%s\n' '--- focused tests ---'
rg -n -F -- 'editor-dropped-file-open' src/renderer/src --glob '*test*' || true
rg -n -F -- 'ownerChanged' src/renderer/src/components src/renderer/src/runtime --glob '*test*' || true
rg -n -F -- 'importExternalPathsToRuntime' src/renderer/src/components src/renderer/src/runtime --glob '*test*' || true
printf '%s\n' '--- openFile definitions and runtime resolution ---'
rg -n -F -- 'openFile:' src/renderer/src/store src/renderer/src --glob '*.ts' --glob '*.tsx' || true
rg -n -F -- 'function openFile' src/renderer/src --glob '*.ts' --glob '*.tsx' || trueRepository: stablyai/orca
Length of output: 42337
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- SSH expectation and runtime stat routing ---'
sed -n '1,240p' src/renderer/src/lib/ssh-mutation-expectation.ts
rg -n -F -- 'function statRuntimePath' src/renderer/src/runtime src/renderer/src
sed -n '1,180p' src/renderer/src/runtime/runtime-file-metadata-client.ts
sed -n '1,180p' src/renderer/src/runtime/runtime-file-routing.ts
printf '%s\n' '--- editor drop tests ---'
sed -n '1,280p' src/renderer/src/components/editor/editor-dropped-file-open.test.ts
sed -n '1,220p' src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx
sed -n '1,240p' src/renderer/src/components/editor/use-editor-group-file-drop-owner.test.tsx
sed -n '1,180p' src/renderer/src/components/editor/editor-dropped-file-open.escaping-link.test.ts
printf '%s\n' '--- importer ownership tests ---'
sed -n '240,470p' src/renderer/src/runtime/runtime-file-client-external-import.test.ts
printf '%s\n' '--- store open behavior ---'
sed -n '1,180p' src/renderer/src/store/slices/editor/actions/open-file-action.ts
sed -n '1,100p' src/renderer/src/store/slices/editor/types/editor-files-slice.tsRepository: stablyai/orca
Length of output: 42963
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- filesystem IPC consumers and owner checks ---'
rg -n -F -- 'importExternalPaths' src --glob '*.ts' --glob '*.tsx' || true
rg -n -F -- 'expectedSshConnectionGeneration' src --glob '*.ts' --glob '*.tsx' || true
rg -n -F -- 'expectedExecutionHostId' src --glob '*.ts' --glob '*.tsx' || true
rg -n -F -- 'fs.stat' src --glob '*.ts' --glob '*.tsx' || true
printf '%s\n' '--- open-file state application ---'
sed -n '1,280p' src/renderer/src/store/slices/editor/actions/open-file-apply.ts
printf '%s\n' '--- exact remote owner-change test range ---'
rg -n -F -- 'refuses when the workspace runtime owner changes' src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx || true
sed -n '180,360p' src/renderer/src/components/editor/use-editor-group-file-drop-owner.remote.test.tsx
printf '%s\n' '--- exact post-stat and post-import test ranges ---'
rg -n -F -- 'during upload' src/renderer/src/components/editor src/renderer/src/runtime --glob '*.test.ts' --glob '*.test.tsx' || true
rg -n -F -- 'during stat' src/renderer/src/components/editor --glob '*.test.ts' --glob '*.test.tsx' || trueRepository: stablyai/orca
Length of output: 41794
🏁 Script executed:
set -o pipefail
printf '%s\n' '--- filesystem mutation validation ---'
rg -n --glob '*.ts' --glob '*.tsx' 'expectedSshConnectionGeneration|expectedExecutionHostId|importExternalPathsSsh' src/main src/shared src/preload
sed -n '130,220p' src/main/ipc/filesystem-mutations.ts
sed -n '1,130p' src/main/ipc/filesystem-import-ssh.ts
sed -n '1,180p' src/main/ipc/filesystem-import-ssh-ops.test.ts
printf '%s\n' '--- editor operation provenance ---'
rg -n --glob '*.ts' --glob '*.tsx' 'captureEditorFileOperationProvenance' src/renderer/src
sed -n '1,260p' src/renderer/src/lib/editor-file-operation-owner.ts
printf '%s\n' '--- runtime import guard ---'
sed -n '1,180p' src/renderer/src/runtime/runtime-file-mutation-rpc.tsRepository: stablyai/orca
Length of output: 43022
Recheck the worktree owner after each awaited operation.
The runtime importer and SSH upload path guard their own runtime pairing and SSH generation. They do not replace a final check that the editor worktree still has the captured owner. A worktree owner can change while the import or stat is pending. The current code then checks only the group and can open a stale path or runtime tab.
Recapture the context after importExternalPathsToRuntime and statUserOpenedPath. Stop opening when sameEditorFileDropOwner returns false. Preserve the existing upload-failure notification.
Suggested owner checks
async function openEditorFileDropPaths(
- { fileContext, worktreePath, connectionId }: EditorFileDropContext,
+ captured: EditorFileDropContext,
destination: EditorFileDropDestination,
paths: readonly string[]
): Promise<void> {
+ const { fileContext, worktreePath, connectionId } = captured
const store = useAppStore.getState()
...
if (!editorGroupStillExists(destination)) {
return
}
- for (const result of results) {
+ const current = captureEditorFileDropContext(worktreeId)
+ const ownerStillCurrent =
+ current !== null && sameEditorFileDropOwner(captured, current)
+ if (!ownerStillCurrent) {
+ showOwnerChangedError()
+ }
+ for (const result of ownerStillCurrent ? results : []) {
...
const stat = await statUserOpenedPath(fileContext, filePath)
if (!editorGroupStillExists(destination)) {
return
}
+ const current = captureEditorFileDropContext(worktreeId)
+ if (!current || !sameEditorFileDropOwner(captured, current)) {
+ showOwnerChangedError()
+ return
+ }
Comment on lines
+109
to
+149
| expectedExecutionHostId: guard.route.expectedExecutionHostId, | ||
| expectedSshTargetId: guard.route.expectedSshTargetId, | ||
| expectedSshConnectionGeneration: guard.route.expectedSshConnectionGeneration | ||
| }, | ||
| prepared.paths, | ||
| destinationDir, | ||
| { assertCurrent: guard.assertCurrent } | ||
| ) | ||
|
|
||
| const { paths, destinationDir } = data | ||
|
|
||
| void (async () => { | ||
| try { | ||
| if (getRelativePathInsideRoot(destinationDir, displayRootRef.current) === null) { | ||
| return | ||
| } | ||
| const operationGuard = captureFileExplorerOperationGuard(wtId, operationOwnerRef.current) | ||
| operationGuard.assertCurrent() | ||
| const { results } = await importExternalPathsToRuntime( | ||
| { | ||
| settings: operationGuard.route.settings, | ||
| worktreeId: wtId, | ||
| worktreePath: worktreePathRef.current, | ||
| connectionId: operationGuard.route.connectionId, | ||
| expectedExecutionHostId: operationGuard.route.expectedExecutionHostId, | ||
| expectedSshTargetId: operationGuard.route.expectedSshTargetId, | ||
| expectedSshConnectionGeneration: operationGuard.route.expectedSshConnectionGeneration | ||
| }, | ||
| paths, | ||
| destinationDir, | ||
| { assertCurrent: operationGuard.assertCurrent } | ||
| ) | ||
|
|
||
| // Refresh the destination directory once per gesture | ||
| await refreshDirRef.current(destinationDir) | ||
| // Refresh the destination directory once per gesture | ||
| await refreshDir(destinationDir) | ||
|
|
||
| // Why: only select (highlight) the first imported file — don't trigger | ||
| // the full reveal machinery because watcher refreshes can otherwise | ||
| // snap the tree viewport away from the user's drop target. | ||
| const imported = results.filter((r) => r.status === 'imported') | ||
| const skipped = results.filter((r) => r.status === 'skipped') | ||
| const failed = results.filter((r) => r.status === 'failed') | ||
| // Why: only select (highlight) the first imported file — don't trigger | ||
| // the full reveal machinery because watcher refreshes can otherwise | ||
| // snap the tree viewport away from the user's drop target. | ||
| const imported = results.filter((r) => r.status === 'imported') | ||
| const skipped = results.filter((r) => r.status === 'skipped') | ||
| const failed = results.filter((r) => r.status === 'failed') | ||
|
|
||
| if ( | ||
| imported.length > 0 && | ||
| activeWorktreeIdRef.current === wtId && | ||
| getRelativePathInsideRoot(imported[0].destPath, displayRootRef.current) !== null | ||
| ) { | ||
| setSelectedPathRef.current(imported[0].destPath) | ||
| } | ||
| const shown = shownRef.current | ||
| if ( | ||
| imported.length > 0 && | ||
| shown.worktreeId === destination.worktreeId && | ||
| getRelativePathInsideRoot(imported[0].destPath, shown.displayRootPath) !== null | ||
| ) { | ||
| setSelectedPath(imported[0].destPath) | ||
| } | ||
|
|
||
| if (failed.length > 0) { | ||
| const noun = failed.length === 1 ? 'file' : 'files' | ||
| toast.error( | ||
| translate( | ||
| 'auto.components.right.sidebar.useFileExplorerImport.132fd0e1e9', | ||
| 'Failed to import {{value0}} {{value1}}.', | ||
| { value0: failed.length, value1: noun } | ||
| ) | ||
| if (failed.length > 0) { | ||
| const noun = failed.length === 1 ? 'file' : 'files' | ||
| toast.error( | ||
| translate( | ||
| 'auto.components.right.sidebar.useFileExplorerImport.132fd0e1e9', | ||
| 'Failed to import {{value0}} {{value1}}.', | ||
| { value0: failed.length, value1: noun } | ||
| ) | ||
| } else if (skipped.length > 0 && imported.length === 0) { | ||
| const noun = skipped.length === 1 ? 'file' : 'files' | ||
| toast.error( | ||
| translate( | ||
| 'auto.components.right.sidebar.useFileExplorerImport.25919b2050', | ||
| 'Skipped {{value0}} {{value1}}.', | ||
| { value0: skipped.length, value1: noun } | ||
| ) | ||
| ) | ||
| } else if (skipped.length > 0 && imported.length === 0) { | ||
| const noun = skipped.length === 1 ? 'file' : 'files' | ||
| toast.error( | ||
| translate( |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
sed -n '65,165p' src/renderer/src/components/right-sidebar/useFileExplorerImport.ts
rg -n 'importExternalPathsToRuntime|assertCurrent|assert.*Guard' src/renderer/src/lib src/renderer/src/components/right-sidebarRepository: stablyai/orca
Length of output: 9233
🏁 Script executed:
printf '%s\n' '--- runtime-file-client references and implementation ---'
rg -n -F -- 'function importExternalPathsToRuntime' src/renderer/src/runtime src/renderer/src || test "$?" -eq 1
rg -n -F -- 'importExternalPathsToRuntime' src/renderer/src/runtime src/renderer/src/lib src/renderer/src/components/right-sidebar || test "$?" -eq 1
printf '%s\n' '--- operation guard implementation ---'
sed -n '1,240p' src/renderer/src/components/right-sidebar/file-explorer-operation-owner.ts
sed -n '1,150p' src/renderer/src/lib/worktree-operation-generation.ts
printf '%s\n' '--- import caller and focused tests ---'
sed -n '1,210p' src/renderer/src/components/right-sidebar/useFileExplorerImport.ts
sed -n '1,280p' src/renderer/src/components/right-sidebar/useFileExplorerImport.remote.test.tsx
sed -n '1,230p' src/renderer/src/components/right-sidebar/useFileExplorerImport.os-file-drop.test.tsxRepository: stablyai/orca
Length of output: 37063
🏁 Script executed:
sed -n '1,260p' src/renderer/src/runtime/runtime-file-client.ts
sed -n '1,240p' src/renderer/src/components/right-sidebar/file-explorer-operation-owner.ts
sed -n '1,150p' src/renderer/src/lib/worktree-operation-generation.ts
sed -n '1,210p' src/renderer/src/components/right-sidebar/useFileExplorerImport.ts
sed -n '1,280p' src/renderer/src/components/right-sidebar/useFileExplorerImport.remote.test.tsx
sed -n '1,230p' src/renderer/src/components/right-sidebar/useFileExplorerImport.os-file-drop.test.tsxRepository: stablyai/orca
Length of output: 34475
🏁 Script executed:
printf '%s\n' '--- importer implementation ---'
sed -n '1,320p' src/renderer/src/runtime/runtime-file-import-client.ts
printf '%s\n' '--- importer tests using assertCurrent and mutation ownership ---'
rg -n -F -- 'assertCurrent' src/renderer/src/runtime/runtime-file-import*.test.ts
sed -n '1,360p' src/renderer/src/runtime/runtime-file-client-mutation-ownership.test.ts
printf '%s\n' '--- importer external tests around all calls ---'
sed -n '1,470p' src/renderer/src/runtime/runtime-file-client-external-import.test.tsRepository: stablyai/orca
Length of output: 35212
🏁 Script executed:
sed -n '1,320p' src/renderer/src/runtime/runtime-file-import-client.ts
rg -n -F -- 'assertCurrent' src/renderer/src/runtime/runtime-file-import*.test.ts
sed -n '1,360p' src/renderer/src/runtime/runtime-file-client-mutation-ownership.test.tsRepository: stablyai/orca
Length of output: 20038
🏁 Script executed:
printf '%s\n' '--- runtime import session and mutation helpers ---'
rg -n -F -- 'createRuntimeImportSessionGuard' src/renderer/src/runtime
rg -n -F -- 'function createRuntimeImportSessionGuard' src/renderer/src/runtime
sed -n '1,360p' src/renderer/src/runtime/runtime-file-mutation-rpc.ts
sed -n '1,360p' src/renderer/src/runtime/runtime-file-upload-client.ts
printf '%s\n' '--- fs import IPC binding and implementation ---'
rg -n -F -- 'importExternalPaths' src/main src/shared src/renderer | head -80
rg -n -F -- 'expectedSshConnectionGeneration' src/main src/shared | head -120Repository: stablyai/orca
Length of output: 25600
🏁 Script executed:
sed -n '1,360p' src/renderer/src/runtime/runtime-file-mutation-rpc.ts
sed -n '1,360p' src/renderer/src/runtime/runtime-file-upload-client.ts
rg -n -F -- 'importExternalPaths' src/main src/shared src/renderer | head -80
rg -n -F -- 'expectedSshConnectionGeneration' src/main src/shared | head -120Repository: stablyai/orca
Length of output: 25067
🏁 Script executed:
printf '%s\n' '--- filesystem import implementation ---'
rg --files src/main src/renderer/src | rg 'filesystem.*import|import.*filesystem|filesystem-import'
rg -n -F -- 'fs:importExternalPaths' src/main src
rg -n -F -- 'importExternalPaths:' src/main src
printf '%s\n' '--- refreshDir bindings for useFileExplorerImport ---'
rg -n -F -- 'refreshDir:' src/renderer/src/components/right-sidebar
rg -n -F -- 'useFileExplorerImport({' src/renderer/src/components/right-sidebarRepository: stablyai/orca
Length of output: 10358
🏁 Script executed:
rg --files src/main src/renderer/src | rg 'filesystem.*import|import.*filesystem|filesystem-import'
rg -n -F -- 'fs:importExternalPaths' src/main src
rg -n -F -- 'importExternalPaths:' src/main src
rg -n -F -- 'useFileExplorerImport({' src/renderer/src/components/right-sidebarRepository: stablyai/orca
Length of output: 6995
🏁 Script executed:
sed -n '130,220p' src/main/ipc/filesystem-mutations.ts
sed -n '1,260p' src/main/ipc/filesystem-import-ssh.ts
sed -n '1,240p' src/main/ipc/filesystem-import-ssh-directory.ts
sed -n '190,255p' src/renderer/src/components/right-sidebar/use-file-explorer-tree-pane-state.tsRepository: stablyai/orca
Length of output: 16555
🏁 Script executed:
rg -n -F -- 'const refreshDir' src/renderer/src/components/right-sidebar src/renderer/src
rg -n -F -- 'function refreshDir' src/renderer/src/components/right-sidebar src/renderer/src
rg -n -F -- 'refreshDir =' src/renderer/src/components/right-sidebar src/renderer/src
sed -n '1,240p' src/renderer/src/components/right-sidebar/useFileExplorerTree.ts
sed -n '1,240p' src/renderer/src/components/right-sidebar/useFileExplorerWatch.tsRepository: stablyai/orca
Length of output: 34011
🏁 Script executed:
sed -n '250,335p' src/renderer/src/components/right-sidebar/useFileExplorerTree.ts
sed -n '1,260p' src/renderer/src/components/right-sidebar/file-explorer-directory-listing.tsRepository: stablyai/orca
Length of output: 4489
Re-check the operation guard before refreshing and selecting the import result.
guard.assertCurrent() runs before the import, but the importer can resolve after its final awaited write. If the runtime or SSH generation changes in that interval, this callback can still refresh and select the returned path. The selection check only compares the worktree ID and display root.
Suggested fix
const { results } = await importExternalPathsToRuntime(
{
settings: guard.route.settings,
worktreeId: destination.worktreeId,
@@
{ assertCurrent: guard.assertCurrent }
)
+ guard.assertCurrent()
// Refresh the destination directory once per gesture
await refreshDir(destinationDir)
+ guard.assertCurrent()
// Why: only select (highlight) the first imported file — don't trigger
7 of 9 tasks
brennanb2025
deleted the
brennanb2025/sta-6940-pr5-explorer-editor-owners
branch
October 8, 2026 02:03
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.
ELI5
When you drag a file in from Finder, Explorer or a file manager, Orca should act on the exact spot where you let go. Before this PR, the file tree, the project sidebar, the editor's tab row and the editor itself never handled the drop themselves. A global listener guessed where the drop was meant to go, and it often guessed "the active workspace". This PR makes each of those four places handle drops on itself, so a file lands where you dropped it.
This is PR 5 of 6 for STA-6940.
PR 3 (#25781) and PR 4 (#26008) have merged; this PR has no open-PR dependency. Main commit
939288e51d342463df636f2e962e9e63a8c6c2f5was merged into this branch without rebasing. The original PR 5 migration is commitae8d3ab871a, followed by its review fixes; the final diff against the merged main commit is byte-for-byte identical to PR 5's reviewed changes afterb4aa7f46ee4.The problem
What you saw: a file dragged in from the operating system sometimes landed somewhere other than where you dropped it:
Why: Orca's preload script (the bridge between the page and Electron) caught every OS file drop at the top of the page and cancelled it, so no element ever saw the real event. It recorded rough location hints from
data-native-file-drop-targetattributes and sent them to the main process. Main sent the drop back to the whole window, where several listeners each guessed whether it was meant for them. The explorer and editor-open listener (useGlobalFileDrop) read the active worktree when the drop arrived, not the one under your cursor. The project sidebar also received the window-wide broadcast, even though its action is simply to add a local project.The review also found a later timing gap: closing the destination split while Orca checked a local file or uploaded it still let the completed operation request an open in that closed group. Tab placement could then use the active split, redirecting the dropped file. This follow-up closes that window too.
What changed (as a user)
dragover; preload did not stop that event from reaching React. The existing "Server runtime active / Use Add Project for host paths" drag hint stays. The existing unavailable-folder toast remains for a runtime becoming active after a local drop was accepted, including during preparation. This is verified from the base source; real operating-system drag dispatch has not been checked in Electron.Mechanism
Each surface now uses the shared owner hook from PR 2 (
useOsFileDropOwner). The hook listens on the element itself, in the capture phase (it runs before any child element, including the code editor). It reads the dropped paths through the preloadgetPathForFilebridge, then asks main to prepare them (fs:prepareDroppedPaths, which copies macOS screenshot temp files). Each surface reads its identity from its own render, never from the active workspace.useFileExplorerImport.ts): the tree root is the drop handler. At drop time it reads the target folder from the row under the cursor (data-file-explorer-drop-dir, falling back to the displayed root folder). It captures the explorer's host check (captureFileExplorerOperationGuard) at that moment, before rows can recycle, then reuses the existing import path. That path covers local, SSH, remote runtime and folder workspaces, and re-checks the host after every await. Rows now handle only drags that start inside Orca. The native-drop highlight cleanup runs on the drop handler's path. Edge auto-scroll moves to a capture handler, because the drop handler stops nativedragoverbefore it bubbles. The broadcast subscriber, its use of the active-worktree ref, and thedata-native-file-drop-*markers are deleted.useSidebarProjectDrop.ts,sidebar/index.tsx): the sidebar is the drop handler while it is open. It refuses with "not allowed" while a remote runtime is active, and re-reads that setting after the drop is prepared. The broadcast subscriber and marker are deleted.use-editor-group-file-drop-owner.tsgives both elements of an editor group one shared ordering queue (PR 2's sequence object), keyed byprops.worktreeId+ group. A tab-row drop and then an editor-area drop are applied in drop order, even if the first takes longer to prepare. The editor-open/upload logic moved out ofuseGlobalFileDropintoeditor-dropped-file-open.ts. It now takes an explicit worktree and group, captures that worktree's host when you drop, and refuses if the host changes before preparation finishes. It opens files withtargetGroupId. Delivery checks the destination group instead of the editor content element; an optional destination-liveness callback in the shared hook empties successful paths for a closed group but preserves preparation failures. Owners without that callback retain the existing root-detachment behavior. The open step also checks that the captured group still exists immediately after each awaited local file check or remote upload and before opening files, so tab placement never receives a closed target group. The floating workspace uses the shared local-owner rule and the existing floating-document open function, which pins reads to this machine and suppresses focused-runtime fallback. Its explicit permanent-tab option matches the prior default.getRuntimeEnvironmentIdForWorktree,getKnownExecutionHostIdForWorktreeand the explorer's operation-owner reader now readgetFloatingWorkspaceOperationRoute, as editor provenance and SSH expectations already do. These existing readers serve floating-workspace operations beyond file drops; only their existing floating special cases change internally, with identical public return values and non-floating behavior. This consolidation has no user-visible behavior change: floating file operations remain local regardless of the focused runtime. No other callers or features were migrated.useGlobalFileDropnow keeps only the rejection toasts and the legacy unmarked-chrome route. That route calls the extracted module with the active worktree, preserving its destination behavior; local multi-file opens now run in order as described above.useOsFileDropOwner'scanAcceptnow receives the drag event, so the file tree can refuse based on the row under the cursor.Why this approach
The plan (STA-6940, approved) moves every surface to handling drops on its own element, because the window-wide broadcast with string filters is what let drops reach the wrong destination. Patching the active-worktree reads one by one would leave the broadcast in place for the next surface to get wrong. The tab row and editor area share one queue because they are one destination. Two queues would let a slow drop on one land after a later drop on the other.
Differences from the common pattern
fs:prepareDroppedPaths, consumermain-reader). The file tree, editor and sidebar may read an uncopied original, as main allows for that consumer.useGlobalFileDrop's legacy route for unmarked chrome, and the editor shown before a workspace has a layout (TerminalLegacyEditorSurface, still unmarked), still open in the active worktree. PR 6 removes the preload drop interception,onFileDrop, the remaining markers anduseGlobalFileDrop.Linked Issue
Linear: STA-6940 (maintainer PR; no GitHub issue).
Visual Proof
A separate QA agent ran this branch (head
e767be62ca0) as a hidden Orca dev window with an isolated profile and two folder workspaces (ws-a and ws-b), and simulated OS file drops through the Chrome DevTools Protocol. All 8 scenarios passed: explorer folder-row and empty-area drops, an explorer drop in the non-active workspace, the editor area, the tab strip, split editor groups, the project sidebar (which shows its usual "open as folder" confirmation for a non-Git folder), and the floating panel's editor. The later round-2 fixes only add re-checks for a group closed mid-upload and consolidate the floating-workspace rule; they do not change these paths.1. File dropped on the explorer's
inboxfolder row. It is imported into that folder.2. Explorer drop in the other workspace. With ws-b shown in the explorer, the dropped file lands in ws-b's
inbox, not in ws-a.3. File dropped on the editor content area. It opens as its own tab (
my report.txt); the file already open (readme.txt) is unchanged, so its text was not inserted.4. Split editor groups. With the left group focused, a file dropped on the right group's tab strip opens in the right group.
Not covered: real SSH, WSL and remote-runtime hosts, Linux and Windows (covered by unit fixtures only).
Testing
Final sync validation (head
2ef4f37349a42118bf4580fbda72e50891b72ede): merged main939288e51d342463df636f2e962e9e63a8c6c2f5after PR 3 and PR 4 merged. Nine conflicted files take main unchanged;use-os-file-drop-owner.tstakes main plus exactly PR 5's event-aware acceptance and destination-liveness changes. Every auto-merged file was also audited: the final 41-file diff is byte-for-byte identical to the saved, reviewed PR 5 diff, with no stale PR 3/4 code or extra feature change.dropScopeKeyis absent, and pnpm's generated@pnpm/exelockfile block was discarded.All 70 named test files / 596 tests passed, including PR 5's named suites, the conflicted modules, PR 1–4 drop regressions, and all three previously timing-out consumer suites. No current timeout remains and no timeout was increased. The command used a non-empty Bash file array, checked every path existed, and ran
node_modules/.bin/vitest run --config config/vitest.config.ts "${F[@]}"withORCA_BACKGROUND_LAUNCH=1.Full
pnpm run typecheckpassed. The post-commitorca-ci-checks --no-typecheckrun passed 22/22 static checks, using that already-passed full typecheck. The initial pre-commit React Doctor run included changes from main through the old merge base; it passed after the merge commit gave the changed-lines checks the correct base. No application was launched during this sync.GitHub CI: full-unit run
37707844544completed successfully on the first attempt for head2ef4f37349a42118bf4580fbda72e50891b72ede. All ten Node 24 test shards, relay integration, static analysis/typecheck, both packaging jobs, repository guards, final verification and selection evidence passed. Across the PR, 18 checks passed and 17 were skipped by their workflow conditions, with no pending or failed checks. No rerun was needed. The PR remains draft, with no merge or auto-merge requested.The following records describe earlier heads and remain as historical evidence.
Round 2 validation: the two additional caller suites,
src/renderer/src/lib/worktree-runtime-owner.test.tsandsrc/renderer/src/components/right-sidebar/file-explorer-operation-generation.test.ts, passed 39/39 tests. They cover floating ownership with a different runtime focused and the existing non-floating cases. At follow-up commit5ed9268b8f2888226531a5fa3786f16b414f2d22, the required 33-file sample: 30 files passed / 3 files timed out; 248 passed / 14 timed out / 262 tests in 746.77 seconds. All new late-close tests and the other 30 files passed. The timeouts are confined to the three consumer files in the isolated comparison below (one tab-strip test, six markdown-editor setup calls and seven tab-lifecycle setup calls). This run is not fully green.Late group-close regression evidence: with only the new tests added and the opener still at
e767be62ca0, the two named owner-hook files produced 12 passed / 4 failed. Both delayed-local-check cases (group removal and removal of that workspace's groups) and both delayed-upload cases (SSH and runtime) failed becauseopenFilewas called for a closed group. The tests use the actual owner delivery path and preserve a preparation-failure toast delivered before the later operation begins. After the fix, both owner-hook files passed 16/16 tests in the 33-file sample, including all four negative-control cases.Isolated timeout comparison: ran exactly
TabBar.os-file-drop.test.tsx,FloatingTerminalPanel.markdown-editor.test.tsxandFloatingTerminalPanel.tab-lifecycle.test.tsxin one Bash-array Vitest command, with no other checks running from this worker. Ate767be62ca0, 20 passed / 3 timed out / 23 tests in 124.15 seconds: the tab-strip dynamic import hit 30 seconds and the first setup in each floating suite hit 60 seconds. On PR 4 baseb4aa7f46ee4, the same three-file command gave 18 passed / 5 timed out / 23 tests in 172.57 seconds: the same tab-strip timeout and two setup timeouts in each floating suite. The base checkout was created withorca worktree create --parent-worktree active --setup skip, detached at the base, and removed after the run; only the PR 5 tab-strip test (absent on PR 4) was copied into it, with no production source changes. This reproduces the import/setup blocker before PR 5; its cause is still unverified, and it is recorded separately from file-drop correctness. No timeout limit was increased and no queue was bypassed.Every Vitest command uses explicitly named files in a Bash array,
node_modules/.bin/vitest run --config config/vitest.config.ts "${F[@]}", andORCA_BACKGROUND_LAUNCH=1.pnpm tc:webpassed. Fullpnpm exec oxlinton all seven touched files andpnpm run check:react-doctor:changedpassed; commit hooks also passed lint and formatting.pnpm run check:code-quality:changedpassed with zero new findings, including its type-aware, React Doctor, design-system and checked-cast scans.orca-ci-checks --no-typecheckcompleted 21/22 checks. The full repository type-aware audit never started: the normal machine-wide gate could not obtain a slot for 1802 seconds and exited 75, without code diagnostics. All other checks passed, including repository lint, code-quality policies, localization and generated-file checks. That earlier push did not await GitHub CI; the final sync result above covers the current head. No queue was bypassed, and the PR stays draft. No new user-visible strings were added, andpnpm-lock.yamlwas restored from HEAD and excluded from the commit.Previous-round recorded validation: 33 explicitly named Vitest files, 258 tests passed at
e767be62ca0; the re-review and current runs below reproduce timeouts, so this is historical evidence rather than a claim that the current sample is fully green. Commands used a Bash array andpnpm exec vitest run --config config/vitest.config.ts, withORCA_BACKGROUND_LAUNCH=1on the final run. This includes all 12 new/moved files requested by the review and its 14-file regression sample:right-sidebar/useFileExplorerImport.os-file-drop.test.tsx,right-sidebar/useFileExplorerImport.committed-scope.test.tsx,sidebar/useSidebarProjectDrop.os-file-drop.test.tsx,editor/use-editor-group-file-drop-owner.test.tsx,tab-bar/TabBar.os-file-drop.test.tsx,tab-group/TabGroupPanel.os-file-drop.test.tsx,editor/editor-dropped-file-open.test.ts,editor/editor-dropped-file-open.escaping-link.test.ts,hooks/useGlobalFileDrop.test.ts,hooks/use-os-file-drop-owner.test.tsx,sidebar/Sidebar.test.tsx,lib/local-file-access.test.ts.NewWorkspaceComposerCard.file-drop.test.tsx,native-chat/native-chat-element-file-drop.test.tsx,native-chat/native-chat-pane-file-drop.test.tsx,native-chat/native-chat-floating-file-drop.test.tsx,terminal-pane/terminal-pane-element-file-drop.test.tsx,lib/os-file-drop-preload-integration.test.tsx,TerminalTitlebarTabs.test.tsx,tab-group/TabGroupSplitLayout.drag.test.tsx,floating-terminal/FloatingTerminalPanel.markdown-editor.test.tsx,floating-terminal/FloatingTerminalPanel.tab-lifecycle.test.tsx,right-sidebar/FileExplorerFilesTreePane.errors.test.tsx,right-sidebar/useFileExplorerDragDrop.test.ts,right-sidebar/useFileExplorerRowDrag.test.ts,right-sidebar/FileExplorerVirtualRows.row-handlers.test.tsx.editor/use-editor-group-file-drop-owner.remote.test.tsx,right-sidebar/useFileExplorerImport.remote.test.tsx,right-sidebar/useFileExplorerDragDrop.os-file-drop.test.tsx,lib/ssh-mutation-expectation.test.ts,lib/open-document-in-floating-workspace.test.ts,lib/editor-file-operation-owner.test.ts,lib/worktree-operation-route.test.ts.hooks/andlib/paths are undersrc/renderer/src/; other paths above are undersrc/renderer/src/components/.Before/after evidence for the behavioral review fixes: temporarily restored the three relevant source files from review head
ae8d3ab871awhile retaining the new tests. Five tests failed: editor-area delivery after the content unmounts, preservation of preparation-failure feedback after the group closes, no sequence creation for an uncommitted suspended render, and both destination-liveness cases in the shared hook. Sources were restored afterwards and all these tests pass. The remount test also proves that pending drops retain order when both roots remount. Owners without a destination callback still discard pending delivery after their root detaches, preserving PR 3/4 behavior.Coverage added for existing behavior: SSH and runtime uploads through the actual editor and explorer owner hooks, with another workspace active and a deliberately different runtime focused; SSH reconnection and workspace runtime-owner changes during delayed preparation; explorer guard-capture failure; collapsed sidebar detachment; and edge auto-scroll through the real
FileExplorerFilesTreePaneand Radix ScrollArea root, including the same rendered root element across a re-render. That assertion does not count callback-ref attachments or prove uninterrupted attachment during a pending drop. No real remote host is used by these tests.Original migration evidence, recorded at
ae8d3ab871a: the new destination tests passed on that commit and failed against PR 4 baseb4aa7f46ee4; unchanged internal explorer row drags passed on both. The original run covered 127 named files / 945 tests. This follow-up reran the 33-file sample above, rather than repeating that full earlier run. The editor-drop assertions moved fromuseGlobalFileDrop.test.tsremain unchanged in the extracted module's tests.Previous-round recorded static checks (not the current result):
pnpm tc:webpassed; fullpnpm exec oxlinton the 18 follow-up files passed, including file-length checks;pnpm run check:code-quality:changedreported zero new findings, including React Doctor and design-system checks;pnpm run check:react-doctor:changedpassed. Its existing warning about awaiting files in a loop is intentional: opening them in order keeps tab order deterministic.orca-ci-checkspassed all 23 checks, including localization extraction, catalogs and coverage, repository lint and quality checks, and the fullpnpm run typecheck. A final rerun of the four files affected by the delivery cleanup passed all 32 tests. No new user-visible strings were added; existing translation keys remain in use.Not verified: no Electron or app launch; real operating-system drag dispatch, cross-world events, portals and floating-panel rendering require the separately arranged UI check. Real SSH, WSL and runtime hosts, the web build, and Windows/Linux execution were not tested. The PR remains draft and has not been merged or marked ready.
Review
Author: @BrennanKB5
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Cross-platform: paths are compared with the existing path utilities, with no hard-coded separators. SSH, remote runtime and WSL keep their existing upload and host-check code downstream of the drop handler. Folder workspaces resolve through the same host lookup. Remote mixed versions: no wire or IPC change in this PR.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)