Skip to content

Add file tree sidebar with cmd-click path composition - #738

Open
AlexBoudreaux wants to merge 2 commits into
manaflow-ai:mainfrom
AlexBoudreaux:alex/file-tree-cmd-click
Open

AlexBoudreaux wants to merge 2 commits into
manaflow-ai:mainfrom
AlexBoudreaux:alex/file-tree-cmd-click

Conversation

@AlexBoudreaux

@AlexBoudreaux AlexBoudreaux commented Mar 2, 2026 •

Copy link
Copy Markdown

Summary

Adds a file tree sidebar that shows the workspace's current directory, with a novel interaction: cmd-click any file or folder to inject its shell-escaped path directly into the active terminal pane.

Ported the file tree concept from PR #601 by @Shehryar (stripped the code editor panel to keep scope focused on path composition). Built on top of current main with no conflicts.

Features

  • Cmd+E or titlebar folder button to toggle between tabs and file tree
  • Cmd-click any file or folder to insert its shell-escaped path into the focused terminal
  • Click folders to expand/collapse, files to select/highlight
  • Right-click context menu with Copy Path, Reveal in Finder, Open in Default App, Insert Path to Terminal
  • FSEvents file watching for automatic tree refresh when files change on disk
  • Lazy directory loading with preserved expand state on refresh
  • Hidden files shown by default
  • Sidebar mode persisted via @AppStorage / UserDefaults so it survives restarts and works reactively across all view hierarchies (SwiftUI ContentView + AppKit titlebar accessory)

How it works

When you cmd-click a file in the tree, the path is shell-escaped (single-quote wrapped with internal quotes escaped) and sent to the focused terminal pane via ghostty_surface_text. This means the path appears as typed text without triggering bracketed paste mode, so it works naturally with any shell or CLI tool waiting for input.

Files changed

File What
Sources/FileTree/FileTreeNode.swift Data model with SF Symbol icon mapping
Sources/FileTree/FileTreeModel.swift Filesystem scanning, lazy expand, FSEvents watching, refresh with state preservation
Sources/FileTree/FileTreeRow.swift Per-row view with cmd-click detection, selection highlight, flash feedback, context menu
Sources/FileTree/FileTreeSidebar.swift ScrollView wrapper, directory header, workspace directory observation
Sources/FileTree/SidebarContentMode.swift Enum for sidebar mode (tabs vs fileTree)
Sources/ContentView.swift Sidebar mode toggle, file tree integration, path injection wiring
Sources/AppDelegate.swift Cmd+E keyboard shortcut handler via UserDefaults
Sources/cmuxApp.swift View > Toggle File Tree menu item
Sources/KeyboardShortcutSettings.swift .toggleFileTree shortcut definition
Sources/Update/UpdateTitlebarAccessory.swift Folder toggle button in titlebar controls
GhosttyTabs.xcodeproj/project.pbxproj File references for new FileTree sources

Test plan

  • Toggle file tree with Cmd+E
  • Click folders to expand/collapse
  • Click files to select/highlight
  • Cmd-click files/folders to inject path into terminal
  • Right-click context menu works (Copy Path, Reveal in Finder, Open in Default App)
  • Titlebar folder button toggles mode
  • File tree auto-refreshes when files are created/deleted in terminal
  • Paths with spaces and special characters are shell-escaped correctly
  • No regression in existing sidebar tab functionality

Credits

File tree concept ported from PR #601 by @Shehryar. Thank you for the foundation!

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • File tree sidebar as an alternative to tab-based sidebar, toggleable via new shortcut (Cmd+Shift+E) or titlebar button.
    • Expandable/collapsible directory view with automatic icons and hidden-file toggle.
    • Right-click menu: copy path, reveal in Finder, open with default app, insert path into terminal (paths safely escaped).
    • Sidebar auto-shows when switching to file-tree mode.

Port file tree sidebar concept from PR manaflow-ai#601 (credit @Shehryar) and add
cmd-click to inject file paths into the active terminal pane.

