Skip to content

Fix file drop on terminal, opening viewer instead of pasting path (#3729) - #3743

Closed
mrosnerr wants to merge 1 commit into
manaflow-ai:mainfrom
mrosnerr:fix/3729-drag-drop-viewer-regression
Closed

mrosnerr wants to merge 1 commit into
manaflow-ai:mainfrom
mrosnerr:fix/3729-drag-drop-viewer-regression

Conversation

@mrosnerr

@mrosnerr mrosnerr commented May 8, 2026 •

Copy link
Copy Markdown

Summary

  • textDropDestinationKindUnderPoint failed to detect terminals when
    PaneDropTargetView captured the hit-test before GhosttyNSView,
    causing canDropAsText to be false and routing all Finder file drops
    to the file preview panel instead of inserting shell-escaped paths.
  • Adds a pane-target fallback: when terminalUnderPoint misses, check
    whether a PaneDropTargetView with a live hostedView exists at the
    drop point — if so, a terminal is present and text routing applies.

Local testing

All verified manually on a tagged Debug build:

  • ✅ Drag file from Finder onto terminal → pastes shell-escaped path
  • ✅ Shift+drag file onto terminal → opens preview/split
  • ✅ Drag image onto file preview panel → splits with two previews
  • ✅ Drag file onto browser pane → no-op (same as before fix)
  • ✅ Drag file to edge of terminal pane → pastes path, does not split
  • ✅ Drag preview panel tab onto terminal → tab/split operation preserved
  • ✅ Drag multiple files onto terminal → all paths pasted correctly
  • ✅ Inverted setting in Settings → File Drop Behavior → confirmed opposite behavior applies

Fixes #3729


Summary by cubic

Fixes a regression where dropping a file onto the terminal opened the preview viewer instead of inserting a shell-escaped path. Adds a pane-target fallback in hit testing so terminals are detected even when PaneDropTargetView captures the hit test.

  • Bug Fixes
    • If terminalUnderPoint misses, treat a PaneDropTargetView with a non-nil hostedView as a terminal for text drops.
    • Restores expected behavior: file drops paste paths; Shift+drag still opens preview/split; non-terminal panes unchanged.

Written for commit 2d6082a. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes
    • Improved file drop target detection for pane areas to ensure more accurate drop handling.

…manaflow-ai#3729)

textDropDestinationKindUnderPoint could not find the GhosttyNSView when
PaneDropTargetView captured the hit-test fallback, causing canDropAsText
to be false and routing all drops to the file preview panel. Add a pane
target fallback: a live hostedView on the PaneDropTargetView proves a
terminal is present.
@vercel

vercel Bot commented May 8, 2026

Copy link
Copy Markdown

@mrosnerr is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented May 8, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The change refines how file drop destinations are classified in textDropDestinationKindUnderPoint(_:) by explicitly checking for pane drop targets with hosted views and treating them as terminal destinations, replacing a less specific terminal-presence check.

Changes

File Drop Terminal Target Detection

Layer / File(s) Summary
Terminal Target Hit Testing Logic
Sources/FileDropOverlayViewHitTesting.swift
Adds conditional logic to classify pane drop targets with non-nil hosted views as .terminal destinations in the file drop routing chain.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

  • manaflow-ai/cmux#3720: Modifies hit-testing to prefer portal-hosted terminal surfaces so input/drop targeting routes correctly to the terminal.
  • manaflow-ai/cmux#3567: Updates file-drop hit-routing behavior for terminal input in overlay/pane hit-testing.
  • manaflow-ai/cmux#1213: Addresses drop-overlay/hosted-terminal interaction by updating hit-testing to treat PaneDropTargetView-with-hostedView as a terminal target.

Poem

🐰 A pane now knows its place,
With hosted views in each case,
Files drop right where they belong,
To terminals swift and strong! ✨

🚥 Pre-merge checks | ✅ 13 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (13 passed)
Check name Status Explanation
Description check ✅ Passed The description includes comprehensive summary, detailed local testing verification, and all required sections from the template.
Linked Issues check ✅ Passed The PR fully addresses issue #3729 by implementing the pane-target fallback to restore terminal path pasting behavior instead of opening the viewer.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the hit-testing logic for file drops on terminals, directly addressing the scope of issue #3729.
Cmux Swift Actor Isolation ✅ Passed Change adds fallback hit-test logic within @MainActor FileDropOverlayView. No implicit MainActor types, Sendable violations, or background context issues introduced. UI types properly isolated.
Cmux Swift Blocking Runtime ✅ Passed The PR adds 4 lines of simple logic checks with no blocking/timing primitives (semaphores, sleeps, main-queue sync, locks, or polling).
Cmux No Hacky Sleeps ✅ Passed PR fix is in Swift code (out of scope). All non-Swift files with sleeps are added/imported, not modified—explicitly allowed as existing code not introduced by this PR.
Cmux Swift Concurrency ✅ Passed File contains synchronous AppKit hit-testing code only. No legacy async patterns detected: no DispatchQueue, Combine, completion handlers, or Tasks. Uses allowed AppKit boundary APIs.
Cmux Swift @Concurrent ✅ Passed The PR adds 4 synchronous lines to FileDropOverlayView (MainActor-isolated). No async functions or @concurrent annotations. Changes are appropriate UI coordination work.
Cmux Swift File And Package Boundaries ✅ Passed 459-line AppKit hit-testing extension meets boundaries: under 800-line limit, single responsibility, allowed UI glue pattern, focused 4-line bug fix, clear separation via extension file.
Cmux Swift Logging ✅ Passed No Swift logging violations. The 4-line change adds pane-target detection without print/debugPrint/dump/NSLog. Debug logs use cmuxDebugLog within #if DEBUG guards.
Cmux Swiftui State Layout ✅ Passed File is AppKit NSView extension with no SwiftUI state management. The 4-line addition is a fallback pane-target check in pure AppKit hit-testing with no state mutations or violations.
Cmux Architecture Rethink ✅ Passed Property check fallback with no timing, dispatch, locks, or mutable state. Clear ownership, single code path. Small correctness fix meeting allowed criteria.
Title check ✅ Passed The title clearly and specifically describes the main change: fixing file drop behavior on terminals to paste paths instead of opening a viewer, directly addressing the issue.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes Finder file drops landing in the file-preview panel instead of pasting shell-escaped paths into the terminal. The root cause was that PaneDropTargetView (Bonsplit's pane-split overlay) captured the AppKit hit-test before GhosttyNSView, causing textDropDestinationKindUnderPoint to return nil and forcing canDropAsText = false — which routes all Finder drops to the preview path.

  • Adds a four-line fallback in textDropDestinationKindUnderPoint: after terminalUnderPoint misses, checks whether paneDropTargetUnderPoint returns a PaneDropTargetView whose hostedView is non-nil; if so, returns .terminal so the text-routing path is taken.
  • The fix is scoped entirely to FileDropOverlayViewHitTesting.swift and has no effect on the bonsplit tab-drag or file-preview-panel flows.

Confidence Score: 3/5

The change correctly fixes the routing decision for the common case, but the actual text-insertion step depends on Bonsplit's PaneDropTargetView.performDragOperation accepting file-URL payloads and there is no recovery path if it returns false.

The routing logic in textDropDestinationKindUnderPoint is patched correctly and the fix is small and targeted. However, the actual drop execution path when the new fallback fires unconditionally delegates to PaneDropTargetView.fileDropPerformDragOperation and returns its result with no fallthrough. If that Bonsplit method returns false for Finder file-URL payloads, the drop silently fails with no recovery.

Sources/FileDropOverlayViewHitTesting.swift — specifically the performDragOperation text-route branch in FileDropOverlayView.swift where a false return from PaneDropTargetView.fileDropPerformDragOperation terminates the drop with no recovery.

Important Files Changed

Filename Overview
Sources/FileDropOverlayViewHitTesting.swift Adds a PaneDropTargetView-with-hostedView fallback in textDropDestinationKindUnderPoint; correctly unblocks text routing when PaneDropTargetView intercepts hit-tests before GhosttyNSView, but the actual text insertion depends on Bonsplit's PaneDropTargetView.performDragOperation accepting file-URL payloads, with no explicit recovery path if it returns false.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Finder file drag dropped] --> B{shouldRouteFileDropToTextDestination?}
    B --> C{terminalUnderPoint != nil?}
    C -- Yes --> D[return .terminal]
    C -- No --> E{NEW: paneDropTargetUnderPoint as PaneDropTargetView with hostedView != nil?}
    E -- Yes --> D
    E -- No --> F[return nil - canDropAsText = false]
    D --> G[Text route taken]
    F --> H[Preview route - bug path]
    G --> I{paneDropTargetForTextDrop finds PaneDropTargetView?}
    I -- Yes --> J[PaneDropTargetView.fileDropPerformDragOperation]
    J -- true --> K[Text drop handled]
    J -- false --> L[Silent failure - no fallthrough]
    I -- No --> M[performFileDropAsText]
    M -- found --> N[insert urls into terminal]
    M -- nil --> O[Returns false]
Loading

Reviews (1): Last reviewed commit: "fix: file drop on terminal routes to tex..." | Re-trigger Greptile

Comment thread Sources/FileDropOverlayViewHitTesting.swift
Comment thread Sources/FileDropOverlayViewHitTesting.swift
@mrosnerr mrosnerr changed the title Fix file drop on terminal opening viewer instead of pasting path (#3729) Fix file drop on terminal, opening viewer instead of pasting path (#3729) May 9, 2026
@mrosnerr

Copy link
Copy Markdown
Author

Superseded by #3755 which rerouted all terminal file drops through the SSH-aware upload planner and updated the overlay/pane drop hit-testing — covers the same hit-test gap this PR addressed.

@mrosnerr mrosnerr closed this May 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Regression: file drag-and-drop opens built-in viewer instead of referencing path

1 participant