Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions Sources/ContentView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -633,6 +633,8 @@ final class FileDropOverlayView: NSView {
}
}

/// Commits the drop: routes to the active WKWebView, a terminal under the cursor,
/// or the folder-drop handler when no terminal is present (e.g. sidebar area).
override func performDragOperation(_ sender: any NSDraggingInfo) -> Bool {
let hasLocalDraggingSource = sender.draggingSource != nil
let types = sender.draggingPasteboard.types
Expand All @@ -657,10 +659,24 @@ final class FileDropOverlayView: NSView {
if let webView {
return webView.performDragOperation(sender)
}
guard let terminal else { return false }
guard let terminal else {
// No terminal under the drop point — delegate to the folder drop handler.
// In practice the only non-terminal, non-browser area visible to this overlay
// is the sidebar: this overlay is anchored to contentView.topAnchor, so the
// title-bar strip above it is never reachable, and all other areas are filled
// by terminal or browser panels. No title-bar / non-sidebar guard is needed.
let urls = sender.draggingPasteboard.readObjects(
forClasses: [NSURL.self],
options: [.urlReadingFileURLsOnly: true]
) as? [URL] ?? []
return onDrop?(urls) ?? false
Comment on lines +662 to +672

@coderabbitai coderabbitai Bot Mar 19, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Limit the folder-drop fallback to the actual sidebar.

These branches now treat any non-terminal, non-browser area as a sidebar drop target, so a plain folder dropped on the titlebar strip or other non-terminal gaps will still show .copy and create/select a workspace. That broadens the feature beyond the sidebar-only behavior described for this PR.

Also applies to: 710-724

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 662 - 669, The fallback that treats
any non-terminal area as a sidebar drop should be restricted to actual sidebar
drops: instead of immediately reading URLs and calling onDrop when guard let
terminal fails, first verify the drop location is within the sidebar view (e.g.,
use sender.draggingLocation, convert(_:from:), and
sidebarView.bounds.contains(...) or sidebarView.hitTest) and only then perform
the folder-on-sidebar fallback (read URLs and call onDrop). Apply the same
restriction to the similar branch around the 710-724 logic so only drops landing
inside the sidebar region trigger the folder fallback.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, but this guard isn't necessary given how the overlay is structured.

FileDropOverlayView is anchored to contentView.topAnchor, which means the title-bar strip above it is physically unreachable by drag-and-drop. Within the overlay's bounds, the only area that is neither a terminal panel nor a browser panel is the sidebar itself — every other pixel is covered by one of those two panel types.

Added a comment in performDragOperation explaining this so the intent is clear to future readers.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wonbywondev Got it—thanks for the clarification. I double-checked the overlay install and the new comment in performDragOperation; since the FileDropOverlayView is anchored to contentView (not the native titlebar) and terminal/browser panels occupy the rest of the content area, the fallback effectively maps to “sidebar only.” No further changes needed.


✏️ Learnings added
Learnt from: wonbywondev
Repo: manaflow-ai/cmux PR: 1805
File: Sources/ContentView.swift:662-672
Timestamp: 2026-03-19T10:48:31.943Z
Learning: Repo manaflow-ai/cmux — Sources/ContentView.swift: FileDropOverlayView is anchored to the window’s contentView (theme frame). Within its bounds, the only non-terminal/non-browser area is the Sidebar, so performDragOperation’s folder-drop fallback (calling onDrop when no terminal/web view is under the cursor) is effectively sidebar-only and does not require additional hit-testing for the titlebar or other regions.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T08:16:21.950Z
Learning: Applies to **/TerminalWindowPortal.swift : In `WindowTerminalHostView.hitTest()` in `TerminalWindowPortal.swift`: all divider/sidebar/drag routing must be gated to pointer events only. Do not add work outside the `isPointerEvent` guard as this path is called on every event including keyboard input.

Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T08:16:21.950Z
Learning: Applies to **/GhosttyTerminalView.swift : `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers. Portal-hosted terminal views can sit above SwiftUI during split/workspace churn.

Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-05T03:20:48.079Z
Learning: Applies to **/*TerminalView.swift : Terminal find layering contract: SurfaceSearchOverlay must be mounted from GhosttySurfaceScrollView in Sources/GhosttyTerminalView.swift (AppKit portal layer), not from SwiftUI panel containers

Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-05T00:05:51.436Z
Learning: Applies to **/GhosttyTerminalView.swift : `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers such as `Sources/Panels/TerminalPanelView.swift`

Learnt from: wonbywondev
Repo: manaflow-ai/cmux PR: 1795
File: Sources/ContentView.swift:1392-1395
Timestamp: 2026-03-19T09:21:22.509Z
Learning: Repo manaflow-ai/cmux — Notifications UI: Production notifications are shown via NSPopover (toggleNotificationsPopover in Sources/Update/UpdateTitlebarAccessory.swift). The SidebarSelectionState.selection == .notifications path in Sources/ContentView.swift is test/scaffolding-only (set in AppDelegate for tests) and isn’t used by user actions; .tabs remains active in normal flows. Therefore, folder-drop handling doesn’t need to flip selection.

Learnt from: SuperManfred
Repo: manaflow-ai/cmux PR: 803
File: Sources/Panels/BrowserPopupWindowController.swift:36-38
Timestamp: 2026-03-04T05:11:56.373Z
Learning: In Sources/Panels/BrowserPopupWindowController.swift and Sources/Panels/BrowserPanel.swift, `webView.isInspectable = true` (guarded by `#available(macOS 13.3, *)`) is intentionally enabled in all builds — not just DEBUG — because cmux is a developer tool and full Web Inspector access is desired in production builds as well. Do not flag this as a security concern.

Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.

Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 318
File: Sources/Panels/BrowserPanel.swift:0-0
Timestamp: 2026-03-17T05:34:44.905Z
Learning: In manaflow-ai/cmux (PR `#318`), the browser import wizard in Sources/Panels/BrowserPanel.swift now exposes an Additional data checkbox and passes its state into BrowserImportScope.fromSelection so users can select .everything (including when only additional-data is chosen). Unit and UI tests cover the additional‑data‑only mapping.

Learnt from: SuperManfred
Repo: manaflow-ai/cmux PR: 1150
File: Sources/AppDelegate.swift:7394-7401
Timestamp: 2026-03-10T10:24:14.017Z
Learning: Repo: manaflow-ai/cmux — For Cmd+W behavior with browser popups: BrowserPopupPanel.performKeyEquivalent is the primary handler to close popups. AppDelegate.handleCustomShortcut(_:), in its Cmd+W fallback, must check both NSApp.keyWindow and event.window for identifier "cmux.browser-popup" and close it if found, before routing to workspace/settings close logic.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:16.241Z
Learning: For manaflow-ai/cmux PR `#819` (Japanese i18n), keep scope limited to localization changes; UX enhancements like preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels should be handled in a separate follow-up issue.

Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.

Learnt from: outoftime
Repo: manaflow-ai/cmux PR: 1528
File: Sources/GhosttyTerminalView.swift:3220-3228
Timestamp: 2026-03-17T18:25:33.286Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift within TerminalSurface.createSurface(for:), when constructing XDG_DATA_DIRS for Fish vendor_conf.d auto-sourcing, treat empty or whitespace-only values from initialEnvironmentOverrides, env, getenv, and ProcessInfo as unset before prefixing the integrationDir. This avoids producing a trailing colon. Keep XDG_DATA_DIRS in protectedStartupEnvironmentKeys so initialEnvironmentOverrides cannot overwrite the prefixed value.

Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.

Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.

Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.

Learnt from: debgotwired
Repo: manaflow-ai/cmux PR: 1149
File: Sources/ContentView.swift:3977-3978
Timestamp: 2026-03-10T09:33:29.814Z
Learning: In manaflow-ai/cmux, for Sources/ContentView.swift, do not modify the sync-seeding behavior of scheduleCommandPaletteResultsRefresh(forceSearchCorpusRefresh:). It is shared infrastructure across all submenus. Treat brief initial flashes as consistent with existing submenus. Any UX improvement (for example making seeding synchronous on a forced corpus refresh) should not be implemented in feature-scoped PRs; instead implement and test such UX changes globally in a dedicated follow-up PR.

Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.

Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.

}
return terminal.performDragOperation(sender)
}

/// Updates the drag target and returns the proposed drag operation.
/// Returns `.copy` only when capture is enabled and the payload contains at least one
/// directory URL, so plain-file drops do not show a misleading copy cursor.
private func updateDragTarget(_ sender: any NSDraggingInfo, phase: String) -> NSDragOperation {
let loc = sender.draggingLocation
let hasLocalDraggingSource = sender.draggingSource != nil
Expand Down Expand Up @@ -694,7 +710,21 @@ final class FileDropOverlayView: NSView {
hasTerminalTarget: hasTerminalTarget
)
#endif
guard shouldCapture, hasTerminalTarget else { return [] }
guard shouldCapture else { return [] }
// Only advertise the copy affordance when the payload contains at least one
// plain directory URL. Filesystem packages (.app, .xcworkspace, etc.) have
// hasDirectoryPath==true but isPackage==true and must be excluded to keep
// the cursor consistent with performDragOperation/handleSidebarFolderDrop.
let urls = sender.draggingPasteboard.readObjects(
forClasses: [NSURL.self],
options: [.urlReadingFileURLsOnly: true]
) as? [URL] ?? []
let hasFolder = hasTerminalTarget || urls.contains { url in
guard url.hasDirectoryPath else { return false }
guard let values = try? url.resourceValues(forKeys: [.isPackageKey]) else { return false }
return values.isPackage != true
}
guard hasFolder else { return [] }
return .copy
}

Expand Down Expand Up @@ -1363,9 +1393,12 @@ func installFileDropOverlay(on window: NSWindow, tabManager: TabManager) {
let overlay = FileDropOverlayView(frame: contentView.frame)
overlay.translatesAutoresizingMaskIntoConstraints = false
overlay.onDrop = { [weak tabManager] urls in
MainActor.assumeIsolated {
guard let tabManager, let terminal = tabManager.selectedWorkspace?.focusedTerminalPanel else { return false }
return terminal.hostedView.handleDroppedURLs(urls)
guard let tabManager else { return false }
// Note: sidebarSelectionState.selection == .notifications is never set in
// production; all notification UI uses NSPopover via toggleNotificationsPopover.
// No sidebar page switch is needed after a successful drop.
return MainActor.assumeIsolated {
tabManager.handleSidebarFolderDrop(urls)
}
}

Expand Down
65 changes: 65 additions & 0 deletions Sources/TabManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1133,6 +1133,71 @@ class TabManager: ObservableObject {
return newWorkspace
}

/// Handles a folder drag-and-drop onto the sidebar or non-terminal window area.
///
/// - If an existing workspace has the same working directory, selects it and adds a new
/// terminal split. If the workspace contains only non-terminal panels (e.g. browser-only),
/// the terminal split is anchored beside the existing panel rather than creating a new workspace.
/// - Otherwise, creates a new workspace rooted at the dropped folder.
///
/// Only the first plain-directory URL is acted upon. Non-directory URLs and filesystem
/// packages (.app, .xcworkspace, .playground, etc.) are silently ignored.
@discardableResult
func handleSidebarFolderDrop(_ urls: [URL]) -> Bool {
let folderURLs = urls.filter { url in
// Accept only plain directories; exclude filesystem packages (.app, .xcworkspace,
// .playground, etc.) which report isDirectory=true but should be ignored like files.
guard let values = try? url.resourceValues(forKeys: [.isDirectoryKey, .isPackageKey])
else { return false }
return values.isDirectory == true && values.isPackage != true
}
let directories = FinderServicePathResolver.orderedUniqueDirectories(
from: folderURLs.filter { $0.isFileURL }
)
guard let path = directories.first else { return false }

#if DEBUG
dlog("sidebar.folderDrop path=\(path) tabCount=\(tabs.count)")
#endif

// Note: resolvingSymlinksInPath is intentionally not applied here.
// orderedUniqueDirectories already uses standardizedFileURL which resolves /tmp → /private/tmp
// on macOS. Adding resolvingSymlinksInPath to both sides would be equivalent but adds
// unnecessary cost; /tmp symlink matching already passes without it.
let existing = tabs.first { workspace in
var cd = workspace.currentDirectory.trimmingCharacters(in: .whitespacesAndNewlines)
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
while cd.count > 1 && cd.hasSuffix("/") { cd.removeLast() }
return cd == path
}

if let existing {
selectedTabId = existing.id
// Prefer a focused or existing TerminalPanel as the split source.
// If the workspace has only non-terminal panels (e.g. browser-only), use any panel
// so a new terminal appears as a split beside it rather than creating a new workspace.
let panelId = existing.focusedPanelId
?? existing.panels.values.compactMap { $0 as? TerminalPanel }.first?.id
?? existing.panels.values.first?.id
if let panelId {
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
let newPanel = existing.newTerminalSplit(from: panelId, orientation: .horizontal)
#if DEBUG
if newPanel == nil {
dlog("sidebar.folderDrop.splitFailed panelId=\(panelId.uuidString.prefix(5))")
}
#endif
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
#if DEBUG
dlog("sidebar.folderDrop.existing workspaceId=\(existing.id.uuidString.prefix(5))")
#endif
} else {
addWorkspace(workingDirectory: path, select: true)
#if DEBUG
dlog("sidebar.folderDrop.new path=\(path)")
#endif
}
return true
}
Comment on lines +1181 to +1199

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not report success when no terminal was created.

At Line 1181 and Line 1198, the method can return true even when panelId is nil or newTerminalSplit fails. That marks the drop as handled without opening a shell.

Suggested hardening
         if let existing {
             selectedTabId = existing.id
             // Prefer a focused or existing TerminalPanel as the split source.
             // If the workspace has only non-terminal panels (e.g. browser-only), use any panel
             // so a new terminal appears as a split beside it rather than creating a new workspace.
             let panelId = existing.focusedPanelId
                 ?? existing.panels.values.compactMap { $0 as? TerminalPanel }.first?.id
                 ?? existing.panels.values.first?.id
-            if let panelId {
-                let newPanel = existing.newTerminalSplit(from: panelId, orientation: .horizontal)
+            if let panelId,
+               existing.newTerminalSplit(from: panelId, orientation: .horizontal) != nil {
 `#if` DEBUG
-                if newPanel == nil {
-                    dlog("sidebar.folderDrop.splitFailed panelId=\(panelId.uuidString.prefix(5))")
-                }
+                dlog("sidebar.folderDrop.existing workspaceId=\(existing.id.uuidString.prefix(5))")
 `#endif`
+            } else {
+                // Ensure drop still opens a terminal if split source/creation fails.
+                _ = addWorkspace(workingDirectory: path, select: true)
+#if DEBUG
+                dlog("sidebar.folderDrop.fallback.new path=\(path)")
+#endif
             }
-#if DEBUG
-            dlog("sidebar.folderDrop.existing workspaceId=\(existing.id.uuidString.prefix(5))")
-#endif
         } else {
             addWorkspace(workingDirectory: path, select: true)
 `#if` DEBUG
             dlog("sidebar.folderDrop.new path=\(path)")
 `#endif`
         }
         return true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TabManager.swift` around lines 1181 - 1199, The drop handler
currently returns true even when no terminal was created (when panelId is nil or
newTerminalSplit(from:orientation:) returns nil); update the logic in the
folder-drop handling block so you only return true if a terminal/view was
actually created — i.e., check panelId and the result of
existing.newTerminalSplit(from:panelId, orientation:) and only treat the drop as
handled when that result is non-nil; if no split was created, fall through (or
return false) so addWorkspace(workingDirectory:select:) is used or the drop is
reported unhandled as appropriate.


@MainActor
private func sendWelcomeWhenReady(to workspace: Workspace) {
if let terminalPanel = workspace.focusedTerminalPanel,
Expand Down
21 changes: 18 additions & 3 deletions Sources/Workspace.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6610,11 +6610,26 @@ final class Workspace: Identifiable, ObservableObject {
// Suppress the old view's becomeFirstResponder side-effects during SwiftUI reparenting.
// Without this, reparenting triggers onFocus + ghostty_surface_set_focus on the old view,
// stealing focus from the new panel and creating model/surface divergence.
let browserSourcePanel = panels[panelId] as? BrowserPanel
if focus {
previousHostedView?.suppressReparentFocus()
focusPanel(newPanel.id, previousHostedView: previousHostedView)
DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) {
previousHostedView?.clearSuppressReparentFocus()
if let browserSourcePanel {
// When the source is a BrowserPanel, the WKWebView portal still covers the full
// workspace rect until SwiftUI layout shrinks it. Focusing the terminal directly
// causes the first click to be swallowed by the stale portal geometry.
// Focusing the browser first lets the portal settle, then we hand off to the
// terminal on the next runloop — mirroring the user clicking browser → terminal.
focusPanel(browserSourcePanel.id, previousHostedView: previousHostedView)
DispatchQueue.main.async { [weak self] in
guard let self else { return }
self.focusPanel(newPanel.id, previousHostedView: nil)
previousHostedView?.clearSuppressReparentFocus()
}
} else {
focusPanel(newPanel.id, previousHostedView: previousHostedView)
DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) {
previousHostedView?.clearSuppressReparentFocus()
}
}
} else {
preserveFocusAfterNonFocusSplit(
Expand Down