- File tree sidebar toggled with Cmd+E or titlebar folder button
- Cmd-click any file/folder to insert shell-escaped path into terminal
- Regular click to select files, expand/collapse folders
- Right-click context menu (Copy Path, Reveal in Finder, Open in Default App)
- FSEvents file watching for automatic tree refresh
- Hidden files shown by default
- Lazy directory loading with preserved expand state on refresh

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 2, 2026

Copy link
Copy Markdown

@AlexBoudreaux 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 Mar 2, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Adds a new file-tree sidebar feature: model, node types, SwiftUI rows and sidebar view; integrates a toggle (titlebar, menu, shortcut), AppStorage state, path-send logic to terminals, and registers new Swift files in the Xcode project.

Changes

Cohort / File(s) Summary
File Tree Core
Sources/FileTree/SidebarContentMode.swift, Sources/FileTree/FileTreeNode.swift, Sources/FileTree/FileTreeModel.swift, Sources/FileTree/FileTreeRow.swift, Sources/FileTree/FileTreeSidebar.swift
New enum, node model, ObservableObject file-tree model with FSEvents watching and refresh/preserve behavior, row UI component, and sidebar view that renders rootNodes and responds to workspace directory changes.
Content & UI Integration
Sources/ContentView.swift, Sources/Update/UpdateTitlebarAccessory.swift
Added AppStorage sidebarContentMode, FileTreeModel state, file-tree sidebar rendering, shell-escaping and send-to-terminal logic, titlebar control and callback wiring for toggling file-tree mode, and UI adjustments (ClearScrollBackground visibility, sidebar background).
Shortcuts & Menu
Sources/KeyboardShortcutSettings.swift, Sources/cmuxApp.swift, Sources/AppDelegate.swift
Introduced toggleFileTree action, default shortcut and defaults key, menu command and AppStorage shortcut binding, AppDelegate shortcut handling to flip mode and reveal sidebar, and menu/shortcut wiring.
Project File
GhosttyTabs.xcodeproj/project.pbxproj
Added references and build-phase entries for the five new Swift source files under Sources/FileTree/....

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Titlebar
    participant Menu
    participant ContentView
    participant FileTreeModel
    participant FSEvents
    participant Terminal

    User->>Titlebar: press toggle button
    Titlebar->>ContentView: onToggleFileTree()
    Menu->>ContentView: (alt) Toggle File Tree command
    ContentView->>ContentView: flip sidebarContentMode (tabs ↔ fileTree)
    ContentView->>ContentView: ensure sidebar visible when enabling fileTree

    alt Enabled fileTree
        ContentView->>FileTreeModel: loadDirectory(workspace.currentDirectory)
        FileTreeModel->>FSEvents: start watching
        FSEvents-->>FileTreeModel: notify changes
        FileTreeModel-->>ContentView: publish rootNodes update
        ContentView->>Terminal: send escaped path (on compose)
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 I nibble paths and hop through trees,
A folder breeze, a toggle please—
I send a path to terminals bright,
From leafy rows to titlebar light,
Hooray, the file tree springs tonight! 🌿✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a file tree sidebar with cmd-click path composition functionality, which aligns with the PR's primary objective.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@AlexBoudreaux

Copy link
Copy Markdown
Author
Screenshot 2026-03-01 at 10 24 59 PM

@AlexBoudreaux

Copy link
Copy Markdown
Author
Screenshot 2026-03-01 at 10 25 34 PM

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (3)
Sources/AppDelegate.swift (1)

5210-5221: Refactor the file-tree toggle to avoid raw UserDefaults literals and duplicate writes.

At Line 5211-Line 5216, using a typed mode transition (single write path) will reduce drift risk with other call sites and make behavior easier to maintain.

♻️ Suggested cleanup
-        if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) {
-            let current = UserDefaults.standard.string(forKey: "sidebarContentMode") ?? SidebarContentMode.tabs.rawValue
-            if current == SidebarContentMode.fileTree.rawValue {
-                UserDefaults.standard.set(SidebarContentMode.tabs.rawValue, forKey: "sidebarContentMode")
-            } else {
-                UserDefaults.standard.set(SidebarContentMode.fileTree.rawValue, forKey: "sidebarContentMode")
-                if sidebarState?.isVisible == false {
-                    sidebarState?.toggle()
-                }
-            }
-            return true
-        }
+        if matchShortcut(event: event, shortcut: KeyboardShortcutSettings.shortcut(for: .toggleFileTree)) {
+            let sidebarContentModeKey = "sidebarContentMode"
+            let currentMode = SidebarContentMode(
+                rawValue: UserDefaults.standard.string(forKey: sidebarContentModeKey) ?? ""
+            ) ?? .tabs
+            let nextMode: SidebarContentMode = (currentMode == .fileTree) ? .tabs : .fileTree
+            UserDefaults.standard.set(nextMode.rawValue, forKey: sidebarContentModeKey)
+
+            if nextMode == .fileTree, sidebarState?.isVisible == false {
+                sidebarState?.toggle()
+            }
+            return true
+        }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/AppDelegate.swift` around lines 5210 - 5221, The toggle logic writes
the same UserDefaults key in two branches and uses a raw string; replace this
with a typed enum transition: read current via SidebarContentMode(rawValue:
...), compute a single newMode (either .tabs or .fileTree) and call
UserDefaults.standard.set(newMode.rawValue, forKey: "sidebarContentMode")
exactly once, then if newMode == .fileTree and sidebarState?.isVisible == false
call sidebarState?.toggle(); update the code paths around matchShortcut(...) /
KeyboardShortcutSettings.shortcut(for:) to use SidebarContentMode explicitly
rather than raw literals.
Sources/cmuxApp.swift (1)

499-510: Extract file-tree mode toggling into a shared helper.

Lines 500-507 duplicate the same UserDefaults toggle branch that now also exists in Sources/Update/UpdateTitlebarAccessory.swift (Lines 736-744). This risks behavior drift over time (key name and sidebar-visibility rule).

♻️ Suggested consolidation
-                splitCommandButton(title: "Toggle File Tree", shortcut: toggleFileTreeMenuShortcut) {
-                    let current = UserDefaults.standard.string(forKey: "sidebarContentMode") ?? SidebarContentMode.tabs.rawValue
-                    if current == SidebarContentMode.fileTree.rawValue {
-                        UserDefaults.standard.set(SidebarContentMode.tabs.rawValue, forKey: "sidebarContentMode")
-                    } else {
-                        UserDefaults.standard.set(SidebarContentMode.fileTree.rawValue, forKey: "sidebarContentMode")
-                        if !sidebarState.isVisible {
-                            sidebarState.toggle()
-                        }
-                    }
-                }
+                splitCommandButton(title: "Toggle File Tree", shortcut: toggleFileTreeMenuShortcut) {
+                    SidebarContentModeToggler.toggle(defaults: .standard, sidebarState: sidebarState)
+                }
enum SidebarContentModeToggler {
    static let defaultsKey = "sidebarContentMode"

    static func toggle(defaults: UserDefaults = .standard, sidebarState: SidebarState) {
        let current = defaults.string(forKey: defaultsKey) ?? SidebarContentMode.tabs.rawValue
        let next: SidebarContentMode = (current == SidebarContentMode.fileTree.rawValue) ? .tabs : .fileTree
        defaults.set(next.rawValue, forKey: defaultsKey)

        if next == .fileTree, !sidebarState.isVisible {
            sidebarState.toggle()
        }
    }
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/cmuxApp.swift` around lines 499 - 510, The sidebar file-tree toggle
logic is duplicated; extract it into a single helper (e.g.,
SidebarContentModeToggler) that exposes a static defaultsKey and a static
toggle(defaults: UserDefaults = .standard, sidebarState: SidebarState) method
which reads the current SidebarContentMode, computes the next (.tabs or
.fileTree), writes next.rawValue to UserDefaults, and only calls
sidebarState.toggle() when next == .fileTree and sidebarState.isVisible is
false; then replace the inline branch in splitCommandButton (and the duplicate
in UpdateTitlebarAccessory) with a call to SidebarContentModeToggler.toggle(...)
to centralize behavior and the key name.
Sources/ContentView.swift (1)

1922-1932: Prefer sidebarContentMode as the single source of truth in toggle handler.

This closure duplicates key strings and writes UserDefaults directly even though @AppStorage is already bound. Centralizing the toggle via sidebarContentMode reduces drift risk.

♻️ Refactor sketch
+    private func toggleSidebarContentMode() {
+        if sidebarContentMode == SidebarContentMode.fileTree.rawValue {
+            sidebarContentMode = SidebarContentMode.tabs.rawValue
+            return
+        }
+        sidebarContentMode = SidebarContentMode.fileTree.rawValue
+        if !sidebarState.isVisible {
+            sidebarState.toggle()
+        }
+    }
+
     private var fullscreenControls: some View {
         TitlebarControlsView(
@@
-            onToggleFileTree: {
-                let current = UserDefaults.standard.string(forKey: "sidebarContentMode") ?? SidebarContentMode.tabs.rawValue
-                if current == SidebarContentMode.fileTree.rawValue {
-                    UserDefaults.standard.set(SidebarContentMode.tabs.rawValue, forKey: "sidebarContentMode")
-                } else {
-                    UserDefaults.standard.set(SidebarContentMode.fileTree.rawValue, forKey: "sidebarContentMode")
-                    if !sidebarState.isVisible {
-                        sidebarState.toggle()
-                    }
-                }
-            },
+            onToggleFileTree: {
+                toggleSidebarContentMode()
+            },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 1922 - 1932, The onToggleFileTree
closure should use the `@AppStorage-backed` sidebarContentMode as the single
source of truth instead of reading/writing UserDefaults directly; update the
closure to toggle sidebarContentMode between SidebarContentMode.tabs and
SidebarContentMode.fileTree, and only call sidebarState.toggle() when you set
sidebarContentMode to .fileTree and sidebarState.isVisible is false (so
visibility is synchronized), eliminating direct UserDefaults.standard access and
duplicated rawValue strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/ContentView.swift`:
- Around line 1826-1835: The shellEscapePath function currently only quotes
paths when certain characters are present, leaving glob metacharacters unquoted;
update shellEscapePath to always single-quote the input path (remove the
needsQuoting guard), escape any internal single quotes by replacing "'" with
"'\\''", and return the fully single-quoted string; reference the existing
shellEscapeCharacters constant in GhosttyTerminalView.swift for the full
character set if needed and ensure the function name shellEscapePath is the one
you change.

In `@Sources/FileTree/FileTreeModel.swift`:
- Around line 21-27: loadDirectory and refresh initiate uncancelled async scans
via Task that can finish out of order and overwrite newer results; fix by
version-gating scan results: add a monotonically-incremented scanVersion
property on the FileTreeModel and increment it before starting each Task (in
loadDirectory and refresh), capture the current version in a local variable
inside the Task, await scanDirectory(path), then only assign
self.rootNodes/self.rootPath if the captured version still equals the model's
current scanVersion; alternatively keep and cancel the previous Task by storing
it (e.g., lastScanTask) and calling cancel() before starting a new one, but the
primary fix is to check the scanVersion after await to prevent stale scan
results from replacing newer state.
- Around line 189-205: The restoreExpandedState implementation sets
nodes[i].children = children when lazily loading a directory but never
re-applies expandedIds to that newly loaded subtree, so nested expanded folders
collapse; in the Task after obtaining children from scanDirectory, update the
closure in findAndUpdate (or immediately after assigning n.children) to call
restoreExpandedState(in: &n.children, expandedIds: expandedIds) (or equivalent)
so the freshly loaded children have their isExpanded and descendant states
reapplied; reference restoreExpandedState(in:inout:expandedIds:),
scanDirectory(), findAndUpdate(in:id:update:), FileTreeNode, and rootNodes and
ensure this reapplication runs on the MainActor like the surrounding Task.

In `@Sources/FileTree/FileTreeSidebar.swift`:
- Around line 68-74: The onChange handler for workspace.currentDirectory needs
to handle transitions to an empty path to avoid leaving stale nodes/watches:
when the trimmed value is empty, call a new model.clearDirectory() (implement
clearDirectory to cancel refreshCoalesceTask, call stopWatching(), set rootPath
= "" and rootNodes = []), and still set selectedFilePath = nil; otherwise keep
the existing branch that calls model.loadDirectory(trimmed) and clears
selectedFilePath. Ensure the new clearDirectory API is added to the model type
so onChange can invoke it.

In `@Sources/KeyboardShortcutSettings.swift`:
- Around line 174-175: The default shortcut for toggleFileTree in
KeyboardShortcutSettings.swift currently returns StoredShortcut(key: "e", ...)
which conflicts with the existing ⌘E binding in cmuxApp.swift ("Use Selection
for Find"); change the default StoredShortcut returned by the case
.toggleFileTree to a non-conflicting key (for example "b" or "t") so the
toggleFileTree default won’t collide, and ensure the change is made inside the
KeyboardShortcutSettings.swift switch for the toggleFileTree case (updating
StoredShortcut initialization accordingly).

---

Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 5210-5221: The toggle logic writes the same UserDefaults key in
two branches and uses a raw string; replace this with a typed enum transition:
read current via SidebarContentMode(rawValue: ...), compute a single newMode
(either .tabs or .fileTree) and call UserDefaults.standard.set(newMode.rawValue,
forKey: "sidebarContentMode") exactly once, then if newMode == .fileTree and
sidebarState?.isVisible == false call sidebarState?.toggle(); update the code
paths around matchShortcut(...) / KeyboardShortcutSettings.shortcut(for:) to use
SidebarContentMode explicitly rather than raw literals.

In `@Sources/cmuxApp.swift`:
- Around line 499-510: The sidebar file-tree toggle logic is duplicated; extract
it into a single helper (e.g., SidebarContentModeToggler) that exposes a static
defaultsKey and a static toggle(defaults: UserDefaults = .standard,
sidebarState: SidebarState) method which reads the current SidebarContentMode,
computes the next (.tabs or .fileTree), writes next.rawValue to UserDefaults,
and only calls sidebarState.toggle() when next == .fileTree and
sidebarState.isVisible is false; then replace the inline branch in
splitCommandButton (and the duplicate in UpdateTitlebarAccessory) with a call to
SidebarContentModeToggler.toggle(...) to centralize behavior and the key name.

In `@Sources/ContentView.swift`:
- Around line 1922-1932: The onToggleFileTree closure should use the
`@AppStorage-backed` sidebarContentMode as the single source of truth instead of
reading/writing UserDefaults directly; update the closure to toggle
sidebarContentMode between SidebarContentMode.tabs and
SidebarContentMode.fileTree, and only call sidebarState.toggle() when you set
sidebarContentMode to .fileTree and sidebarState.isVisible is false (so
visibility is synchronized), eliminating direct UserDefaults.standard access and
duplicated rawValue strings.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aaf6a24 and 2124532.

📒 Files selected for processing (11)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/FileTree/FileTreeModel.swift
  • Sources/FileTree/FileTreeNode.swift
  • Sources/FileTree/FileTreeRow.swift
  • Sources/FileTree/FileTreeSidebar.swift
  • Sources/FileTree/SidebarContentMode.swift
  • Sources/KeyboardShortcutSettings.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Sources/cmuxApp.swift

Comment thread Sources/ContentView.swift
Comment thread Sources/FileTree/FileTreeModel.swift
Comment thread Sources/FileTree/FileTreeModel.swift
Comment thread Sources/FileTree/FileTreeSidebar.swift
Comment thread Sources/KeyboardShortcutSettings.swift Outdated
@greptile-apps

greptile-apps Bot commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a file tree sidebar that displays the workspace directory with a novel cmd-click interaction to inject shell-escaped paths into the terminal. The implementation shows solid architectural patterns including FSEvents file watching, lazy directory loading with preserved expand state, and proper memory management with [weak self] captures.

Key changes:

  • New FileTree module with SwiftUI views (FileTreeNode, FileTreeModel, FileTreeRow, FileTreeSidebar)
  • Integration points in ContentView, AppDelegate, cmuxApp, and titlebar controls
  • Cmd+E keyboard shortcut and titlebar folder button to toggle between tabs and file tree
  • Right-click context menu with Copy Path, Reveal in Finder, Open in Default App, Insert Path

Issues found:

  • Critical: Shell escaping in ContentView.shellEscapePath() is incomplete. The needsQuoting check excludes glob characters (*, ?, []), command substitution (`), brace expansion ({}), and other shell metacharacters. A file named test*.txt or $(dangerous) would be sent unquoted and trigger shell expansion/execution.
  • Consistency: The escaping strategy differs from the existing GhosttyTerminalView.escapeDropForShell() which uses backslash escaping for \\ ()[]{}<>"'\!#$&;|*?\t`. Both file drop and file tree should use the same approach.

Recommendations:

  • Fix shell escaping to either always wrap in single quotes OR use the same backslash escaping as existing file drop code
  • Consider extracting the escaping logic to a shared utility to maintain consistency

Confidence Score: 3/5

  • Merge-blocking shell escaping bug that could cause command injection or unintended shell expansion
  • The file tree implementation is architecturally sound with good patterns (FSEvents, lazy loading, proper memory management), but the shell escaping logic has a critical flaw. Files with glob characters or command substitution syntax would be sent unquoted to the terminal, causing shell expansion or potential command execution. This needs to be fixed before merging.
  • Sources/ContentView.swift - fix shellEscapePath() to handle all shell metacharacters

Important Files Changed

Filename Overview
Sources/FileTree/FileTreeModel.swift FSEvents watching, lazy loading, proper memory management; could add error handling for permission issues
Sources/FileTree/FileTreeRow.swift UI component with cmd-click detection, context menu, selection/hover states
Sources/FileTree/FileTreeSidebar.swift ScrollView wrapper that observes workspace directory changes and refreshes tree
Sources/ContentView.swift Integrates file tree with incomplete shell escaping (missing glob chars, command substitution); inconsistent with existing escaping strategy

Sequence Diagram

sequenceDiagram
    participant User
    participant FileTreeRow
    participant FileTreeSidebar
    participant ContentView
    participant TerminalPanel
    participant Ghostty

    User->>FileTreeRow: Cmd+Click file/folder
    FileTreeRow->>FileTreeRow: Detect NSApp.currentEvent modifierFlags
    FileTreeRow->>FileTreeRow: Trigger flash animation
    FileTreeRow->>FileTreeSidebar: onComposePath(node.path)
    FileTreeSidebar->>ContentView: sendPathToFocusedTerminal(path)
    ContentView->>ContentView: shellEscapePath(path)
    Note over ContentView: ⚠️ Incomplete escaping<br/>Missing: *, ?, [], {}, ~, #, \, etc.
    ContentView->>TerminalPanel: sendText(escapedPath)
    TerminalPanel->>Ghostty: ghostty_surface_text()
    Note over Ghostty: Path injected as typed text<br/>(not bracketed paste)
    Ghostty-->>User: Path appears in terminal

    User->>FileTreeRow: Right-click file/folder
    FileTreeRow->>FileTreeRow: Show context menu
    User->>FileTreeRow: Select "Insert Path to Terminal"
    FileTreeRow->>FileTreeSidebar: onComposePath(node.path)
    Note over FileTreeSidebar: Same flow as Cmd+Click
Loading

Last reviewed commit: 2124532

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

11 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment thread Sources/ContentView.swift
Comment on lines +1803 to +1811
}
}

private var sidebarView: some View {
VStack(spacing: 0) {
if sidebarContentMode == SidebarContentMode.fileTree.rawValue {
fileTreeSidebarContent
} else {
tabsSidebarContent

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Incomplete shell escaping - missing glob chars (*, ?, [, ]), brace expansion ({, }), command substitution (`), tilde (~), hash (#), backslash (\), and control characters. A file named test*.txt or $(dangerous) would be sent unquoted and cause shell expansion/execution.

Safer approach: always wrap in single quotes:

Suggested change
}
}
private var sidebarView: some View {
VStack(spacing: 0) {
if sidebarContentMode == SidebarContentMode.fileTree.rawValue {
fileTreeSidebarContent
} else {
tabsSidebarContent
private func shellEscapePath(_ path: String) -> String {
// Always wrap in single quotes and escape internal single quotes
let escaped = path.replacingOccurrences(of: "'", with: "'\\''")
return "'\(escaped)'"
}

Comment thread Sources/ContentView.swift
Comment on lines +1826 to 1835
private func shellEscapePath(_ path: String) -> String {
let needsQuoting = path.contains(" ") || path.contains("'") || path.contains("\"")
|| path.contains("(") || path.contains(")") || path.contains("&")
|| path.contains("|") || path.contains(";") || path.contains("$")
|| path.contains("`") || path.contains("!")
guard needsQuoting else { return path }
// Use single quotes with escaped inner single quotes
let escaped = path.replacingOccurrences(of: "'", with: "'\\''")
return "'\(escaped)'"
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Inconsistent with existing shell escaping in GhosttyTerminalView.escapeDropForShell which uses backslash escaping for \\ ()[]{}<>\"'\!#$&;|*?\t`. File drop and file tree should use the same escaping strategy.

Extract escapeDropForShell to a shared utility or use the same character set and approach here.

- Always single-quote paths in shellEscapePath to prevent glob expansion
- Add scanVersion to prevent stale async scans from overwriting newer results
- Recursively restore expanded state on lazily loaded children
- Add clearDirectory() to handle empty directory transitions
- Change shortcut to Cmd+Shift+E to avoid conflict with Use Selection for Find

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Sources/ContentView.swift (1)

1916-1926: Use @AppStorage state directly instead of raw UserDefaults toggling.

This avoids dual state paths and keeps sidebar mode changes in one source of truth.

♻️ Suggested refactor
             onToggleFileTree: {
-                let current = UserDefaults.standard.string(forKey: "sidebarContentMode") ?? SidebarContentMode.tabs.rawValue
-                if current == SidebarContentMode.fileTree.rawValue {
-                    UserDefaults.standard.set(SidebarContentMode.tabs.rawValue, forKey: "sidebarContentMode")
+                if sidebarContentMode == SidebarContentMode.fileTree.rawValue {
+                    sidebarContentMode = SidebarContentMode.tabs.rawValue
                 } else {
-                    UserDefaults.standard.set(SidebarContentMode.fileTree.rawValue, forKey: "sidebarContentMode")
+                    sidebarContentMode = SidebarContentMode.fileTree.rawValue
                     if !sidebarState.isVisible {
                         sidebarState.toggle()
                     }
                 }
             },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 1916 - 1926, The onToggleFileTree
closure currently reads and writes UserDefaults directly (using keys like
"sidebarContentMode") causing dual state paths; replace those raw UserDefaults
accesses with the `@AppStorage-backed` property that represents the sidebar
content mode (e.g., sidebarContentMode: SidebarContentMode.RawValue) so the view
uses a single source of truth; update the onToggleFileTree logic to toggle the
`@AppStorage-backed` sidebarContentMode between
SidebarContentMode.fileTree.rawValue and SidebarContentMode.tabs.rawValue and
keep the existing behavior of calling sidebarState.toggle() when switching to
fileTree and the sidebar is not visible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Sources/ContentView.swift`:
- Around line 1818-1824: The sendPathToFocusedTerminal function currently sends
the escaped path without a trailing space, causing concatenated tokens when
cmd-clicking multiple paths; update the code in sendPathToFocusedTerminal so
that after calling shellEscapePath(path) you append a single space to the string
passed to terminalPanel.sendText (i.e., send escaped + " ") so each inserted
path is separated; reference sendPathToFocusedTerminal, shellEscapePath, and
terminalPanel.sendText when making the change.

---

Nitpick comments:
In `@Sources/ContentView.swift`:
- Around line 1916-1926: The onToggleFileTree closure currently reads and writes
UserDefaults directly (using keys like "sidebarContentMode") causing dual state
paths; replace those raw UserDefaults accesses with the `@AppStorage-backed`
property that represents the sidebar content mode (e.g., sidebarContentMode:
SidebarContentMode.RawValue) so the view uses a single source of truth; update
the onToggleFileTree logic to toggle the `@AppStorage-backed` sidebarContentMode
between SidebarContentMode.fileTree.rawValue and
SidebarContentMode.tabs.rawValue and keep the existing behavior of calling
sidebarState.toggle() when switching to fileTree and the sidebar is not visible.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2124532 and 35cdeff.

📒 Files selected for processing (4)
  • Sources/ContentView.swift
  • Sources/FileTree/FileTreeModel.swift
  • Sources/FileTree/FileTreeSidebar.swift
  • Sources/KeyboardShortcutSettings.swift
🚧 Files skipped from review as they are similar to previous changes (1)
  • Sources/FileTree/FileTreeModel.swift

Comment thread Sources/ContentView.swift
Comment on lines +1818 to +1824
private func sendPathToFocusedTerminal(_ path: String) {
guard let workspace = tabManager.selectedTab,
let terminalPanel = workspace.focusedTerminalPanel else { return }
// Shell-escape the path so spaces and special chars don't break commands
let escaped = shellEscapePath(path)
terminalPanel.sendText(escaped)
}

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify delimiter logic in onComposePath call sites and sendPathToFocusedTerminal usage
rg -n -C5 'onComposePath|sendPathToFocusedTerminal|shellEscapePath' Sources --type swift | head -200

Repository: manaflow-ai/cmux

Length of output: 7141


🏁 Script executed:

#!/bin/bash
# Find terminalPanel.sendText implementation and other call sites
rg -n 'func sendText|\.sendText\(' Sources --type swift -A2 | head -100

Repository: manaflow-ai/cmux

Length of output: 3789


🏁 Script executed:

#!/bin/bash
# Read the complete sendText implementation in GhosttyTerminalView
sed -n '2100,2120p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 748


Add trailing space to sendPathToFocusedTerminal to support multiple cmd-click path insertion.

When users cmd-click multiple file paths sequentially, each call to sendText() concatenates without spacing, resulting in invalid shell syntax like '/path/one''/path/two'. Append a space after the escaped path to ensure proper token separation:

private func sendPathToFocusedTerminal(_ path: String) {
    guard let workspace = tabManager.selectedTab,
          let terminalPanel = workspace.focusedTerminalPanel else { return }
    // Shell-escape the path so spaces and special chars don't break commands
    let escaped = shellEscapePath(path)
    terminalPanel.sendText(escaped + " ")
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 1818 - 1824, The
sendPathToFocusedTerminal function currently sends the escaped path without a
trailing space, causing concatenated tokens when cmd-clicking multiple paths;
update the code in sendPathToFocusedTerminal so that after calling
shellEscapePath(path) you append a single space to the string passed to
terminalPanel.sendText (i.e., send escaped + " ") so each inserted path is
separated; reference sendPathToFocusedTerminal, shellEscapePath, and
terminalPanel.sendText when making the change.

@teamleaderleo

Copy link
Copy Markdown
Collaborator

This remains live on current main: the sidebar has file preview support, but no dedicated file-tree sidebar or cmd-click path composition. Leaving it open for a fresh integration with the current sidebar architecture. :)

@teamleaderleo teamleaderleo added the area: sidebar The workspace sidebar: list, groups, status, reordering label Sep 30, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: sidebar The workspace sidebar: list, groups, status, reordering

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants