Skip to content

RIP CURSOR: Add Monaco Editor panel with VS Code-style explorer - #1447

Open
imadbz wants to merge 17 commits into
manaflow-ai:mainfrom
imadbz:feature/monaco-editor-panel
Open

imadbz wants to merge 17 commits into
manaflow-ai:mainfrom
imadbz:feature/monaco-editor-panel

Conversation

@imadbz

@imadbz imadbz commented Mar 14, 2026 •

Copy link
Copy Markdown

Agents code. Humans monitor. For that we need a file explorer with some editing capabilities.
Once we figure this out ... we no longer need Cursor as it is too heavy and not needed.

Summary

  • Adds a new editor panel type that embeds Monaco Editor in a WKWebView, providing a VS Code-like file explorer and code editor integrated into cmux
  • Opens via the doc icon button in the pane tab bar, Cmd+Shift+E keyboard shortcut, or open_editor socket command
  • Scoped to the current workspace's project directory

Screenshot

Screenshot 2026-03-25 at 1 50 17 PM

Features

VS Code-faithful explorer:

  • 22px rows, 16x16 codicon icons, indent guides — exact VS Code Dark Modern dimensions
  • File type-colored icons for 20+ extensions
  • Git status badges (M/A/D/U/R/!) with exact VS Code decoration colors
  • .gitignore support — ignored files faded at 0.4 opacity
  • Parent folder dot indicators bubbled from child git status

File management:

  • Create, rename (F2 with selection cycling), delete files and folders
  • Mouse-based drag-and-drop for moving files/folders
  • Auto-expand folders on 500ms drag hover
  • Multi-select with Cmd+Click and Shift+Click
  • Context menu with New File, New Folder, Rename, Delete, Copy Path

Editor:

  • Monaco Editor with syntax highlighting for 30+ languages
  • Tabbed editor with dirty state indicators
  • Cmd+S save, Cmd+W close, Cmd+N new file
  • Bracket pair colorization, indentation guides, sticky scroll

Integration:

  • Dynamic theming — reads Ghostty terminal background and derives all UI colors
  • Live theme updates when cmux settings change
  • Session persistence and restore
  • File watching with 2s polling (pauses during input/drag)

Security

  • Path traversal prevention with resolvingSymlinksInPath() + exact prefix check
  • All Swift-to-JS string transport uses JSONSerialization (no manual escaping)
  • Sandboxed file I/O — all operations validated against rootPath

Test plan

  • Open editor via tab bar button — verify file tree loads with project files
  • Open/edit/save files — verify syntax highlighting and dirty state
  • Create new file/folder via context menu and header buttons
  • Drag file to folder — verify move and tab path update
  • Multi-select with Cmd+Click and Shift+Click, then drag or delete
  • Change terminal theme in settings — verify editor colors update live
  • Verify git status badges appear on modified/new/untracked files
  • Session restore — close and reopen workspace, verify editor panel restores
  • open_editor /path via socket command

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • In-app Monaco-powered editor with tabs, welcome screen, persistent session state, and theme sync
    • Explorer sidebar: resizable pane, file tree, drag‑and‑drop, inline rename, context menus, and Git status badges
    • Keyboard & UI: Cmd+Shift+E shortcut, command-palette "Editor" entry, terminal "open_editor" command, and common editor shortcuts
    • Host bridge API for editor integration and improved editor styling/resources

imadbz and others added 4 commits March 15, 2026 00:45
New editor panel type that embeds Monaco Editor in a WKWebView, providing
a VS Code-like file explorer and code editor integrated into cmux.

Features:
- Monaco Editor with syntax highlighting for 30+ languages
- File tree sidebar with expand/collapse, sorted (dirs first)
- Tabbed editor with dirty state indicators
- File watching (2s polling, auto-refresh on changes)
- New file/folder creation via context menu, header buttons, or Cmd+N
- Cmd+S save, Cmd+W close tab
- Swift-to-JS bridge for sandboxed file I/O with path traversal protection
- Session persistence and restore for editor panels
- Keyboard shortcut Cmd+Shift+E to open editor for current project
- Socket command: open_editor [path]
- Claude launcher button (sparkle icon) that opens terminal with
  claude --dangerously-skip-permissions
- Editor and Claude buttons added to pane tab bar alongside
  existing terminal and browser buttons

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Major upgrade to the Monaco editor panel:

Design:
- VS Code Dark Modern exact dimensions (22px rows, 16x16 icons, indent guides)
- Codicon font for all icons (files, folders, chevrons, buttons)
- File type-colored icons for 20+ extensions
- VS Code interaction states (hover, active selection, inactive selection)

Git integration:
- git status --porcelain parsing with M/A/D/U/R/! badges
- Exact VS Code git decoration colors (#e2c08d modified, #81b88b added, etc.)
- Ignored files faded at 0.4 opacity
- Deleted files with strikethrough
- Parent folder dot indicators bubbled from child status
- .gitignore respected via git ls-files --ignored

Features:
- Multi-select with Cmd+Click (toggle) and Shift+Click (range)
- Mouse-based drag-and-drop for moving files/folders (WKWebView compatible)
- Auto-expand folders on 500ms drag hover
- Inline rename with F2 selection cycling (stem -> full -> ext)
- Context menu: New File, New Folder, Rename, Delete, Copy Path
- File delete and rename with open tab updates
- File watching pauses during inline input and drag operations

Theme integration:
- Reads Ghostty terminal background color on startup
- Derives all UI colors (sidebar, editor, tabs, borders) from terminal theme
- Listens for ghosttyDefaultBackgroundDidChange notifications
- Monaco editor theme updates dynamically
- Auto-detects light/dark mode from background brightness

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Path traversal: use resolvingSymlinksInPath() instead of standardizingPath
  to prevent symlink-based directory escape
- JS injection: use JSONSerialization for all Swift-to-JS string transport
  instead of manual escaping (which missed newlines and control chars)
- Pipe deadlock: read git process output before waitUntilExit to prevent
  blocking when git status produces large output
- Path prefix check: use exact match or slash-delimited prefix to prevent
  rootPath="/foo" matching "/foobar"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Mar 14, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@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.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented Mar 14, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This pull request introduces a comprehensive Monaco editor and file explorer feature set. It adds Monaco editor integration via a pooled WebView system with Swift↔JavaScript messaging, implements a file explorer sidebar with git status support, adds file search functionality, extends keyboard shortcuts, and updates app infrastructure to support editor panel creation, persistence, and lifecycle management across the UI.

Changes

Cohort / File(s) Summary
Project Configuration
GhosttyTabs.xcodeproj/project.pbxproj
Registers new Swift source files (EditorPanel, EditorPanelView, ExplorerSidebarPanel, ExplorerSidebarView, MonacoWebViewPool, NativeFileExplorer, FileSearchView, SidebarTabSelector) and resource bundles (editor/, explorer/) in build phases and group hierarchy.
Editor Web Resources
Resources/editor/editor.js, Resources/editor/index.html
Implements Monaco editor bridge with window.cmux API supporting file opening, theming, large-file handling, dirty state tracking, and async file writing via postMessage. HTML shell provides editor container and large-file overlay with CSS variables for dark theme.
Explorer Web Resources
Resources/explorer/explorer.js, Resources/explorer/explorer.css, Resources/explorer/index.html
Delivers sidebar file explorer with multi-root folder trees, lazy-loading, git status badges, context menu (create/rename/delete), inline editing, and diffing refresh logic. CSS defines full sidebar theme including colors, indentation, git decorations, and context menu styling.
Editor Panel Implementation
Sources/Panels/EditorPanel.swift, Sources/Panels/EditorPanelView.swift
Introduces EditorPanel @MainActor wrapper around WKWebView with JS message handler for file I/O, dirty/active-file tracking, and large-file detection. Adds EditorPanelView SwiftUI view with focus-flash animation, pointer observer for mouse-driven focus, and focus ring overlay.
Explorer Panel Implementation
Sources/Panels/ExplorerSidebarPanel.swift, Sources/Panels/ExplorerSidebarView.swift
Adds ExplorerSidebarPanel @MainActor with WKWebView hosting explorer UI, ExplorerMessageHandler routing JS messages (readDir, createFile, renameFile, deleteFile, gitStatus), FSEventStream for refresh triggers, and root-path sync. ExplorerSidebarView wraps the webView in SwiftUI.
File Search Implementation
Sources/FileSearchView.swift
Provides FileSearchViewModel performing async recursive directory scanning with result capping (200 max), FileSearchView rendering search field with results list, FileSearchResultRow displaying file paths, and SidebarSearchField NSViewRepresentable ensuring first-responder behavior.
Native File Explorer
Sources/NativeFileExplorer.swift
Introduces FileNode observable tree nodes with lazy-loaded children, FileTreeRoot managing git status/ignored detection via git CLI and FSEvents-driven refresh, NativeFileExplorerViewModel flattening visible rows, and NativeFileExplorerView rendering roots/rows with expand/collapse, git badges, and file icons.
UI Integration & Sidebar
Sources/SidebarTabSelector.swift, Sources/ContentView.swift, Sources/Panels/PanelContentView.swift
Adds SidebarTabSelector buttons for Workspaces/Explorer/Search tabs with multi-tab sidebar switching. ContentView extends to switch between VerticalTabsSidebar, NativeFileExplorerView, and FileSearchView based on selected tab; wires explorer/search viewmodels, file-open/pin behavior, and sidebar toggle shortcut. PanelContentView adds .editor case rendering EditorPanelView.
Keyboard & Shortcuts
Sources/KeyboardShortcutSettings.swift, Sources/AppDelegate.swift
Adds KeyboardShortcutSettings.Action.openEditor case with Cmd+Shift+E default and openEditorShortcut() helper. AppDelegate pre-warms MonacoWebViewPool on app launch, registers Cmd+Shift+F (sidebar search) and Cmd+Shift+E (open editor) handlers, and introduces openEditorForCurrentProject() to resolve workspace root and open editor.
Editor Infrastructure
Sources/MonacoWebViewPool.swift, Sources/GhosttyTerminalView.swift
Implements MonacoWebViewPool singleton maintaining queue of pre-warmed (WKWebView, EditorMessageHandler) pairs with warmUp()/take() lifecycle. GhosttyTerminalView adds guard in applyFirstResponderIfNeeded to prevent stealing focus from actively-edited text fields (e.g., inline rename inputs).
Core Framework Updates
Sources/Panels/Panel.swift, Sources/TabManager.swift, Sources/Workspace.swift, Sources/SessionPersistence.swift, Sources/TerminalController.swift
PanelType gains .editor case. TabManager.openEditor(rootPath:filePath:focus:) -> UUID? creates editor surfaces. Workspace adds editorPanel(for:) accessor, newEditorSurface(inPane:rootPath:filePath:focus:) factory, session snapshot/restore for editors, and SurfaceKind.editor wiring for Bonsplit tabs. SessionPersistence introduces SessionEditorPanelSnapshot. TerminalController adds open_editor socket command handler routing to openEditor().
Submodule Updates
ghostty, vendor/bonsplit
Updates ghostty submodule pointer (bc9be90... → 6f773e0...) and vendor/bonsplit pointer (73c1ef2... → b7dda7f...) without direct functional code changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through code so grand,
With Monaco and file trees planned,
WebViews warmed and shortcuts set,
The finest editor yet, I bet!
From sidebar tabs to search so swift,
This feature is a marvelous gift! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.93% 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
Title check ✅ Passed The title clearly summarizes the main change: adding a Monaco Editor panel with VS Code-style explorer to cmux, directly reflecting the primary feature introduced in this changeset.
Description check ✅ Passed PR description covers what changed (Monaco editor + explorer panel) and why (lightweight alternative to Cursor), includes screenshots, features, security approach, and test plan with checkmarks.

✏️ 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.

@cubic-dev-ai cubic-dev-ai 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.

10 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/ContentView.swift">

<violation number="1" location="Sources/ContentView.swift:4437">
P3: Add `commandPalette.kind.editor` to the localization catalog; currently this new UI label has no translated entry and will always fall back to English.</violation>
</file>

<file name="Sources/Panels/EditorPanel.swift">

<violation number="1" location="Sources/Panels/EditorPanel.swift:146">
P2: Check `createFile`'s return value before sending success; otherwise failed creates are incorrectly reported as successful.</violation>

<violation number="2" location="Sources/Panels/EditorPanel.swift:274">
P2: Reorder status checks so conflict cases are evaluated before generic added/deleted checks; otherwise some merge conflicts are labeled incorrectly.</violation>
</file>

<file name="Resources/editor/index.html">

<violation number="1" location="Resources/editor/index.html:29">
P1: Bundle Monaco assets with the app instead of loading them from jsDelivr at runtime.</violation>
</file>

<file name="Sources/TerminalController.swift">

<violation number="1" location="Sources/TerminalController.swift:12384">
P2: `open_editor` always focuses the new editor panel, bypassing socket focus-mutation gating used by other non-focus commands.</violation>
</file>

<file name="Resources/editor/editor.css">

<violation number="1" location="Resources/editor/editor.css:2">
P2: Do not load required editor UI assets from a public CDN; bundle `codicon.css` with app resources and import it locally to avoid external dependency and supply-chain risk.</violation>
</file>

<file name="Resources/editor/editor.js">

<violation number="1" location="Resources/editor/editor.js:426">
P2: The file watcher can run overlapping async polls, causing concurrent refresh races and extra I/O.</violation>

<violation number="2" location="Resources/editor/editor.js:725">
P1: Dirty tracking is bound to the original file path, so edits stop being tracked after rename/move.</violation>

<violation number="3" location="Resources/editor/editor.js:758">
P1: Closing a dirty tab discards unsaved changes without confirmation.</violation>

<violation number="4" location="Resources/editor/editor.js:938">
P2: Drag-move does not de-duplicate nested selections, so moving a parent and child together can fail mid-operation.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Resources/editor/index.html Outdated
Comment thread Resources/editor/editor.js Outdated
Comment thread Resources/editor/editor.js Outdated
Comment thread Sources/Panels/EditorPanel.swift Outdated
Comment thread Sources/Panels/EditorPanel.swift Outdated
Comment thread Sources/TerminalController.swift Outdated
Comment thread Resources/editor/editor.css Outdated
Comment thread Resources/editor/editor.js Outdated
Comment thread Resources/editor/editor.js Outdated
Comment thread Sources/ContentView.swift
JSONSerialization.data(withJSONObject:) does not accept bare strings as
top-level objects. Wrap in array and strip brackets for jsStringLiteral.
Also simplify sendResponse to pass JSON data directly without re-encoding.

Co-Authored-By: Claude Opus 4.6 (1M context) <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: 18

🧹 Nitpick comments (1)
Resources/editor/index.html (1)

8-8: Add Subresource Integrity (SRI) hashes to external CDN resources.

Loading Monaco from jsDelivr without integrity hashes exposes the application to supply-chain attacks. Additionally, relying on external CDN means the editor won't function offline.

To obtain the SRI hashes, use jsDelivr's metadata API:

curl "https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/editor/editor.main.css?meta" | jq .integrity
curl "https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/loader.js?meta" | jq .integrity

Then add the integrity and crossorigin="anonymous" attributes to both the CSS link (line 8) and JS script (line 29).

Also applies to: 29-29

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

In `@Resources/editor/index.html` at line 8, The external Monaco assets are
included without Subresource Integrity and crossorigin attributes; update the
<link> tag with data-name="vs/editor/editor.main" (href
"https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/editor/editor.main.css")
and the corresponding <script> tag that loads
"https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/loader.js" to include
the SRI integrity attribute values (obtained via jsDelivr metadata API) and add
crossorigin="anonymous" to both tags so the browser can verify the CDN-delivered
files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@ghostty`:
- Line 1: The submodule pointer was updated to commit
6f773e068064904dd002572158fed9aa4f9e6ec9 which only exists locally; push the
ghostty fork branch containing that commit to the remote (manaflow-ai/ghostty)
first, update docs/ghostty-fork.md with the new fork changes and any conflict
notes, and only then update and commit the submodule pointer in the parent repo
so the commit hash is verifiable by CI and reviewers.

In `@Resources/editor/editor.css`:
- Around line 125-128: Add keyboard focus-visible styles mirroring the hover
states for interactive selectors so keyboard users receive comparable visual
feedback: update the rules for .header-btn, .tab, .tab-close,
.context-menu-item, and .tree-row to include :focus-visible variants (e.g.,
.header-btn:focus-visible) that apply the same background/opacity as their
:hover counterparts and include an accessible focus indicator (outline or
box-shadow using a theme color variable). Ensure the :focus-visible rules are
added alongside the existing :hover selectors so they only show for keyboard
focus and do not alter mouse hover behavior.
- Line 114: Update every font-family declaration that currently uses the quoted
literal 'codicon' (occurrences of font-family: 'codicon') to include a generic
fallback (e.g., sans-serif) so it satisfies stylelint; locate each occurrence
(the font-family: 'codicon' declarations around the blocks at lines referenced
in the review) and append a generic family after the quoted name, keeping the
quoted family first and separating with a comma.
- Line 2: Replace the external CDN import in editor.css with a local vendored
copy of the codicons stylesheet: remove the "@import
url('https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css');"
line in Resources/editor/editor.css, add the codicon.css file into
Resources/editor/ (vendor the exact version currently referenced) and update
editor.css to import or reference that local codicon.css; ensure the new
Resources/editor/codicon.css is checked into the repo so the WKWebView/desktop
build uses the local asset instead of fetching from jsDelivr.

In `@Resources/editor/editor.js`:
- Around line 845-848: The forEach arrow callbacks return the expression value
and trigger lint/suspicious/useIterableCallbackReturn; update the callbacks to
use block bodies so they are explicit side-effect-only. In the clearDropTarget
function replace the arrow callback in
treeEl.querySelectorAll('.drop-target').forEach(el =>
el.classList.remove('drop-target')) with a block-bodied arrow (el => {
el.classList.remove('drop-target'); }) and make the same change for the other
querySelectorAll(...).forEach usage around lines 925-926 to eliminate returned
values from the callbacks.
- Around line 938-971: The current bulk move can fail when sources contains a
folder and its descendant; before the renameFile loop, reduce the
Array.from(dragSourcePaths) into only top-level paths by removing any path that
is a descendant of another source (i.e., filter out any src where there exists
another src2 such that src.startsWith(src2 + '/')); do this deduping/filter step
right after creating sources and before performing the moves and refresh steps
(so renameFile, openFiles updates, renderTabs, refreshTree, buildSnapshot
operate only on the top-level items).
- Around line 425-427: The startWatching() implementation uses setInterval(poll,
2000) which can start a new poll() before the previous one finishes (poll calls
buildSnapshot() and getGitStatus()); change the logic so polls never overlap by
making poll() async-aware and either (a) implement an in-flight guard boolean
(e.g., isPolling) checked at the top of poll() and skipped if true, or (b)
replace setInterval with a self-scheduling loop that awaits poll() completion
and then uses setTimeout to schedule the next run; update references in
startWatching() and the poll()/buildSnapshot()/getGitStatus() call sites
accordingly.
- Around line 535-543: The current renameFile resolution only updates an exact
openFiles key and activeFilePath, but must also rewrite any descendant paths
when a folder is renamed; inside the renameFile(...).then(...) callback (the
block containing renameFile, openFiles, activeFilePath, renderTabs), extend the
logic to iterate over openFiles keys and for any key that startsWith(path + "/")
replace that prefix with newPath (update the Map by deleting the old key and
setting the new key with the same file value), and likewise if activeFilePath
startsWith(path + "/") update it to the equivalent newPath-prefixed value; keep
the existing exact-match handling and call renderTabs() after all updates (use
the same symbols: renameFile, openFiles, activeFilePath, renderTabs).
- Around line 975-977: The code currently loads Monaco from jsDelivr via
require.config and require(['vs/editor/editor.main'], ...) which risks executing
remote code; instead vendor Monaco into the application and update the module
loader to reference local files. Install or bundle the monaco-editor package (or
copy its min/vs directory into your app's static assets), change require.config
to use a local relative path for 'vs' (e.g., '/static/monaco/min/vs' or the
app's asset URL) and ensure require(['vs/editor/editor.main'], ...) points to
that local path; finally update build/static asset packaging and any CSP to
serve those files so the editor works offline and no external CDN is contacted.

In `@Sources/AppDelegate.swift`:
- Around line 8790-8797: openEditorForCurrentProject currently ignores a
possible nil from tabManager.openEditor(rootPath:), so update that function to
handle the failure path: capture the return value of
tabManager.openEditor(rootPath:) and if it is nil, emit lightweight feedback
such as logging an error via process logger or showing a user alert/notification
indicating the editor failed to open for workspace.currentDirectory; ensure you
reference openEditorForCurrentProject and tabManager.openEditor(rootPath:) when
making the change so the failure branch is added and feedback is provided.

In `@Sources/Panels/EditorPanel.swift`:
- Around line 174-189: The delete handler currently allows an empty relative
path which resolves to the workspace root and enables destructive operations;
update handleDeleteFile to reject empty or root-targeting operations by
validating the incoming relativePath before resolving (e.g., if
relativePath.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty) and by
comparing the resolvedPath against rootPath and returning sendError for such
cases; apply the same guard logic to the companion rename/delete blocks (the
similar code around the renameFile/removeItem operations) so no API call can
operate on the workspace root even if UI sends an empty path.
- Around line 68-69: The loop over entries in EditorPanel.swift currently skips
every name starting with a dot via entry.hasPrefix("."), which removes valid
project files; remove that blanket check and instead explicitly skip only the
repository folder (e.g., test for entry == ".git") or any other specific names
you really want hidden; update the for-entry loop that calls entries.sorted()
and replace the entry.hasPrefix(".") condition with explicit equality checks
(e.g., entry == ".git" or a small whitelist/blacklist) so dotfiles like
.gitignore or .env are preserved for the JS tree.
- Around line 145-147: The code currently calls
FileManager.createFile(atPath:fullPath,contents:nil) and unconditionally calls
self.sendResponse(..., data: ["success": true], ...); change this to capture the
Bool result (let created = fm.createFile(...)) and only send ["success": true]
when created is true, otherwise send ["success": false] (and optionally include
an error message or log) using the same self.sendResponse(requestId: requestId,
data: ..., webView: webView) path; update the block around fm.createDirectory,
fm.createFile, fullPath, parentDir and requestId in EditorPanel.swift
accordingly.

In `@Sources/TerminalController.swift`:
- Around line 1745-1748: The help output is missing the new "open_editor"
command; update the helpText() function to include a line describing
"open_editor" so it appears in the socket help. Locate helpText() and add a
concise entry for "open_editor" (matching how other commands are documented
there) referencing the same semantics as the switch/case handler (openEditor) so
users can discover the command.
- Around line 12365-12386: The openEditor(_:) command handler currently calls
tabManager.openEditor(rootPath:) which forces focus=true and bypasses
socketCommandAllowsInAppFocusMutations(); change the TabManager API to
openEditor(rootPath: String, focus: Bool) and pass focus through to
workspace.newEditorSurface(..., focus: focus), then update this handler to call
tabManager.openEditor(rootPath: rootPath, focus:
socketCommandAllowsInAppFocusMutations()) so the focus behavior follows the
existing socket focus-mutation policy.

In `@Sources/Workspace.swift`:
- Around line 5271-5272: The code calling newEditorSurface(inPane: pane,
rootPath: currentDirectory) uses the global currentDirectory which can be wrong
for the target pane; change the rootPath to the pane-specific or workspace
project root (e.g. use pane.selectedEditorRoot or pane.projectRoot or
Workspace.shared.projectRoot) so the editor opens against the correct project
tree — update the "editor" case in Workspace.swift to pass the pane's
selected/project root instead of currentDirectory when invoking
newEditorSurface.
- Around line 2538-2573: The editor subscription is only set in
newEditorSurface(_:rootPath:focus:) via installEditorPanelSubscription(_:), so
when an existing EditorPanel is rehomed by
attachDetachedSurface(_:inPane:atIndex:focus:) its title/dirty badge stop
syncing; update the reattach branch that handles detached.panel (the branch that
checks `else if let editorPanel = detached.panel as? EditorPanel`) to call
installEditorPanelSubscription(editorPanel) after reattaching, and apply the
same fix in the other reattach block referenced around lines 2576-2602 to ensure
editor panels regain their subscription after moves.

In `@vendor/bonsplit`:
- Line 1: The parent repo references a bonsplit submodule commit
(b7dda7faffbb58fe70e54d4333f196c442cbc39f) that does not exist on the bonsplit
remote main; either push that commit to the bonsplit repository’s origin main or
move the submodule pointer to an existing commit on origin/main. To fix: go into
vendor/bonsplit, push the branch/commit containing b7dda7f... to the bonsplit
remote main (git push origin HEAD:main) and then commit the updated submodule
pointer in the parent, or check out a valid commit from origin/main in
vendor/bonsplit and update the parent repo’s submodule reference (git add
vendor/bonsplit && git commit) so the parent only points to a commit that exists
on bonsplit’s origin/main.

---

Nitpick comments:
In `@Resources/editor/index.html`:
- Line 8: The external Monaco assets are included without Subresource Integrity
and crossorigin attributes; update the <link> tag with
data-name="vs/editor/editor.main" (href
"https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/editor/editor.main.css")
and the corresponding <script> tag that loads
"https://cdn.jsdelivr.net/npm/monaco-editor@0.52.2/min/vs/loader.js" to include
the SRI integrity attribute values (obtained via jsDelivr metadata API) and add
crossorigin="anonymous" to both tags so the browser can verify the CDN-delivered
files.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 388cc683-1d64-43e2-a9f9-0197de1fddf6

📥 Commits

Reviewing files that changed from the base of the PR and between 6c203b5 and e5095a9.

📒 Files selected for processing (17)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/editor/editor.css
  • Resources/editor/editor.js
  • Resources/editor/index.html
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/KeyboardShortcutSettings.swift
  • Sources/Panels/EditorPanel.swift
  • Sources/Panels/EditorPanelView.swift
  • Sources/Panels/Panel.swift
  • Sources/Panels/PanelContentView.swift
  • Sources/SessionPersistence.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
  • Sources/Workspace.swift
  • ghostty
  • vendor/bonsplit

Comment thread ghostty
Comment thread Resources/editor/editor.css Outdated
Comment thread Resources/editor/editor.css Outdated
Comment thread Resources/editor/editor.css Outdated
Comment thread Resources/editor/editor.js Outdated
Comment on lines +1745 to +1748
// Editor panel commands
case "open_editor":
return openEditor(args)

@coderabbitai coderabbitai Bot Mar 14, 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

Add open_editor to helpText() so the new command is discoverable.

open_editor is now routable, but it is not listed in the socket help output.

💡 Suggested update
         Browser commands:
+          open_editor [root_path]        - Create editor panel (defaults to current workspace directory)
           open_browser [url]              - Create browser panel with optional URL
           navigate <panel_id> <url>       - Navigate browser to URL
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 1745 - 1748, The help output
is missing the new "open_editor" command; update the helpText() function to
include a line describing "open_editor" so it appears in the socket help. Locate
helpText() and add a concise entry for "open_editor" (matching how other
commands are documented there) referencing the same semantics as the switch/case
handler (openEditor) so users can discover the command.

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.

Good catch — will add in a follow-up. The open_editor socket command still works but should be documented in helpText.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rate Limit Exceeded

@imadbz have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 18 seconds before sending another message.

Comment thread Sources/TerminalController.swift
Comment thread Sources/Workspace.swift
Comment on lines +2538 to +2573
func newEditorSurface(
inPane paneId: PaneID,
rootPath: String,
focus: Bool? = nil
) -> EditorPanel? {
let shouldFocusNewTab = focus ?? (bonsplitController.focusedPaneId == paneId)

let editorPanel = EditorPanel(workspaceId: id, rootPath: rootPath)
panels[editorPanel.id] = editorPanel
panelTitles[editorPanel.id] = editorPanel.displayTitle

guard let newTabId = bonsplitController.createTab(
title: editorPanel.displayTitle,
icon: editorPanel.displayIcon,
kind: SurfaceKind.editor,
isDirty: editorPanel.isDirty,
isLoading: false,
isPinned: false,
inPane: paneId
) else {
panels.removeValue(forKey: editorPanel.id)
panelTitles.removeValue(forKey: editorPanel.id)
return nil
}

surfaceIdToPanelId[newTabId] = editorPanel.id

if shouldFocusNewTab {
bonsplitController.focusPane(paneId)
bonsplitController.selectTab(newTabId)
applyTabSelection(tabId: newTabId, inPane: paneId)
}

installEditorPanelSubscription(editorPanel)

return editorPanel

@coderabbitai coderabbitai Bot Mar 14, 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

Reinstall the editor tab subscription after detach/attach moves.

newEditorSurface() is the only path that calls installEditorPanelSubscription(_:). attachDetachedSurface(_:inPane:atIndex:focus:) can rehome an existing EditorPanel into another workspace/window without going through this path, so after a move the tab title and dirty badge stop syncing to Bonsplit.

Follow-up to add in the reattach path
} else if let editorPanel = detached.panel as? EditorPanel {
    installEditorPanelSubscription(editorPanel)
}

Also applies to: 2576-2602

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

In `@Sources/Workspace.swift` around lines 2538 - 2573, The editor subscription is
only set in newEditorSurface(_:rootPath:focus:) via
installEditorPanelSubscription(_:), so when an existing EditorPanel is rehomed
by attachDetachedSurface(_:inPane:atIndex:focus:) its title/dirty badge stop
syncing; update the reattach branch that handles detached.panel (the branch that
checks `else if let editorPanel = detached.panel as? EditorPanel`) to call
installEditorPanelSubscription(editorPanel) after reattaching, and apply the
same fix in the other reattach block referenced around lines 2576-2602 to ensure
editor panels regain their subscription after moves.

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.

Valid point — will address in a follow-up. Editor panels are not commonly detached/reattached across workspaces but the subscription should be reinstalled for correctness.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rate Limit Exceeded

@imadbz have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 15 seconds before sending another message.

Comment thread Sources/Workspace.swift
Comment on lines +5271 to +5272
case "editor":
_ = newEditorSurface(inPane: pane, rootPath: currentDirectory)

@coderabbitai coderabbitai Bot Mar 14, 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 | 🟠 Major

Use the workspace project root here, not currentDirectory.

currentDirectory is workspace-global and follows the last focused panel, not the target pane. In a split workspace this can open the editor on the wrong tree, and if a terminal has cd'd outside the repo it also weakens the intended project-root sandboxing. Prefer the stable workspace/project root, or at minimum the selected editor root in pane.

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

In `@Sources/Workspace.swift` around lines 5271 - 5272, The code calling
newEditorSurface(inPane: pane, rootPath: currentDirectory) uses the global
currentDirectory which can be wrong for the target pane; change the rootPath to
the pane-specific or workspace project root (e.g. use pane.selectedEditorRoot or
pane.projectRoot or Workspace.shared.projectRoot) so the editor opens against
the correct project tree — update the "editor" case in Workspace.swift to pass
the pane's selected/project root instead of currentDirectory when invoking
newEditorSurface.

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.

Valid observation. In the current design, the editor rootPath is used for path resolution (security boundary) while the file is opened by absolute path from the explorer. The currentDirectory is used as the rootPath when creating a new editor panel — this could drift but the file content is read by absolute path regardless.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rate Limit Exceeded

@imadbz have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 14 seconds before sending another message.

Comment thread vendor/bonsplit
@@ -1 +1 @@
Subproject commit 73c1ef2df9a6c8a2837212ecce900794d0f21826
Subproject commit b7dda7faffbb58fe70e54d4333f196c442cbc39f

@coderabbitai coderabbitai Bot Mar 14, 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 | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify the bonsplit submodule commit is on remote main

cd vendor/bonsplit
git fetch origin main
git merge-base --is-ancestor b7dda7faffbb58fe70e54d4333f196c442cbc39f origin/main

if [ $? -eq 0 ]; then
  echo "✓ Commit b7dda7faffbb58fe70e54d4333f196c442cbc39f is on origin/main"
else
  echo "✗ Commit b7dda7faffbb58fe70e54d4333f196c442cbc39f is NOT on origin/main"
  exit 1
fi

Repository: manaflow-ai/cmux

Length of output: 407


The bonsplit submodule commit must be pushed to remote main before updating the pointer.

The commit b7dda7faffbb58fe70e54d4333f196c442cbc39f does not exist on the remote main branch. Per the submodule workflow, the commit must be pushed to the bonsplit repository's remote main branch before the parent repo can reference it. Update the submodule pointer to a commit that exists on origin/main, or push the current work to remote main and update the reference accordingly.

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

In `@vendor/bonsplit` at line 1, The parent repo references a bonsplit submodule
commit (b7dda7faffbb58fe70e54d4333f196c442cbc39f) that does not exist on the
bonsplit remote main; either push that commit to the bonsplit repository’s
origin main or move the submodule pointer to an existing commit on origin/main.
To fix: go into vendor/bonsplit, push the branch/commit containing b7dda7f... to
the bonsplit remote main (git push origin HEAD:main) and then commit the updated
submodule pointer in the parent, or check out a valid commit from origin/main in
vendor/bonsplit and update the parent repo’s submodule reference (git add
vendor/bonsplit && git commit) so the parent only points to a commit that exists
on bonsplit’s origin/main.

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.

The bonsplit submodule has a local isItalic addition for preview tab italic title rendering. This needs to be pushed to the bonsplit fork before merge. Will handle in pre-merge prep.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Rate Limit Exceeded

@imadbz have exceeded the limit for the number of chat messages per hour. Please wait 1 minutes and 13 seconds before sending another message.

@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.

♻️ Duplicate comments (2)
Sources/Panels/EditorPanel.swift (2)

174-191: ⚠️ Potential issue | 🔴 Critical

Block destructive operations against the workspace root.

resolvedPath("") returns rootPath (Lines 311-312), and the guard at Line 321 permits canonical == canonicalRoot. This means deleteFile("") will recursively remove the entire project directory. Add an explicit check to reject empty or root-targeting paths for destructive operations.

 private func handleDeleteFile(body: [String: Any], webView: WKWebView?) {
     let relativePath = body["path"] as? String ?? ""
     let requestId = body["requestId"] as? String ?? ""

+    // Block deletion of root directory
+    guard !relativePath.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
+        self.sendError(requestId: requestId, message: "Cannot delete root directory", webView: webView)
+        return
+    }
+
     DispatchQueue.global(qos: .userInitiated).async { [rootPath] in
         let fullPath = self.resolvedPath(relativePath, rootPath: rootPath)
         guard let fullPath else {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/EditorPanel.swift` around lines 174 - 191, handleDeleteFile
currently allows deleting the workspace root when relativePath is empty or
resolves to root; update handleDeleteFile to explicitly reject empty paths and
any path that equals or is the same canonical path as rootPath before calling
FileManager.removeItem. Use resolvedPath(relativePath, rootPath: rootPath) and
compare the resolved fullPath (or its canonicalized URL/path) against rootPath
(or its canonicalized form) and if they match or relativePath.isEmpty, call
sendError(requestId: message: webView:) with an explicit message like "Refusing
to delete workspace root" and return; only proceed to removeItem if the check
passes.

193-215: ⚠️ Potential issue | 🔴 Critical

Apply the same root protection to rename operations.

Similar to delete, renameFile("", ...) could move the entire workspace directory. Block empty oldPath values.

 private func handleRenameFile(body: [String: Any], webView: WKWebView?) {
     let oldRelPath = body["oldPath"] as? String ?? ""
     let newRelPath = body["newPath"] as? String ?? ""
     let requestId = body["requestId"] as? String ?? ""

+    // Block renaming of root directory
+    guard !oldRelPath.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
+        self.sendError(requestId: requestId, message: "Cannot rename root directory", webView: webView)
+        return
+    }
+
     DispatchQueue.global(qos: .userInitiated).async { [rootPath] in
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/EditorPanel.swift` around lines 193 - 215, The rename handler
(handleRenameFile) currently allows empty oldPath which can resolve to the
workspace root and permit moving the entire workspace; add a guard at the start
of the method (before resolving paths) to reject empty oldPath (oldRelPath ==
"") by calling sendError(requestId: requestId, message: "Invalid path", webView:
webView) and returning; ensure you similarly treat an empty newPath as invalid
and return early so resolvedPath is never called with an empty string
(references: oldRelPath, newRelPath, resolvedPath, sendError, sendResponse in
handleRenameFile).
🧹 Nitpick comments (1)
Sources/Panels/EditorPanel.swift (1)

217-244: Consider using xcrun or PATH lookup for git.

The hardcoded /usr/bin/git path works when Xcode Command Line Tools are installed but may fail in environments where git is only available via Homebrew or other package managers. Consider using /usr/bin/xcrun git or looking up git in PATH for broader compatibility.

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

In `@Sources/Panels/EditorPanel.swift` around lines 217 - 244, The code in
handleGitStatus currently hardcodes "/usr/bin/git" for statusProcess and
ignoredProcess which can fail if git is not at that path; change to resolve git
dynamically (e.g., prefer using "/usr/bin/xcrun" as the executable and pass
"git" as the first argument, or look up git on PATH at startup and store its
URL) and then set executableURL and arguments of statusProcess and
ignoredProcess accordingly; if resolution fails, call sendError(requestId:
requestId, message: "git not available", webView: webView) as before. Ensure the
change touches the statusProcess/ignoredProcess creation in handleGitStatus and
uses the resolved git executable instead of the hardcoded path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Sources/Panels/EditorPanel.swift`:
- Around line 174-191: handleDeleteFile currently allows deleting the workspace
root when relativePath is empty or resolves to root; update handleDeleteFile to
explicitly reject empty paths and any path that equals or is the same canonical
path as rootPath before calling FileManager.removeItem. Use
resolvedPath(relativePath, rootPath: rootPath) and compare the resolved fullPath
(or its canonicalized URL/path) against rootPath (or its canonicalized form) and
if they match or relativePath.isEmpty, call sendError(requestId: message:
webView:) with an explicit message like "Refusing to delete workspace root" and
return; only proceed to removeItem if the check passes.
- Around line 193-215: The rename handler (handleRenameFile) currently allows
empty oldPath which can resolve to the workspace root and permit moving the
entire workspace; add a guard at the start of the method (before resolving
paths) to reject empty oldPath (oldRelPath == "") by calling
sendError(requestId: requestId, message: "Invalid path", webView: webView) and
returning; ensure you similarly treat an empty newPath as invalid and return
early so resolvedPath is never called with an empty string (references:
oldRelPath, newRelPath, resolvedPath, sendError, sendResponse in
handleRenameFile).

---

Nitpick comments:
In `@Sources/Panels/EditorPanel.swift`:
- Around line 217-244: The code in handleGitStatus currently hardcodes
"/usr/bin/git" for statusProcess and ignoredProcess which can fail if git is not
at that path; change to resolve git dynamically (e.g., prefer using
"/usr/bin/xcrun" as the executable and pass "git" as the first argument, or look
up git on PATH at startup and store its URL) and then set executableURL and
arguments of statusProcess and ignoredProcess accordingly; if resolution fails,
call sendError(requestId: requestId, message: "git not available", webView:
webView) as before. Ensure the change touches the statusProcess/ignoredProcess
creation in handleGitStatus and uses the resolved git executable instead of the
hardcoded path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 165d1f9a-399a-489e-87eb-bc422a7c3ee1

📥 Commits

Reviewing files that changed from the base of the PR and between e5095a9 and b3f5a3e.

📒 Files selected for processing (1)
  • Sources/Panels/EditorPanel.swift

imadbz and others added 2 commits March 15, 2026 04:14
- Dirty tab close: confirm before discarding unsaved changes
- Dirty tracking: closure references fileEntry directly, survives rename
- Git conflict ordering: U/U, A/A, D/D checks before generic A/D
- createFile: check return value, error on failure
- Socket focus: respect socketCommandAllowsInAppFocusMutations()
- Poll guard: prevent overlapping async polls with pollRunning flag
- Drag dedup: filter nested selections when parent is also selected

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Block delete/rename of project root (empty path guard + canonical check)
- Show dotfiles except .git (was hiding all dotfiles)
- Folder rename propagates to all descendant open tabs
- Submodule commits on detached HEAD acknowledged as known limitation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@imadbz

imadbz commented Mar 14, 2026

Copy link
Copy Markdown
Author

Review feedback addressed

All issues from the automated reviews have been addressed in commits ee7d7ede and 5055f5e8:

Fixed

Issue Fix
Dirty tab close discards changes Added confirmation dialog before closing dirty tabs
Dirty tracking breaks after rename Closure references fileEntry directly instead of capturing path
Git conflict ordering Conflict checks (U/U, A/A, D/D) now evaluate before generic A/D
createFile return unchecked Now checks return value and sends error on failure
Socket focus gating bypassed open_editor now respects socketCommandAllowsInAppFocusMutations()
Overlapping async polls pollRunning guard with try/finally
Nested drag dedup Filters children when parent is also selected
Delete/rename project root Empty path guard + canonical root check blocks destructive ops on workspace root
Dotfiles hidden Now only hides .git; all other dotfiles (.gitignore, .env, etc.) visible
Folder rename breaks tabs Inline rename now propagates to all descendant open tabs

Acknowledged (follow-up)

Issue Plan
Bundle Monaco/codicons locally Planned as separate PR — keeps initial diff manageable (~4MB vendor assets)
Submodule commits on detached HEAD Known workflow constraint — need pushing to fork remotes before merge
Add open_editor to help text Will add in follow-up
Localization catalog entries Keys use String(localized:defaultValue:) which auto-registers; translations in follow-up

@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: 2

♻️ Duplicate comments (4)
Resources/editor/editor.js (2)

984-986: ⚠️ Potential issue | 🔴 Critical

Bundle Monaco with the app instead of executing it from jsDelivr.

Anything loaded from this CDN runs inside the same privileged WKWebView and can call window.webkit.messageHandlers.cmuxEditor.postMessage(...) to reach the file-system bridge. That keeps editor I/O dependent on third-party runtime code and also makes the editor unavailable offline.

#!/bin/bash
# Verify Monaco is still loaded remotely and not bundled under Resources/editor.
rg -n 'cdn.jsdelivr.net/npm/monaco-editor|require\.config' Resources/editor/editor.js
find Resources/editor -maxdepth 3 \( -type d -name vs -o -type f -name 'editor.main.js' \)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resources/editor/editor.js` around lines 984 - 986, The code currently loads
Monaco from jsDelivr via require.config and require(['vs/editor/editor.main'],
...); replace the remote loading with a bundled local copy: remove the CDN URL
in the require.config paths and point "vs" to the local Resources/editor/vs
directory (after adding the Monaco package files under Resources/editor/vs and
editor.main.js), update any build scripts to copy Monaco into Resources/editor,
and verify require(['vs/editor/editor.main']) loads the local editor.main.js so
the editor works offline and no third-party runtime runs inside the app's
WKWebView.

540-548: ⚠️ Potential issue | 🟠 Major

Rewrite descendant open paths on folder rename.

This still only updates an exact openFiles hit. Renaming a directory leaves nested tabs and activeFilePath under the old prefix, so the next save targets a path that no longer exists.

💡 Suggested fix
-                    if (openFiles.has(path)) {
-                        const file = openFiles.get(path);
-                        openFiles.delete(path);
-                        openFiles.set(newPath, file);
-                        if (activeFilePath === path) activeFilePath = newPath;
-                        renderTabs();
-                    }
+                    const updates = [];
+                    for (const [openPath, file] of openFiles) {
+                        if (openPath === path || openPath.startsWith(path + '/')) {
+                            updates.push([openPath, newPath + openPath.substring(path.length), file]);
+                        }
+                    }
+                    for (const [oldPath, rewrittenPath, file] of updates) {
+                        openFiles.delete(oldPath);
+                        openFiles.set(rewrittenPath, file);
+                        if (activeFilePath === oldPath) activeFilePath = rewrittenPath;
+                    }
+                    renderTabs();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resources/editor/editor.js` around lines 540 - 548, When handling the
renameFile(path, newPath).then(...) callback, currently only the exact key is
updated; instead, iterate over openFiles (the Map) and for every key that is
equal to path or startsWith(path + '/'), compute the suffix =
key.slice(path.length) and set a newKey = newPath + suffix, move the value
(get/delete/set) to newKey, and if activeFilePath === key or activeFilePath
startsWith(path + '/'), update activeFilePath to the corresponding newKey;
finally call renderTabs() once. Update the block that currently checks
openFiles.has(path) to perform this descendant-key rewrite for all matching
keys.
Sources/Panels/EditorPanel.swift (2)

68-69: ⚠️ Potential issue | 🟠 Major

Do not blanket-hide dotfiles from the explorer.

This drops .gitignore, .env, .swiftlint.yml, and similar project files before the JS tree ever sees them. If the intent is only to hide repository internals, filter .git explicitly instead of every leading-dot entry.

💡 Suggested fix
-                if entry.hasPrefix(".") { continue }
+                if entry == ".git" { continue }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/EditorPanel.swift` around lines 68 - 69, The loop in
EditorPanel.swift currently skips every leading-dot entry (if
entry.hasPrefix(".")) which hides legitimate dotfiles; change that check to only
filter repository internals by explicitly skipping the .git directory (e.g.,
replace if entry.hasPrefix(".") { continue } with if entry == ".git" { continue
}) so files like .gitignore and .env are included in the entries shown.

177-213: ⚠️ Potential issue | 🔴 Critical

Reject destructive operations against the workspace root.

resolvedPath("") returns rootPath, so an empty relative path can still delete the entire workspace or rename/move it. The blank-area context menu in the editor can already emit "", so these bridge handlers need to reject empty/root-targeting paths before resolving them.

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

In `@Sources/Panels/EditorPanel.swift` around lines 177 - 213, Reject operations
that target the workspace root by validating inputs in handleDeleteFile and
handleRenameFile: first, immediately reject and call sendError if the incoming
relative path strings (relativePath, oldRelPath, newRelPath) are empty or only
whitespace; then after resolving with resolvedPath, also check that the resolved
full path(s) are not equal to rootPath and reject if they are (for rename ensure
neither oldFull nor newFull equals rootPath). Apply these checks before
performing FileManager.removeItem or moveItem, using the existing requestId and
webView to return errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resources/editor/editor.js`:
- Around line 45-60: updateMonacoTheme currently returns early if monacoInstance
or editor are not ready, causing the host theme to be lost; modify the logic so
that when monacoInstance or editor is missing the function caches the provided
editorBg and editorFg (e.g., module-level variables like
cachedEditorBg/cachedEditorFg) and still returns, and then ensure the Monaco
initialization path (the function that sets up monacoInstance/editor) checks for
those cachedEditorBg/cachedEditorFg and calls updateMonacoTheme(cachedEditorBg,
cachedEditorFg) after monacoInstance/editor are created; reference
updateMonacoTheme and the Monaco init routine so the host theme is applied as
soon as Monaco becomes ready.
- Around line 495-504: The confirmDelete handler currently only checks
openFiles.has(path) so deleting a directory leaves descendant file tabs open;
modify confirmDelete (and the other delete site around functions referenced
similarly) to, when isDir is true, iterate openFiles (keys or entries) and for
each filePath that is a descendant of the deleted directory (startsWith(path +
'/' or path + pathSeparator), or otherwise matches directory prefix) call
closeFileTab(filePath) before refreshing; retain the existing single-file
behavior (openFiles.has(path) -> closeFileTab(path)) when isDir is false, and
ensure this descendant-closing logic is applied in both places mentioned
(confirmDelete and the duplicate delete block later).

---

Duplicate comments:
In `@Resources/editor/editor.js`:
- Around line 984-986: The code currently loads Monaco from jsDelivr via
require.config and require(['vs/editor/editor.main'], ...); replace the remote
loading with a bundled local copy: remove the CDN URL in the require.config
paths and point "vs" to the local Resources/editor/vs directory (after adding
the Monaco package files under Resources/editor/vs and editor.main.js), update
any build scripts to copy Monaco into Resources/editor, and verify
require(['vs/editor/editor.main']) loads the local editor.main.js so the editor
works offline and no third-party runtime runs inside the app's WKWebView.
- Around line 540-548: When handling the renameFile(path, newPath).then(...)
callback, currently only the exact key is updated; instead, iterate over
openFiles (the Map) and for every key that is equal to path or startsWith(path +
'/'), compute the suffix = key.slice(path.length) and set a newKey = newPath +
suffix, move the value (get/delete/set) to newKey, and if activeFilePath === key
or activeFilePath startsWith(path + '/'), update activeFilePath to the
corresponding newKey; finally call renderTabs() once. Update the block that
currently checks openFiles.has(path) to perform this descendant-key rewrite for
all matching keys.

In `@Sources/Panels/EditorPanel.swift`:
- Around line 68-69: The loop in EditorPanel.swift currently skips every
leading-dot entry (if entry.hasPrefix(".")) which hides legitimate dotfiles;
change that check to only filter repository internals by explicitly skipping the
.git directory (e.g., replace if entry.hasPrefix(".") { continue } with if entry
== ".git" { continue }) so files like .gitignore and .env are included in the
entries shown.
- Around line 177-213: Reject operations that target the workspace root by
validating inputs in handleDeleteFile and handleRenameFile: first, immediately
reject and call sendError if the incoming relative path strings (relativePath,
oldRelPath, newRelPath) are empty or only whitespace; then after resolving with
resolvedPath, also check that the resolved full path(s) are not equal to
rootPath and reject if they are (for rename ensure neither oldFull nor newFull
equals rootPath). Apply these checks before performing FileManager.removeItem or
moveItem, using the existing requestId and webView to return errors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 64f61196-43f6-4992-a1f6-d3c13f3b1052

📥 Commits

Reviewing files that changed from the base of the PR and between b3f5a3e and ee7d7ed.

📒 Files selected for processing (4)
  • Resources/editor/editor.js
  • Sources/Panels/EditorPanel.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Sources/TabManager.swift
  • Sources/TerminalController.swift

Comment on lines +45 to +60
updateMonacoTheme(editorBg, editorFg) {
if (!monacoInstance || !editor) return;
monacoInstance.editor.defineTheme('cmux-dark', {
base: 'vs-dark',
inherit: true,
rules: [],
colors: {
'editor.background': editorBg,
'editorGutter.background': editorBg,
'editor.lineHighlightBackground': editorBg + '20',
'editorLineNumber.foreground': editorFg + '55',
'editorLineNumber.activeForeground': editorFg + 'cc'
}
});
monacoInstance.editor.setTheme('cmux-dark');
}

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

Cache the host theme until Monaco is ready.

updateMonacoTheme() drops the initial colors whenever Swift calls it before monacoInstance and editor exist, and the init path then installs hard-coded defaults instead. The result is that the explorer picks up the Ghostty-derived palette immediately, but Monaco stays on the fallback colors until some later theme-change event fires.

💡 Suggested fix
+    const pendingTheme = { editorBg: '#1f1f1f', editorFg: '#c6c6c6' };
     window.cmux = {
         ...
         updateMonacoTheme(editorBg, editorFg) {
-            if (!monacoInstance || !editor) return;
+            pendingTheme.editorBg = editorBg;
+            pendingTheme.editorFg = editorFg;
+            if (!monacoInstance || !editor) return;
             monacoInstance.editor.defineTheme('cmux-dark', {
                 base: 'vs-dark',
                 inherit: true,
@@
     require(['vs/editor/editor.main'], async function (monaco) {
         monacoInstance = monaco;
-
-        monaco.editor.defineTheme('cmux-dark', {
-            base: 'vs-dark',
-            inherit: true,
-            rules: [],
-            colors: {
-                'editor.background': '#1f1f1f',
-                'editorGutter.background': '#1f1f1f',
-                'editor.lineHighlightBackground': '#2a2d2e',
-                'editorLineNumber.foreground': '#5a5a5a',
-                'editorLineNumber.activeForeground': '#c6c6c6'
-            }
-        });
 
         editor = monaco.editor.create(document.getElementById('editor-container'), {
             theme: 'cmux-dark',
             ...
         });
+        window.cmux.updateMonacoTheme(pendingTheme.editorBg, pendingTheme.editorFg);

Also applies to: 989-1020

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

In `@Resources/editor/editor.js` around lines 45 - 60, updateMonacoTheme currently
returns early if monacoInstance or editor are not ready, causing the host theme
to be lost; modify the logic so that when monacoInstance or editor is missing
the function caches the provided editorBg and editorFg (e.g., module-level
variables like cachedEditorBg/cachedEditorFg) and still returns, and then ensure
the Monaco initialization path (the function that sets up monacoInstance/editor)
checks for those cachedEditorBg/cachedEditorFg and calls
updateMonacoTheme(cachedEditorBg, cachedEditorFg) after monacoInstance/editor
are created; reference updateMonacoTheme and the Monaco init routine so the host
theme is applied as soon as Monaco becomes ready.

Comment thread Resources/editor/editor.js Outdated
Comment on lines +495 to +504
async function confirmDelete(path, name, isDir) {
// Simple confirm — could be a modal later
if (!confirm(`Delete "${name}"?`)) return;
try {
await deleteFile(path);
// Close if open in editor
if (openFiles.has(path)) closeFileTab(path);
await refreshTree();
lastTreeSnapshot = await buildSnapshot('');
} catch (err) { console.error('Delete failed:', err); }

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

Close descendant tabs when deleting a directory.

Both delete paths only check openFiles.has(path), but openFiles contains file paths, not directory paths. Deleting a folder therefore leaves every open descendant tab mounted against files that no longer exist, and dirty descendants bypass the normal discard flow until much later.

Also applies to: 821-825

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

In `@Resources/editor/editor.js` around lines 495 - 504, The confirmDelete handler
currently only checks openFiles.has(path) so deleting a directory leaves
descendant file tabs open; modify confirmDelete (and the other delete site
around functions referenced similarly) to, when isDir is true, iterate openFiles
(keys or entries) and for each filePath that is a descendant of the deleted
directory (startsWith(path + '/' or path + pathSeparator), or otherwise matches
directory prefix) call closeFileTab(filePath) before refreshing; retain the
existing single-file behavior (openFiles.has(path) -> closeFileTab(path)) when
isDir is false, and ensure this descendant-closing logic is applied in both
places mentioned (confirmDelete and the duplicate delete block later).

@imadbz

imadbz commented Mar 14, 2026

Copy link
Copy Markdown
Author

@lawrencecchen @austinywang
first of all ... thanks for pushing such an awesome product! I completely ditched cursor/warp/t3.code/codex in favor of CMUX. I feel 10x more productive with it.

I added a lightweight file explorer in this PR. The idea is since agents doing most of the work, you dont need heavy VS code stuff to code, you just need a lightweight notepad thing just to monitor the files. read the output ... etc.

let me know if you have any feedback, would love to help out

@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.

♻️ Duplicate comments (4)
Resources/editor/editor.js (4)

857-857: ⚠️ Potential issue | 🟡 Minor

Use block-bodied forEach callbacks to satisfy Biome.

Line 857 and Line 937 still trigger lint/suspicious/useIterableCallbackReturn.

🧹 Suggested fix
- treeEl.querySelectorAll('.drop-target').forEach(el => el.classList.remove('drop-target'));
+ treeEl.querySelectorAll('.drop-target').forEach(el => {
+   el.classList.remove('drop-target');
+ });

- treeEl.querySelectorAll('.cut').forEach(el => el.classList.remove('cut'));
+ treeEl.querySelectorAll('.cut').forEach(el => {
+   el.classList.remove('cut');
+ });

Also applies to: 937-937

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

In `@Resources/editor/editor.js` at line 857, Replace the concise arrow callbacks
used with forEach to use block-bodied callbacks so Biome lint passes;
specifically locate the calls like
treeEl.querySelectorAll('.drop-target').forEach(el =>
el.classList.remove('drop-target')) (and the similar occurrence around the other
instance) and change the callback to a block form that calls
el.classList.remove('drop-target') inside a { ... } body (and do the analogous
change for the second occurrence).

45-60: ⚠️ Potential issue | 🟠 Major

Cache host theme values until Monaco is ready.

Line 46 drops early theme updates, so Monaco can stay on fallback colors even though explorer CSS already updated. Persist the latest host colors and reapply after editor creation.

💡 Suggested fix
 let editor = null;
 let monacoInstance = null;
+const pendingTheme = { editorBg: '#1f1f1f', editorFg: '#c6c6c6' };

 window.cmux = {
   ...
   updateMonacoTheme(editorBg, editorFg) {
-      if (!monacoInstance || !editor) return;
+      pendingTheme.editorBg = editorBg;
+      pendingTheme.editorFg = editorFg;
+      if (!monacoInstance || !editor) return;
       monacoInstance.editor.defineTheme('cmux-dark', {
         base: 'vs-dark',
         inherit: true,
@@
 require(['vs/editor/editor.main'], async function (monaco) {
   monacoInstance = monaco;

   monaco.editor.defineTheme('cmux-dark', {
     base: 'vs-dark',
     inherit: true,
     rules: [],
     colors: {
-      'editor.background': '#1f1f1f',
-      'editorGutter.background': '#1f1f1f',
-      'editor.lineHighlightBackground': '#2a2d2e',
-      'editorLineNumber.foreground': '#5a5a5a',
-      'editorLineNumber.activeForeground': '#c6c6c6'
+      'editor.background': pendingTheme.editorBg,
+      'editorGutter.background': pendingTheme.editorBg,
+      'editor.lineHighlightBackground': pendingTheme.editorBg + '20',
+      'editorLineNumber.foreground': pendingTheme.editorFg + '55',
+      'editorLineNumber.activeForeground': pendingTheme.editorFg + 'cc'
     }
   });

   editor = monaco.editor.create(document.getElementById('editor-container'), {
     theme: 'cmux-dark',
@@
     stickyScroll: { enabled: true }
   });
+  window.cmux.updateMonacoTheme(pendingTheme.editorBg, pendingTheme.editorFg);

Also applies to: 994-1005

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

In `@Resources/editor/editor.js` around lines 45 - 60, updateMonacoTheme currently
returns early when monacoInstance or editor isn't ready, dropping host theme
updates; change this by caching the last editorBg/editorFg in module-level
variables (e.g., lastEditorBg, lastEditorFg) inside updateMonacoTheme so calls
always store the latest values, and then when the editor is created/initialized
(where monacoInstance and editor are set) call updateMonacoTheme() or reapply
the cached values to defineTheme/setTheme so the persisted host colors are
applied once Monaco is ready; reference updateMonacoTheme, monacoInstance,
editor, and the editor creation/initialization code to add the reapply step.

495-503: ⚠️ Potential issue | 🟠 Major

Close descendant tabs when deleting a folder.

Line 501 and Line 829 only close exact path matches. Directory deletes leave open descendant tabs pointing to missing files.

🧩 Suggested fix
 async function confirmDelete(path, name, isDir) {
   ...
   await deleteFile(path);
-  if (openFiles.has(path)) closeFileTab(path);
+  if (isDir) {
+    const toClose = Array.from(openFiles.keys()).filter(p => p === path || p.startsWith(path + '/'));
+    for (const p of toClose) closeFileTab(p);
+  } else if (openFiles.has(path)) {
+    closeFileTab(path);
+  }
   await refreshTree();
   ...
 }

 // keyboard delete
 for (const p of paths) {
   try {
     await deleteFile(p);
-    if (openFiles.has(p)) closeFileTab(p);
+    const row = treeEl.querySelector(`.tree-row[data-path="${CSS.escape(p)}"]`);
+    const isDir = row?.dataset.isDir === '1';
+    if (isDir) {
+      const toClose = Array.from(openFiles.keys()).filter(fp => fp === p || fp.startsWith(p + '/'));
+      for (const fp of toClose) closeFileTab(fp);
+    } else if (openFiles.has(p)) {
+      closeFileTab(p);
+    }
   } catch (err) { console.error('Delete failed:', err); }
 }

Also applies to: 821-830

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

In `@Resources/editor/editor.js` around lines 495 - 503, When deleting a directory
in confirmDelete, detect isDir and after successful delete iterate openFiles to
close any open file whose path is a descendant of the deleted directory (e.g.,
startsWith(deletedPathWithSeparator)) instead of only closing exact matches; use
the same logic where closeFileTab is used elsewhere (refer to confirmDelete,
openFiles, closeFileTab, deleteFile, refreshTree, buildSnapshot) and
normalize/trailing-separator the deleted path before matching so sibling
prefixes don't get accidentally closed.

989-990: ⚠️ Potential issue | 🔴 Critical

Avoid loading Monaco from CDN in this privileged bridge context.

Line 989 fetches executable editor runtime from jsDelivr. With window.webkit.messageHandlers.cmuxEditor available, this keeps the filesystem bridge exposed to third-party runtime code. Bundle Monaco locally and load vs from app resources.

#!/bin/bash
# Verify runtime Monaco source and local vendoring status.
rg -n "require\\.config\\(\\{ paths: \\{ vs: 'https://cdn\\.jsdelivr\\.net/npm/monaco-editor" Resources/editor/editor.js
fd --hidden --type d "vs" Resources/editor
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Resources/editor/editor.js` around lines 989 - 990, The code currently
configures RequireJS to load Monaco from a remote CDN via the require.config
call (the "vs" path), which pulls executable runtime into the privileged bridge;
change this to use a locally bundled copy of Monaco instead: vendor and include
the monaco "vs" directory inside your app resources, update the require.config({
paths: { vs: ... } }) entry to point to that local resource path (relative app
bundle path under Resources/editor or equivalent), and remove any external CDN
URL so Monaco is loaded from the packaged files rather than jsDelivr; ensure the
new path resolves at runtime in the same module-loading spot where
require.config is currently invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@Resources/editor/editor.js`:
- Line 857: Replace the concise arrow callbacks used with forEach to use
block-bodied callbacks so Biome lint passes; specifically locate the calls like
treeEl.querySelectorAll('.drop-target').forEach(el =>
el.classList.remove('drop-target')) (and the similar occurrence around the other
instance) and change the callback to a block form that calls
el.classList.remove('drop-target') inside a { ... } body (and do the analogous
change for the second occurrence).
- Around line 45-60: updateMonacoTheme currently returns early when
monacoInstance or editor isn't ready, dropping host theme updates; change this
by caching the last editorBg/editorFg in module-level variables (e.g.,
lastEditorBg, lastEditorFg) inside updateMonacoTheme so calls always store the
latest values, and then when the editor is created/initialized (where
monacoInstance and editor are set) call updateMonacoTheme() or reapply the
cached values to defineTheme/setTheme so the persisted host colors are applied
once Monaco is ready; reference updateMonacoTheme, monacoInstance, editor, and
the editor creation/initialization code to add the reapply step.
- Around line 495-503: When deleting a directory in confirmDelete, detect isDir
and after successful delete iterate openFiles to close any open file whose path
is a descendant of the deleted directory (e.g.,
startsWith(deletedPathWithSeparator)) instead of only closing exact matches; use
the same logic where closeFileTab is used elsewhere (refer to confirmDelete,
openFiles, closeFileTab, deleteFile, refreshTree, buildSnapshot) and
normalize/trailing-separator the deleted path before matching so sibling
prefixes don't get accidentally closed.
- Around line 989-990: The code currently configures RequireJS to load Monaco
from a remote CDN via the require.config call (the "vs" path), which pulls
executable runtime into the privileged bridge; change this to use a locally
bundled copy of Monaco instead: vendor and include the monaco "vs" directory
inside your app resources, update the require.config({ paths: { vs: ... } })
entry to point to that local resource path (relative app bundle path under
Resources/editor or equivalent), and remove any external CDN URL so Monaco is
loaded from the packaged files rather than jsDelivr; ensure the new path
resolves at runtime in the same module-loading spot where require.config is
currently invoked.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7243c1ce-e50e-4e02-be3b-6b0a186da31a

📥 Commits

Reviewing files that changed from the base of the PR and between ee7d7ed and 5055f5e.

📒 Files selected for processing (2)
  • Resources/editor/editor.js
  • Sources/Panels/EditorPanel.swift

@imadbz imadbz changed the title Add Monaco Editor panel with VS Code-style explorer Replace CURSOR: Add Monaco Editor panel with VS Code-style explorer Mar 16, 2026
@imadbz imadbz changed the title Replace CURSOR: Add Monaco Editor panel with VS Code-style explorer RIP CURSOR: Add Monaco Editor panel with VS Code-style explorer Mar 16, 2026
@jsklan

jsklan commented Mar 19, 2026

Copy link
Copy Markdown

Just found this and would love to get this feature merged. Not sure how complex it would be to also make it so I didn't need to see the file tree as well. I'm thinking having files openable with syntax highlighting, live updates if an ai agent edits, and editability with autosave would be the main features I want. It would just be sick to never have to have full view of agents, code, and github all in one cmux workspace.
@lawrencecchen @austinywang any opposition to adding a feature like this?

@lawrencecchen

Copy link
Copy Markdown
Contributor

Super cool! I think main thing right now is that there's doubly nested tabs, which feels kinda weird ideally I think all tabs should be top level. Also file explorer might deserve to stick to the right? And will need to handle every directory that has a terminal present, as well as remote ssh stuff (cmux ssh is in latest nightly(

image

@cjraft

cjraft commented Mar 20, 2026 •

Copy link
Copy Markdown

@imadbz just need this capability, and I’m glad to see you’ve already built it. Thanks, great job.

imadbz and others added 8 commits March 23, 2026 13:50
# Conflicts:
#	Sources/Workspace.swift
#	vendor/bonsplit
- Add file explorer WebView in the left sidebar with git status, tree
  navigation, context menus, inline rename, and new file/folder creation
- Clicking a file in the sidebar opens it as a new editor tab (one file
  per panel)
- Strip editor panel down to single-file Monaco: no built-in explorer,
  no tabs, no folder watching — just readFile/writeFile/save
- Editor auto-opens its file when Monaco signals ready (editorReady msg)
- Explorer uses its own JS bridge (cmuxExplorer) with self-contained
  file system operations separate from the editor's cmuxEditor bridge

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Editor panel is now a minimal single-file Monaco viewer (no sidebar,
  no tabs, no folder watching)
- VS Code preview tab behavior: single-click reuses preview tab (italic
  title), double-click or editing pins it, pinned files open new tabs
- Pre-warm WKWebView pool (2 WebViews with Monaco loaded at startup) so
  editor tabs open instantly with zero flash
- Add isItalic property to Bonsplit Tab for italic tab title rendering
- Monaco loaded from CDN with language worker support for full syntax
  highlighting
- Explorer sidebar: double-click pins file, context menu rename moved
  to right-click only

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sible

- Explorer shows all unique panel directories within the current
  workspace as collapsible root folders (switches on workspace change)
- Replace JS polling with native FSEvents file watching (50ms coalesce)
  for near-instant tree updates on file changes
- Diff-based refresh: only patches git badges in-place unless directory
  structure actually changed (no DOM teardown flash)
- Show all dotfiles (.gitignore, .env, etc.) — only .git is hidden
- Gitignored files/folders greyed out (40% opacity) including children
  of ignored directories
- Folders never get strikethrough even with deleted children
- Folder git status priority matches VS Code: conflict > modified >
  deleted > added > untracked > renamed
- Large file guard: files over 1MB or 50k lines show italic filename
  with size/line count message instead of loading Monaco
- Added statFile bridge for size check before content read
- Single git status command (--porcelain -unormal --ignored) replaces
  separate status + ls-files calls

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace WebView explorer with native SwiftUI file tree (lazy loading,
  git status, expand/collapse, gitignore greying, FSEvents watching)
- Add file search view with async search across all workspace roots
- Tabbed sidebar: workspaces | explorer | search with icon selector
- Cmd+Shift+F opens sidebar search tab
- Incremental refresh: git badges update in-place, structure diffed
  before reload, expanded state preserved across refreshes
- Disable Monaco Cmd+Shift+F/P/N shortcuts to avoid conflicts
- Fix traffic light padding for tabbed sidebar layout

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Flatten file tree into single ForEach for instant expand/collapse
  (no recursive views, LazyVStack diffs by stable node ID)
- Synchronous file read in openFileByPath — zero dispatch hops, content
  injected directly into Monaco via JSON-encoded evaluateJavaScript
- Fix crash: JSONSerialization wrap-in-array for safe string encoding
- Fix terminal stealing sidebar search focus: applyFirstResponderIfNeeded
  now skips when an NSTextField outside the terminal is being edited
- NSTextField-based search field (SidebarSearchField) for proper AppKit
  first responder handling
- Remove duplicate SidebarTopScrim from VerticalTabsSidebar — parent
  sidebarView owns the scrim for all three tabs
- Always show root headers in explorer (no 1-vs-many branch switch)
- loadChildrenForNode synchronous (directory listing is <1ms)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WebView pool makes MonacoCache redundant (Monaco pre-loaded at startup).
Swift reads files directly — JS bridge readFile/statFile no longer used.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…lorer

# Conflicts:
#	Sources/KeyboardShortcutSettings.swift
#	vendor/bonsplit

@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: 16

♻️ Duplicate comments (3)
Sources/AppDelegate.swift (1)

9640-9644: ⚠️ Potential issue | 🟡 Minor

Handle editor-open failure instead of silently ignoring it.

Same issue as the earlier review: openEditor(rootPath:) can return nil, and the shortcut path still drops that failure on the floor. When panel creation fails, Cmd+Shift+E just looks dead.

💡 Minimal fix
 func openEditorForCurrentProject() {
-    guard let tabManager = tabManager,
-          let tabId = tabManager.selectedTabId,
-          let workspace = tabManager.tabs.first(where: { $0.id == tabId }) else { return }
+    guard let tabManager = tabManager,
+          let workspace = tabManager.selectedWorkspace else { return }
     let rootPath = workspace.currentDirectory
-    _ = tabManager.openEditor(rootPath: rootPath)
+    guard tabManager.openEditor(rootPath: rootPath) != nil else {
+        NSSound.beep()
+        return
+    }
 }

Also applies to: 9886-9892

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

In `@Sources/AppDelegate.swift` around lines 9640 - 9644, The Cmd+Shift+E handler
calls openEditorForCurrentProject() which eventually calls openEditor(rootPath:)
that can return nil, but the shortcut path currently ignores that failure so the
shortcut appears to do nothing; update the shortcut handling (the block using
matchShortcut(...) and calling openEditorForCurrentProject()) to check the
result of openEditor/openEditorForCurrentProject (or have
openEditorForCurrentProject propagate the optional) and handle a nil return by
logging the failure and presenting a user-facing alert or error panel (so users
see why the editor failed to open) rather than silently dropping the error;
ensure the same pattern is applied to the other occurrence referenced (the block
around lines 9886-9892).
Resources/editor/editor.js (1)

34-48: ⚠️ Potential issue | 🟠 Major

Cache the first theme update until Monaco is ready.

updateMonacoTheme() still returns before storing anything, so any host theme sent during startup is lost and the editor stays on fallback colors until a later theme-change event arrives.

🎨 Minimal fix
+    const pendingTheme = { editorBg: '#1f1f1f', editorFg: '#c6c6c6' };
     window.cmux = {
         handleResponse(requestId, data) {
             const p = pendingRequests.get(requestId);
             if (!p) return;
@@
         },
         updateMonacoTheme(editorBg, editorFg) {
-            if (!monacoInstance || !editor) return;
+            pendingTheme.editorBg = editorBg;
+            pendingTheme.editorFg = editorFg;
+            if (!monacoInstance || !editor) return;
             monacoInstance.editor.defineTheme('cmux-dark', {
                 base: 'vs-dark', inherit: true, rules: [],
                 colors: {
@@
             editor = monaco.editor.create(document.getElementById('editor-container'), {
                 theme: 'cmux-dark',
                 fontSize: 13,
@@
                 stickyScroll: { enabled: true }
             });
+
+            window.cmux.updateMonacoTheme(pendingTheme.editorBg, pendingTheme.editorFg);
 
             // Cmd+S — save
             editor.addCommand(monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyS, () => saveActive());

Also applies to: 163-218

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

In `@Resources/editor/editor.js` around lines 34 - 48, updateMonacoTheme currently
returns early if monacoInstance or editor is not ready, so initial host theme
messages are dropped; change it to save the first passed editorBg/editorFg into
a small cached object (e.g., pendingTheme) when monacoInstance or editor is
falsy, and return; then when Monaco is initialized (where monacoInstance and
editor are set) call a new applyPendingTheme() that runs the existing
theme-define / setTheme logic and sets the --editor-bg CSS var, and clear
pendingTheme; reference updateMonacoTheme, monacoInstance, editor and add
applyPendingTheme/pendingTheme so early theme updates are applied once Monaco
becomes ready.
Sources/MonacoWebViewPool.swift (1)

93-110: ⚠️ Potential issue | 🔴 Critical

The prewarmed editor still executes Monaco from a CDN.

injectMonacoPaths() points the privileged editor webview at jsDelivr. Any third-party code loaded there runs with access to the cmuxEditor file-system bridge, and the pool also stops working offline. Use bundled Monaco assets for vsPath and cssHref instead.

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

In `@Sources/MonacoWebViewPool.swift` around lines 93 - 110, injectMonacoPaths
currently builds CDN URLs (vsPath, cssHref) and calls window.cmux.initMonaco
which causes the prewarmed editor to load third‑party code and fail offline;
change it to use bundled local file URLs instead. Locate injectMonacoPaths and
replace construction of vsPath/cssHref so they point to the app bundle (e.g.
Bundle.main/Bundle.module resource URLs or file:// URLs for the monaco min/vs
directory and editor.main.css shipped in the app), ensure the strings passed
into window.cmux.initMonaco are properly escaped file URLs, and verify the
webView can load those local assets (using loadFileURL:allowingReadAccessToURL:
if necessary) so Monaco runs from bundled assets rather than jsDelivr.
🧹 Nitpick comments (4)
Sources/NativeFileExplorer.swift (2)

78-87: loadChildrenForNode performs synchronous file I/O on the main thread.

Unlike loadChildren() which dispatches to a background queue, loadChildrenForNode runs FileManager.contentsOfDirectory synchronously. For directories with many files, this could cause UI jank during expansion.

Proposed async pattern matching loadChildren()
 func loadChildrenForNode(_ node: FileNode) {
     let dirPath = (node.rootPath as NSString).appendingPathComponent(node.relativePath)
-    node.children = Self.loadEntries(
-        at: dirPath,
-        relativeTo: node.relativePath,
-        rootPath: node.rootPath,
-        gitStatusMap: gitStatusMap,
-        gitIgnoredPaths: gitIgnoredPaths
-    )
+    let statusMap = gitStatusMap
+    let ignored = gitIgnoredPaths
+    let relPath = node.relativePath
+    let rootPath = node.rootPath
+    
+    DispatchQueue.global(qos: .userInitiated).async {
+        let entries = Self.loadEntries(
+            at: dirPath,
+            relativeTo: relPath,
+            rootPath: rootPath,
+            gitStatusMap: statusMap,
+            gitIgnoredPaths: ignored
+        )
+        DispatchQueue.main.async { [weak node] in
+            node?.children = entries
+        }
+    }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/NativeFileExplorer.swift` around lines 78 - 87, loadChildrenForNode
currently performs synchronous I/O on the main thread (calling Self.loadEntries
/ FileManager.contentsOfDirectory) which can cause UI jank; change it to match
loadChildren() by dispatching the directory read to a background queue (e.g.,
DispatchQueue.global(qos:.userInitiated)) to call
Self.loadEntries(at:relativeTo:rootPath:gitStatusMap:gitIgnoredPaths:), then
assign node.children back on the main thread (DispatchQueue.main.async) to avoid
UI blocking; ensure any errors are handled similarly to loadChildren and keep
the same parameters and behavior.

167-215: Consider adding a timeout for the git process.

While git status --porcelain is local-only and typically fast, pathological repositories (huge indexes, filesystem issues) could cause indefinite hangs. A timeout would improve resilience.

Optional: Add process termination timeout
 func loadGitStatus() {
     DispatchQueue.global(qos: .userInitiated).async { [path] in
         let process = Process()
         let pipe = Pipe()
         process.executableURL = URL(fileURLWithPath: "/usr/bin/git")
         process.arguments = ["-C", path, "status", "--porcelain=v1", "-unormal", "--ignored"]
         process.standardOutput = pipe
         process.standardError = FileHandle.nullDevice

         guard (try? process.run()) != nil else { return }
+        
+        // Timeout after 10 seconds
+        DispatchQueue.global().asyncAfter(deadline: .now() + 10) {
+            if process.isRunning { process.terminate() }
+        }
+        
         let data = pipe.fileHandleForReading.readDataToEndOfFile()
         process.waitUntilExit()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/NativeFileExplorer.swift` around lines 167 - 215, The loadGitStatus
function can hang waiting for the git Process; add a timeout by starting the
Process as before but monitor it with a DispatchSemaphore/DispatchGroup or
DispatchWorkItem on a background queue and after a configured timeout (e.g. 5s)
call process.terminate() (or process.interrupt()) if it hasn’t exited, then read
whatever output is available and proceed; update the handler in loadGitStatus to
treat termination as failure (return early or set empty maps) and ensure you
close the pipe/file handles and call process.waitUntilExit() after termination
to avoid zombies. Ensure you reference and modify loadGitStatus, the local
process/pipe variables, and the DispatchQueue completion block so
gitStatusMap/gitIgnoredPaths are only set when the process completed or after a
clean timeout termination.
Sources/FileSearchView.swift (1)

15-21: Local variable shadows instance property query.

Line 16 declares let query = query.trimmingCharacters(...) which shadows self.query. This works but is confusing—consider renaming to trimmedQuery or searchQuery for clarity.

Suggested rename for clarity
 func search() {
-    let query = query.trimmingCharacters(in: .whitespacesAndNewlines)
-    guard !query.isEmpty else {
+    let trimmedQuery = query.trimmingCharacters(in: .whitespacesAndNewlines)
+    guard !trimmedQuery.isEmpty else {
         results = []
         isSearching = false
         return
     }
     searchTask?.cancel()
     isSearching = true
     let paths = rootPaths
-    let searchQuery = query.lowercased()
+    let searchQuery = trimmedQuery.lowercased()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/FileSearchView.swift` around lines 15 - 21, In the search() method
the local let query = query.trimmingCharacters(...) shadows the instance
property query; rename the local variable to something clear (e.g., trimmedQuery
or searchQuery) and update all references inside search() to use the new name so
you trim the instance property without shadowing it; ensure the guard uses the
renamed variable (e.g., guard !trimmedQuery.isEmpty) and any subsequent logic
that relied on the local name is adjusted accordingly.
Sources/Panels/EditorPanel.swift (1)

367-372: Incomplete JS string escaping — prefer JSON serialization uniformly.

jsEscape handles only \, ', \n, \r but misses \t, \0, and Unicode line/paragraph separators (U+2028, U+2029) which are valid in file names and would break the JS literal. The class already has jsStringLiteral in EditorMessageHandler using JSON serialization—consider reusing that pattern here for consistency and correctness.

Use JSON serialization for all JS string injection
-    private func jsEscape(_ s: String) -> String {
-        s.replacingOccurrences(of: "\\", with: "\\\\")
-         .replacingOccurrences(of: "'", with: "\\'")
-         .replacingOccurrences(of: "\n", with: "\\n")
-         .replacingOccurrences(of: "\r", with: "\\r")
-    }
+    /// Encode a Swift string as a safe JavaScript string literal using JSON serialization.
+    private func jsStringLiteral(_ value: String) -> String {
+        guard let data = try? JSONSerialization.data(withJSONObject: [value]),
+              let arrayStr = String(data: data, encoding: .utf8),
+              arrayStr.count > 2 else { return "\"\"" }
+        let start = arrayStr.index(after: arrayStr.startIndex)
+        let end = arrayStr.index(before: arrayStr.endIndex)
+        return String(arrayStr[start..<end])
+    }

Then update call sites to use double-quoted JS strings with the JSON-escaped values.

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

In `@Sources/Panels/EditorPanel.swift` around lines 367 - 372, The jsEscape(_:)
implementation in EditorPanel.swift is incomplete (it only escapes \, ', \n, \r)
and can break JS when filenames include \t, \0, or Unicode line/paragraph
separators; replace its logic by reusing the JSON-based escaping used in
EditorMessageHandler.jsStringLiteral (or implement equivalent
JSON.stringify-based serialization) so all injected JS strings are JSON-escaped,
then update call sites in EditorPanel.swift to emit double-quoted JS string
literals using that JSON-escaped value instead of the old jsEscape output.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Resources/editor/editor.js`:
- Around line 59-65: When replacing the current buffer in
showLargeFile(fileName, reason) (and similarly in doOpenFileWithContent),
explicitly clear the native "dirty" flag and dispose the old Monaco model:
before detaching the model call model.dispose() (if editor.getModel() exists)
then editor.setModel(null), and send a message to Swift setting dirtyState =
false (use the existing notify/IPC helper used elsewhere). Ensure both
showLargeFile and doOpenFileWithContent perform the dispose + setModel(null)
sequence and invoke the same notify to Swift to clear dirtyState.
- Around line 152-160: The saveActive function currently swallows writeFile
failures in the catch block; change it to propagate the error to the host and
surface an inline error instead of just console.error. Replace the empty catch
in saveActive (around the await writeFile(currentFilePath, content)) so that on
failure you do not swallow the error — either rethrow the caught err (so the
caller/Swift can handle it) or call the platform bridge to report the failure
(e.g., window.webkit.messageHandlers.saveFailed.postMessage(err) or your app's
equivalent) and display an inline error UI, while leaving
originalContent/isDirty unchanged and not calling notifyDirty(false). Ensure the
error path still preserves the buffer dirty state and lets the host show the
failure.

In `@Resources/explorer/explorer.css`:
- Around line 68-83: The CSS uses quoted single-word font names and lacks a
generic fallback; update the font-family declarations for the codicon usage
(e.g., in the .header-btn rule and the two other codicon occurrences) by
removing the quotes and adding a generic fallback so they read like:
font-family: codicon, sans-serif; ensuring all three instances are changed.
- Line 2: Replace the runtime jsDelivr import for Codicons in explorer.css (the
`@import`
url('https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css')
line) with a local reference to a bundled codicon CSS (e.g., `@import`
'./codicon.css' or a relative path) and add the Codicons CSS and font files into
the Resources/explorer/ package; ensure the bundled codicon.css uses relative
font URLs so the fonts are resolved locally and update your build/copy steps to
include those font assets so the explorer can render icons offline.

In `@Resources/explorer/explorer.js`:
- Around line 57-60: The refresh path can interrupt inline rename/create because
refresh(), fullRefresh(), and diffRefresh() don't check the inlineInputActive
flag; update each of these functions (refresh, fullRefresh, diffRefresh) to
early-return when inlineInputActive is true so background FSEvents or
diff-driven rerenders won't remove or blur the inline input element mid-edit,
ensuring the inline input state is preserved during active edits.
- Around line 183-310: The tree rows created in renderTree are currently plain
divs and only respond to mouse events, preventing keyboard-only users from
navigating and invoking actions; make each row focusable (set tabindex="0"), add
appropriate ARIA attributes (role="treeitem", aria-expanded for directories,
aria-selected when dk === selectedTreePath) and ensure label elements are
accessible, then attach a keydown handler on the same element to map keys to
existing handlers: Enter/Space should call openFileExternal (files) or toggle
expansion (directories) and call handleSelection(dk,e) as needed; F2 should
trigger rename flow (use the same flow as the context menu rename command
invoked by showContextMenu), Delete should invoke delete behavior,
ArrowRight/ArrowLeft should expand/collapse directories by updating expandedDirs
and calling renderTree/children class toggles (same code paths used in the row
click handler), and ArrowUp/ArrowDown should move focus to the previous/next
.tree-row elements and update selection via handleSelection; update references
to selectedTreePath, expandedDirs, handleSelection, openFileExternal,
pinFileExternal, showContextMenu, and renderTree to reuse existing logic rather
than duplicating behavior.

In `@Sources/AppDelegate.swift`:
- Around line 2245-2246: Skip calling MonacoWebViewPool.shared.warmUp() on the
synchronous launch path: guard against running during tests (e.g. detect XCTest
via ProcessInfo.processInfo.environment["XCTestConfigurationFilePath"] or
presence of XCTest classes) and instead schedule
MonacoWebViewPool.shared.warmUp() to run asynchronously after launch (dispatch
to the main queue or schedule a short Task/MainActor.run tick) from the
AppDelegate launch completion path (where MonacoWebViewPool.shared.warmUp() is
currently invoked) so the pre-warm does not block startup or run during XCTest.
- Around line 9634-9637: The notification for file search currently posts
.cmuxSidebarSwitchToSearch with object: nil, dropping the window context
resolved by handleCustomShortcut(event:), which breaks multi-window routing;
update the post call in the matchShortcut branch (the block using
matchShortcut(event: event, shortcut: StoredShortcut(...))) to include the
resolved window (e.g., pass the local window/routedWindow variable as the object
or include it in userInfo) so receivers can scope the action to that specific
NSWindow instead of broadcasting globally.

In `@Sources/ContentView.swift`:
- Around line 2332-2357: The titlebar affordances (WindowDragHandleView and
HiddenTitlebarSidebarControlsView) must be moved out of VerticalTabsSidebar and
placed into the new top spacer area in sidebarView so they are active for all
tabs and render within the 28pt titlebar band; update sidebarView (near
sidebarTrafficLightPadding and SidebarTabSelector) to include
WindowDragHandleView and HiddenTitlebarSidebarControlsView above or inside the
Spacer that uses sidebarTrafficLightPadding, and remove or stop mounting those
views in VerticalTabsSidebar (and adjust VerticalTabsSidebar props if they no
longer need to manage them).
- Around line 2391-2409: The current panel.onOpenFile closure reimplements
editor-open logic and calls tabManager.openEditor directly, which bypasses the
shared text-editor routing; change this closure to funnel through the shared
openFileInTextEditor(_:) path instead: keep the preview reuse detection (check
EditorPanel.isPreview and call preview.openFileByPath + workspace.focusPanel)
but when creating a new editor, call openFileInTextEditor(filePath) rather than
tabManager.openEditor; implement openFileInTextEditor(_:) so it first attempts
workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that
returns nil, falls back to
workspace.newTextEditorSurface(inPane:filePath:focus:) using
bonsplitController.focusedPaneId, ensuring the same change is applied to the
other similar closure at the noted lines.
- Around line 2365-2368: The notification handler in ContentView currently
reacts to .cmuxSidebarSwitchToSearch globally; change the
.onReceive(NotificationCenter.default.publisher(for:
.cmuxSidebarSwitchToSearch)) closure to first verify the notification is
targeted at this view's owning window (the same gating used for the
command-palette notifications) by inspecting notification.object or the window
identifier in notification.userInfo and comparing it to this view's window, and
only then set sidebarTab = .search and toggle sidebarState.isVisible — this
ensures only the window that owns the notification mutates its local
sidebarTab/sidebarState.

In `@Sources/NativeFileExplorer.swift`:
- Around line 268-290: The FSEvents callback stores an unretained pointer to
self which can crash if FileTreeRoot is deallocated; fix startFSEvents by
avoiding passUnretained: either (A) use Unmanaged.passRetained(self) when
assigning context.info and call Unmanaged.fromOpaque(...).release() in
FileTreeRoot.deinit (and ensure fsEventStream is stopped/invalidate in deinit),
or (B) store a small heap-allocated weak-wrapper object (e.g., class WeakBox {
weak var root: FileTreeRoot? }) in context.info and in the callback convert the
opaque pointer to the WeakBox and safely bail if root is nil before calling
root.debouncedRefresh(); update fsEventStream lifecycle code to stop and
invalidate the stream in deinit to avoid callbacks after deallocation.

In `@Sources/Panels/ExplorerSidebarPanel.swift`:
- Around line 26-39: The pinFileExternal and openFileExternal handlers build
fullPath without validating path traversal; update the handlers that call
onPinFile and onOpenFile to resolve and validate the combined path before
invoking callbacks—use rootPath(for:) and then call the same resolvedPath()
validation used elsewhere (or a shared validate/resolved helper) to ensure the
final path is within the root; only call self?.onPinFile?(fullPath) or
self?.onOpenFile?(fullPath) after resolvedPath confirms the path is canonical
and inside the root, and early-return/log if validation fails.
- Around line 424-431: The manual JSON string building in sendRootsToJS fails to
escape backslashes and other characters; instead build an array of dictionaries
(e.g., using rootPaths.enumerated() to produce [{"name": name, "rootIndex":
index}, ...]) and serialize it with JSONSerialization or JSONEncoder to produce
a safe JSON string, then inject that serialized string into the js string passed
to webView.evaluateJavaScript (keep using window.cmuxExplorer.setRoots but
substitute the serialized JSON), ensuring you handle serialization errors and
avoid manual escaping of name.
- Around line 338-368: The FSEvents callback uses Unmanaged.passUnretained(self)
which can cause a use-after-free if deinit runs while callbacks are queued;
update startFSEvents/stopFSEvents to ensure the panel is retained while
callbacks may run: when creating the context in startFSEvents use
Unmanaged.passRetained(self) (so the callback can safely call
Unmanaged<ExplorerSidebarPanel>.fromOpaque(info).takeRetainedValue()), and then
in stopFSEvents synchronously stop/unschedule/invalidate the FSEventStream on
the main run loop and explicitly release the retained reference with
Unmanaged.release(...) (or call takeRetainedValue() once and let it deinit) so
the retain is balanced and no in-flight callback can touch freed memory; ensure
deinit calls stopFSEvents on the main thread to avoid races with
FSEventStreamStart/FSEventStreamSchedule.

In `@Sources/SidebarTabSelector.swift`:
- Around line 21-37: The icon-only tab buttons created in ForEach(tabs, id: \.0)
{ tab, icon in ... } need localized accessible names and a selected trait so
VoiceOver can identify each tab and which is active; update the Button (not just
the Image) to call .accessibilityLabel(String(localized: "sidebar.<tabKey>",
defaultValue: "<English name>")) and .help(String(localized:
"sidebar.<tabKey>.help", defaultValue: "<English name>")) (one key per tab like
"workspaces", "explorer", "search"), and when selected == tab also add
.accessibilityAddTraits(.isSelected) (or remove that trait when not selected) so
the selected state is announced. Ensure all user-facing strings use
String(localized:..., defaultValue:...) per guidelines and attach these
modifiers to the Button that sets selected.

---

Duplicate comments:
In `@Resources/editor/editor.js`:
- Around line 34-48: updateMonacoTheme currently returns early if monacoInstance
or editor is not ready, so initial host theme messages are dropped; change it to
save the first passed editorBg/editorFg into a small cached object (e.g.,
pendingTheme) when monacoInstance or editor is falsy, and return; then when
Monaco is initialized (where monacoInstance and editor are set) call a new
applyPendingTheme() that runs the existing theme-define / setTheme logic and
sets the --editor-bg CSS var, and clear pendingTheme; reference
updateMonacoTheme, monacoInstance, editor and add applyPendingTheme/pendingTheme
so early theme updates are applied once Monaco becomes ready.

In `@Sources/AppDelegate.swift`:
- Around line 9640-9644: The Cmd+Shift+E handler calls
openEditorForCurrentProject() which eventually calls openEditor(rootPath:) that
can return nil, but the shortcut path currently ignores that failure so the
shortcut appears to do nothing; update the shortcut handling (the block using
matchShortcut(...) and calling openEditorForCurrentProject()) to check the
result of openEditor/openEditorForCurrentProject (or have
openEditorForCurrentProject propagate the optional) and handle a nil return by
logging the failure and presenting a user-facing alert or error panel (so users
see why the editor failed to open) rather than silently dropping the error;
ensure the same pattern is applied to the other occurrence referenced (the block
around lines 9886-9892).

In `@Sources/MonacoWebViewPool.swift`:
- Around line 93-110: injectMonacoPaths currently builds CDN URLs (vsPath,
cssHref) and calls window.cmux.initMonaco which causes the prewarmed editor to
load third‑party code and fail offline; change it to use bundled local file URLs
instead. Locate injectMonacoPaths and replace construction of vsPath/cssHref so
they point to the app bundle (e.g. Bundle.main/Bundle.module resource URLs or
file:// URLs for the monaco min/vs directory and editor.main.css shipped in the
app), ensure the strings passed into window.cmux.initMonaco are properly escaped
file URLs, and verify the webView can load those local assets (using
loadFileURL:allowingReadAccessToURL: if necessary) so Monaco runs from bundled
assets rather than jsDelivr.

---

Nitpick comments:
In `@Sources/FileSearchView.swift`:
- Around line 15-21: In the search() method the local let query =
query.trimmingCharacters(...) shadows the instance property query; rename the
local variable to something clear (e.g., trimmedQuery or searchQuery) and update
all references inside search() to use the new name so you trim the instance
property without shadowing it; ensure the guard uses the renamed variable (e.g.,
guard !trimmedQuery.isEmpty) and any subsequent logic that relied on the local
name is adjusted accordingly.

In `@Sources/NativeFileExplorer.swift`:
- Around line 78-87: loadChildrenForNode currently performs synchronous I/O on
the main thread (calling Self.loadEntries / FileManager.contentsOfDirectory)
which can cause UI jank; change it to match loadChildren() by dispatching the
directory read to a background queue (e.g.,
DispatchQueue.global(qos:.userInitiated)) to call
Self.loadEntries(at:relativeTo:rootPath:gitStatusMap:gitIgnoredPaths:), then
assign node.children back on the main thread (DispatchQueue.main.async) to avoid
UI blocking; ensure any errors are handled similarly to loadChildren and keep
the same parameters and behavior.
- Around line 167-215: The loadGitStatus function can hang waiting for the git
Process; add a timeout by starting the Process as before but monitor it with a
DispatchSemaphore/DispatchGroup or DispatchWorkItem on a background queue and
after a configured timeout (e.g. 5s) call process.terminate() (or
process.interrupt()) if it hasn’t exited, then read whatever output is available
and proceed; update the handler in loadGitStatus to treat termination as failure
(return early or set empty maps) and ensure you close the pipe/file handles and
call process.waitUntilExit() after termination to avoid zombies. Ensure you
reference and modify loadGitStatus, the local process/pipe variables, and the
DispatchQueue completion block so gitStatusMap/gitIgnoredPaths are only set when
the process completed or after a clean timeout termination.

In `@Sources/Panels/EditorPanel.swift`:
- Around line 367-372: The jsEscape(_:) implementation in EditorPanel.swift is
incomplete (it only escapes \, ', \n, \r) and can break JS when filenames
include \t, \0, or Unicode line/paragraph separators; replace its logic by
reusing the JSON-based escaping used in EditorMessageHandler.jsStringLiteral (or
implement equivalent JSON.stringify-based serialization) so all injected JS
strings are JSON-escaped, then update call sites in EditorPanel.swift to emit
double-quoted JS string literals using that JSON-escaped value instead of the
old jsEscape output.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 40fdfa48-ffa8-49a1-8213-4546658b5741

📥 Commits

Reviewing files that changed from the base of the PR and between 5055f5e and 4701a85.

📒 Files selected for processing (23)
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Resources/editor/editor.js
  • Resources/editor/index.html
  • Resources/explorer/explorer.css
  • Resources/explorer/explorer.js
  • Resources/explorer/index.html
  • Sources/AppDelegate.swift
  • Sources/ContentView.swift
  • Sources/FileSearchView.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/KeyboardShortcutSettings.swift
  • Sources/MonacoWebViewPool.swift
  • Sources/NativeFileExplorer.swift
  • Sources/Panels/EditorPanel.swift
  • Sources/Panels/ExplorerSidebarPanel.swift
  • Sources/Panels/ExplorerSidebarView.swift
  • Sources/Panels/Panel.swift
  • Sources/Panels/PanelContentView.swift
  • Sources/SessionPersistence.swift
  • Sources/SidebarTabSelector.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
  • Sources/Workspace.swift
✅ Files skipped from review due to trivial changes (1)
  • Resources/explorer/index.html
🚧 Files skipped from review as they are similar to previous changes (3)
  • Resources/editor/index.html
  • Sources/Workspace.swift
  • Sources/TerminalController.swift

Comment on lines +59 to +65
// Called from Swift when file is too large
showLargeFile(fileName, reason) {
currentFilePath = null;
if (editor) editor.setModel(null);
showLargeFileNotice(fileName, reason);
notifyActive(fileName);
},

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

Clear native dirty state whenever you replace the current buffer.

Both showLargeFile() and doOpenFileWithContent() reset local state, but neither sends dirtyState = false back to Swift. If the previous file was dirty, the panel can stay marked dirty after discard/open, and showLargeFile() also leaves the old Monaco model undisposed.

🧽 Minimal fix
         showLargeFile(fileName, reason) {
             currentFilePath = null;
-            if (editor) editor.setModel(null);
+            const oldModel = editor ? editor.getModel() : null;
+            if (editor) editor.setModel(null);
+            if (oldModel) oldModel.dispose();
+            originalContent = '';
+            isDirty = false;
+            notifyDirty(false);
             showLargeFileNotice(fileName, reason);
             notifyActive(fileName);
         },
@@
     function doOpenFileWithContent(relativePath, fileName, content) {
         hideLargeFileNotice();
         currentFilePath = relativePath;
         originalContent = content;
         isDirty = false;
+        notifyDirty(false);
         const lang = getLang(fileName);
         const model = monacoInstance.editor.createModel(content, lang);

Also applies to: 95-113

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

In `@Resources/editor/editor.js` around lines 59 - 65, When replacing the current
buffer in showLargeFile(fileName, reason) (and similarly in
doOpenFileWithContent), explicitly clear the native "dirty" flag and dispose the
old Monaco model: before detaching the model call model.dispose() (if
editor.getModel() exists) then editor.setModel(null), and send a message to
Swift setting dirtyState = false (use the existing notify/IPC helper used
elsewhere). Ensure both showLargeFile and doOpenFileWithContent perform the
dispose + setModel(null) sequence and invoke the same notify to Swift to clear
dirtyState.

Comment on lines +152 to +160
async function saveActive() {
if (!currentFilePath || !editor) return;
const content = editor.getModel().getValue();
try {
await writeFile(currentFilePath, content);
originalContent = content;
isDirty = false;
notifyDirty(false);
} catch (err) { console.error('Save failed:', err); }

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

Don't hide failed saves in the web console.

A rejected writeFile() only hits console.error, which is invisible in normal app usage. Cmd+S can fail silently here; bubble the error back to Swift or show an inline error while leaving the buffer dirty.

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

In `@Resources/editor/editor.js` around lines 152 - 160, The saveActive function
currently swallows writeFile failures in the catch block; change it to propagate
the error to the host and surface an inline error instead of just console.error.
Replace the empty catch in saveActive (around the await
writeFile(currentFilePath, content)) so that on failure you do not swallow the
error — either rethrow the caught err (so the caller/Swift can handle it) or
call the platform bridge to report the failure (e.g.,
window.webkit.messageHandlers.saveFailed.postMessage(err) or your app's
equivalent) and display an inline error UI, while leaving
originalContent/isDirty unchanged and not calling notifyDirty(false). Ensure the
error path still preserves the buffer dirty state and lets the host show the
failure.

@@ -0,0 +1,211 @@
/* cmux Sidebar Explorer — file tree only */
@import url('https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css');

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

🧩 Analysis chain

🏁 Script executed:

find . -name "explorer.css" -type f

Repository: manaflow-ai/cmux

Length of output: 93


🏁 Script executed:

find . -name "*codicon*" -type f

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

find . -name ".stylelintrc*" -o -name "stylelint.config.*" | head -20

Repository: manaflow-ai/cmux

Length of output: 79


🏁 Script executed:

wc -l Resources/explorer/explorer.css

Repository: manaflow-ai/cmux

Length of output: 95


🏁 Script executed:

cat -n Resources/explorer/explorer.css

Repository: manaflow-ai/cmux

Length of output: 9004


🏁 Script executed:

cat .stylelintrc.json

Repository: manaflow-ai/cmux

Length of output: 1066


🏁 Script executed:

find . -name "package.json" -type f | head -5

Repository: manaflow-ai/cmux

Length of output: 328


🏁 Script executed:

find . -name "webpack.config.*" -o -name "vite.config.*" -o -name "rollup.config.*" | head -10

Repository: manaflow-ai/cmux

Length of output: 109


🏁 Script executed:

find . -path ./node_modules -prune -o -type f -name ".github" -o -name "*.yml" -o -name "*.yaml" | grep -E "(github|\.github|\.gitignore)" | head -20

Repository: manaflow-ai/cmux

Length of output: 491


🏁 Script executed:

cat package.json | head -50

Repository: manaflow-ai/cmux

Length of output: 145


🏁 Script executed:

ls -la Resources/explorer/

Repository: manaflow-ai/cmux

Length of output: 359


🏁 Script executed:

grep -r "codicon" . --include="*.js" --include="*.ts" --include="*.json" | grep -v node_modules | head -20

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

grep -A 20 "stylelint" .github/workflows/ci.yml

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

head -100 Resources/explorer/explorer.js | grep -i "codicon\|font-family\|icon"

Repository: manaflow-ai/cmux

Length of output: 754


🏁 Script executed:

cat .github/workflows/ci.yml | grep -A 30 "style\|lint" || echo "No stylelint in CI found"

Repository: manaflow-ai/cmux

Length of output: 84


🏁 Script executed:

rg "import.*url" Resources/ --type css

Repository: manaflow-ai/cmux

Length of output: 177


Ship Codicons locally instead of fetching from jsDelivr at runtime.

The explorer depends on a network fetch for core icons, so offline or fresh launches render blank toolbar and tree icons. Replace the @import url() with a local Codicon CSS/font file in Resources/explorer/.

🧰 Tools
🪛 Stylelint (17.4.0)

[error] 2-2: Expected "url('https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css')" to be "'https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css'" (import-notation)

(import-notation)

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

In `@Resources/explorer/explorer.css` at line 2, Replace the runtime jsDelivr
import for Codicons in explorer.css (the `@import`
url('https://cdn.jsdelivr.net/npm/@vscode/codicons@0.0.36/dist/codicon.css')
line) with a local reference to a bundled codicon CSS (e.g., `@import`
'./codicon.css' or a relative path) and add the Codicons CSS and font files into
the Resources/explorer/ package; ensure the bundled codicon.css uses relative
font URLs so the fonts are resolved locally and update your build/copy steps to
include those font assets so the explorer can render icons offline.

Comment on lines +68 to +83
.header-btn {
background: none;
border: none;
color: var(--sidebar-fg);
opacity: 0.7;
cursor: pointer;
font-family: 'codicon';
font-size: 16px;
width: 22px;
height: 22px;
display: flex;
align-items: center;
justify-content: center;
border-radius: 3px;
line-height: 1;
}

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:

find . -name "explorer.css" -type f

Repository: manaflow-ai/cmux

Length of output: 93


🏁 Script executed:

cat -n ./Resources/explorer/explorer.css | sed -n '60,140p'

Repository: manaflow-ai/cmux

Length of output: 3046


🏁 Script executed:

find . -name ".stylelintrc*" -o -name "stylelint.config.*" | head -20

Repository: manaflow-ai/cmux

Length of output: 79


🏁 Script executed:

cat ./.stylelintrc.json

Repository: manaflow-ai/cmux

Length of output: 1066


🌐 Web query:

stylelint-config-standard font-family-name-quotes font-family-no-missing-generic-family-keyword rules

💡 Result:

In stylelint-config-standard (latest v40.0.0 as of 2026), the relevant font-family rules are: - "font-family-name-quotes": "always-where-recommended" — Requires quotes around font family names only when recommended by CSS spec (e.g., names with whitespace, digits, or certain punctuation, but not keywords like "sans-serif" or vendor-prefixed system fonts). - "font-family-no-missing-generic-family-keyword": Not explicitly set in stylelint-config-standard's index.js, so it inherits the default from stylelint-config-recommended. stylelint-config-recommended focuses on error-avoidance rules and does not include this convention-enforcement rule (confirmed by absence in changelogs, issues, and rule lists). This rule is available in core Stylelint but must be explicitly enabled if desired. To use: Extend "stylelint-config-standard" in your .stylelintrc and override/add rules as needed, e.g.: { "extends": "stylelint-config-standard", "rules": { "font-family-no-missing-generic-family-keyword": true } }

Citations:


Remove quotes and add generic fallback to codicon font-family declarations.

The font-family-name-quotes rule in stylelint-config-standard flags 'codicon' because single-word font names don't need quotes. Update all three occurrences:

Lint fix
-    font-family: 'codicon';
+    font-family: codicon, sans-serif;

Apply at lines 74, 121, and 132.

Adding a generic fallback (sans-serif) provides a reliable browser default if the codicon font fails to load.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.header-btn {
background: none;
border: none;
color: var(--sidebar-fg);
opacity: 0.7;
cursor: pointer;
font-family: 'codicon';
font-size: 16px;
width: 22px;
height: 22px;
display: flex;
align-items: center;
justify-content: center;
border-radius: 3px;
line-height: 1;
}
.header-btn {
background: none;
border: none;
color: var(--sidebar-fg);
opacity: 0.7;
cursor: pointer;
font-family: codicon, sans-serif;
font-size: 16px;
width: 22px;
height: 22px;
display: flex;
align-items: center;
justify-content: center;
border-radius: 3px;
line-height: 1;
}
🧰 Tools
🪛 Stylelint (17.4.0)

[error] 74-74: Unexpected quotes around "codicon" (font-family-name-quotes)

(font-family-name-quotes)


[error] 74-74: Unexpected missing generic font family (font-family-no-missing-generic-family-keyword)

(font-family-no-missing-generic-family-keyword)

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

In `@Resources/explorer/explorer.css` around lines 68 - 83, The CSS uses quoted
single-word font names and lacks a generic fallback; update the font-family
declarations for the codicon usage (e.g., in the .header-btn rule and the two
other codicon occurrences) by removing the quotes and adding a generic fallback
so they read like: font-family: codicon, sans-serif; ensuring all three
instances are changed.

Comment on lines +57 to +60
// Called from Swift on FSEvents file change — diffed, no flash
refresh() {
if (roots.length > 0) diffRefresh();
}

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

Pause tree refreshes while inline rename/create is active.

inlineInputActive is set by both edit flows, but refresh(), fullRefresh(), and diffRefresh() never consult it. A background rerender can remove the input row mid-edit or trigger a blur commit with a half-typed name.

🛑 Minimal guard
         refresh() {
-            if (roots.length > 0) diffRefresh();
+            if (!inlineInputActive && roots.length > 0) diffRefresh();
         }
@@
     async function fullRefresh() {
+        if (inlineInputActive) return;
         for (const root of roots) {
             await refreshGitStatus(root.rootIndex);
         }
@@
     async function diffRefresh() {
+        if (inlineInputActive) return;
         // Snapshot current expanded dirs' entry names before refresh
         const oldSnapshots = new Map();

Also applies to: 590-659

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

In `@Resources/explorer/explorer.js` around lines 57 - 60, The refresh path can
interrupt inline rename/create because refresh(), fullRefresh(), and
diffRefresh() don't check the inlineInputActive flag; update each of these
functions (refresh, fullRefresh, diffRefresh) to early-return when
inlineInputActive is true so background FSEvents or diff-driven rerenders won't
remove or blur the inline input element mid-edit, ensuring the inline input
state is preserved during active edits.

Comment on lines +268 to +290
private func startFSEvents() {
let paths = [path] as CFArray
var context = FSEventStreamContext()
context.info = Unmanaged.passUnretained(self).toOpaque()

let callback: FSEventStreamCallback = { _, info, _, _, _, _ in
guard let info else { return }
let root = Unmanaged<FileTreeRoot>.fromOpaque(info).takeUnretainedValue()
DispatchQueue.main.async {
root.debouncedRefresh()
}
}

guard let stream = FSEventStreamCreate(
nil, callback, &context, paths,
FSEventStreamEventId(kFSEventStreamEventIdSinceNow),
1.0, UInt32(kFSEventStreamCreateFlagUseCFTypes)
) else { return }

FSEventStreamScheduleWithRunLoop(stream, CFRunLoopGetMain(), CFRunLoopMode.defaultMode.rawValue)
FSEventStreamStart(stream)
fsEventStream = stream
}

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

FSEvents callback retains an unretained reference—risk of crash if FileTreeRoot deallocates while a callback is pending.

Unmanaged.passUnretained(self) stores a raw pointer in the FSEvents context. If the FileTreeRoot is deallocated before a queued callback fires, the fromOpaque dereference will crash.

Consider using passRetained and releasing in deinit, or checking validity via a weak reference wrapper.

Proposed fix using weak reference wrapper
+private final class WeakFileTreeRoot {
+    weak var value: FileTreeRoot?
+    init(_ value: FileTreeRoot) { self.value = value }
+}

 private func startFSEvents() {
     let paths = [path] as CFArray
     var context = FSEventStreamContext()
-    context.info = Unmanaged.passUnretained(self).toOpaque()
+    let weak = WeakFileTreeRoot(self)
+    let unmanagedWeak = Unmanaged.passRetained(weak)
+    context.info = unmanagedWeak.toOpaque()

     let callback: FSEventStreamCallback = { _, info, _, _, _, _ in
         guard let info else { return }
-        let root = Unmanaged<FileTreeRoot>.fromOpaque(info).takeUnretainedValue()
+        let weak = Unmanaged<WeakFileTreeRoot>.fromOpaque(info).takeUnretainedValue()
+        guard let root = weak.value else { return }
         DispatchQueue.main.async {
             root.debouncedRefresh()
         }
     }

Also release in deinit:

 deinit {
     if let stream = fsEventStream {
         FSEventStreamStop(stream)
         FSEventStreamInvalidate(stream)
+        // Release the retained weak wrapper
         FSEventStreamRelease(stream)
     }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private func startFSEvents() {
let paths = [path] as CFArray
var context = FSEventStreamContext()
context.info = Unmanaged.passUnretained(self).toOpaque()
let callback: FSEventStreamCallback = { _, info, _, _, _, _ in
guard let info else { return }
let root = Unmanaged<FileTreeRoot>.fromOpaque(info).takeUnretainedValue()
DispatchQueue.main.async {
root.debouncedRefresh()
}
}
guard let stream = FSEventStreamCreate(
nil, callback, &context, paths,
FSEventStreamEventId(kFSEventStreamEventIdSinceNow),
1.0, UInt32(kFSEventStreamCreateFlagUseCFTypes)
) else { return }
FSEventStreamScheduleWithRunLoop(stream, CFRunLoopGetMain(), CFRunLoopMode.defaultMode.rawValue)
FSEventStreamStart(stream)
fsEventStream = stream
}
private final class WeakFileTreeRoot {
weak var value: FileTreeRoot?
init(_ value: FileTreeRoot) { self.value = value }
}
private func startFSEvents() {
let paths = [path] as CFArray
var context = FSEventStreamContext()
let weak = WeakFileTreeRoot(self)
let unmanagedWeak = Unmanaged.passRetained(weak)
context.info = unmanagedWeak.toOpaque()
let callback: FSEventStreamCallback = { _, info, _, _, _, _ in
guard let info else { return }
let weak = Unmanaged<WeakFileTreeRoot>.fromOpaque(info).takeUnretainedValue()
guard let root = weak.value else { return }
DispatchQueue.main.async {
root.debouncedRefresh()
}
}
guard let stream = FSEventStreamCreate(
nil, callback, &context, paths,
FSEventStreamEventId(kFSEventStreamEventIdSinceNow),
1.0, UInt32(kFSEventStreamCreateFlagUseCFTypes)
) else { return }
FSEventStreamScheduleWithRunLoop(stream, CFRunLoopGetMain(), CFRunLoopMode.defaultMode.rawValue)
FSEventStreamStart(stream)
fsEventStream = stream
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/NativeFileExplorer.swift` around lines 268 - 290, The FSEvents
callback stores an unretained pointer to self which can crash if FileTreeRoot is
deallocated; fix startFSEvents by avoiding passUnretained: either (A) use
Unmanaged.passRetained(self) when assigning context.info and call
Unmanaged.fromOpaque(...).release() in FileTreeRoot.deinit (and ensure
fsEventStream is stopped/invalidate in deinit), or (B) store a small
heap-allocated weak-wrapper object (e.g., class WeakBox { weak var root:
FileTreeRoot? }) in context.info and in the callback convert the opaque pointer
to the WeakBox and safely bail if root is nil before calling
root.debouncedRefresh(); update fsEventStream lifecycle code to stop and
invalidate the stream in deinit to avoid callbacks after deallocation.

Comment on lines +26 to +39
case "pinFileExternal":
guard let relativePath = body["path"] as? String,
let root = rootPath(for: body) else { return }
let fullPath = (root as NSString).appendingPathComponent(relativePath)
DispatchQueue.main.async { [weak self] in
self?.onPinFile?(fullPath)
}
case "openFileExternal":
guard let relativePath = body["path"] as? String,
let root = rootPath(for: body) else { return }
let fullPath = (root as NSString).appendingPathComponent(relativePath)
DispatchQueue.main.async { [weak self] in
self?.onOpenFile?(fullPath)
}

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

Path traversal not validated for openFileExternal/pinFileExternal callbacks.

Unlike readDir, createFile, etc., these actions construct fullPath via simple appendingPathComponent without calling resolvedPath() to validate against traversal. A malicious JS payload could send path: "../../etc/passwd" and the callback would receive an out-of-root path.

Suggested fix: validate paths before invoking callbacks
 case "pinFileExternal":
     guard let relativePath = body["path"] as? String,
           let root = rootPath(for: body) else { return }
-    let fullPath = (root as NSString).appendingPathComponent(relativePath)
+    guard let fullPath = resolvedPath(relativePath, rootPath: root) else { return }
     DispatchQueue.main.async { [weak self] in
         self?.onPinFile?(fullPath)
     }
 case "openFileExternal":
     guard let relativePath = body["path"] as? String,
           let root = rootPath(for: body) else { return }
-    let fullPath = (root as NSString).appendingPathComponent(relativePath)
+    guard let fullPath = resolvedPath(relativePath, rootPath: root) else { return }
     DispatchQueue.main.async { [weak self] in
         self?.onOpenFile?(fullPath)
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/ExplorerSidebarPanel.swift` around lines 26 - 39, The
pinFileExternal and openFileExternal handlers build fullPath without validating
path traversal; update the handlers that call onPinFile and onOpenFile to
resolve and validate the combined path before invoking callbacks—use
rootPath(for:) and then call the same resolvedPath() validation used elsewhere
(or a shared validate/resolved helper) to ensure the final path is within the
root; only call self?.onPinFile?(fullPath) or self?.onOpenFile?(fullPath) after
resolvedPath confirms the path is canonical and inside the root, and
early-return/log if validation fails.

Comment on lines +338 to +368
private func startFSEvents() {
stopFSEvents()
guard !rootPaths.isEmpty else { return }

let paths = rootPaths as CFArray
var context = FSEventStreamContext()
// Use Unmanaged to pass self as a pointer
context.info = Unmanaged.passUnretained(self).toOpaque()

let callback: FSEventStreamCallback = { _, info, numEvents, eventPaths, _, _ in
guard let info else { return }
let panel = Unmanaged<ExplorerSidebarPanel>.fromOpaque(info).takeUnretainedValue()
DispatchQueue.main.async {
panel.handleFSEvent()
}
}

guard let stream = FSEventStreamCreate(
nil,
callback,
&context,
paths,
FSEventStreamEventId(kFSEventStreamEventIdSinceNow),
0.05, // 50ms latency
UInt32(kFSEventStreamCreateFlagUseCFTypes)
) else { return }

FSEventStreamScheduleWithRunLoop(stream, CFRunLoopGetMain(), CFRunLoopMode.defaultMode.rawValue)
FSEventStreamStart(stream)
fsEventStream = stream
}

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

Potential use-after-free race in FSEvents callback.

Unmanaged.passUnretained(self) doesn't prevent deallocation during callback execution. If deinit is invoked while an FSEvents callback is queued or executing, the takeUnretainedValue() on Line 349 could access freed memory. The 50ms latency reduces but doesn't eliminate this window.

Suggested fix: stop stream synchronously before release

The current deinit already stops the stream, but the callback could still be in-flight. Consider stopping the stream on the main run loop synchronously to ensure no callbacks are pending:

 deinit {
     if let observer = themeObserver {
         NotificationCenter.default.removeObserver(observer)
     }
     if let stream = fsEventStream {
+        // Ensure callback completes before we deallocate
         FSEventStreamStop(stream)
         FSEventStreamInvalidate(stream)
         FSEventStreamRelease(stream)
     }
 }

Alternatively, use passRetained/takeRetainedValue to prevent deallocation while callbacks are active, then explicitly release in stopFSEvents.

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

In `@Sources/Panels/ExplorerSidebarPanel.swift` around lines 338 - 368, The
FSEvents callback uses Unmanaged.passUnretained(self) which can cause a
use-after-free if deinit runs while callbacks are queued; update
startFSEvents/stopFSEvents to ensure the panel is retained while callbacks may
run: when creating the context in startFSEvents use Unmanaged.passRetained(self)
(so the callback can safely call
Unmanaged<ExplorerSidebarPanel>.fromOpaque(info).takeRetainedValue()), and then
in stopFSEvents synchronously stop/unschedule/invalidate the FSEventStream on
the main run loop and explicitly release the retained reference with
Unmanaged.release(...) (or call takeRetainedValue() once and let it deinit) so
the retain is balanced and no in-flight callback can touch freed memory; ensure
deinit calls stopFSEvents on the main thread to avoid races with
FSEventStreamStart/FSEventStreamSchedule.

Comment on lines +424 to +431
func sendRootsToJS() {
let rootsJSON = rootPaths.enumerated().map { index, path in
let name = (path as NSString).lastPathComponent
return "{\"name\":\"\(name.replacingOccurrences(of: "\"", with: "\\\""))\",\"rootIndex\":\(index)}"
}.joined(separator: ",")
let js = "if (window.cmuxExplorer && window.cmuxExplorer.setRoots) { window.cmuxExplorer.setRoots([\(rootsJSON)]); }"
webView.evaluateJavaScript(js, completionHandler: nil)
}

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

Manual JSON construction is incomplete — backslashes not escaped.

Line 427 escapes " but not \, so a folder named test\folder produces invalid JSON {"name":"test\folder",...}. Use JSONSerialization for correctness.

Suggested fix using JSONSerialization
 func sendRootsToJS() {
-    let rootsJSON = rootPaths.enumerated().map { index, path in
-        let name = (path as NSString).lastPathComponent
-        return "{\"name\":\"\(name.replacingOccurrences(of: "\"", with: "\\\""))\",\"rootIndex\":\(index)}"
-    }.joined(separator: ",")
-    let js = "if (window.cmuxExplorer && window.cmuxExplorer.setRoots) { window.cmuxExplorer.setRoots([\(rootsJSON)]); }"
+    let roots = rootPaths.enumerated().map { index, path -> [String: Any] in
+        ["name": (path as NSString).lastPathComponent, "rootIndex": index]
+    }
+    guard let jsonData = try? JSONSerialization.data(withJSONObject: roots),
+          let jsonString = String(data: jsonData, encoding: .utf8) else { return }
+    let js = "if (window.cmuxExplorer && window.cmuxExplorer.setRoots) { window.cmuxExplorer.setRoots(\(jsonString)); }"
     webView.evaluateJavaScript(js, completionHandler: nil)
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/ExplorerSidebarPanel.swift` around lines 424 - 431, The manual
JSON string building in sendRootsToJS fails to escape backslashes and other
characters; instead build an array of dictionaries (e.g., using
rootPaths.enumerated() to produce [{"name": name, "rootIndex": index}, ...]) and
serialize it with JSONSerialization or JSONEncoder to produce a safe JSON
string, then inject that serialized string into the js string passed to
webView.evaluateJavaScript (keep using window.cmuxExplorer.setRoots but
substitute the serialized JSON), ensuring you handle serialization errors and
avoid manual escaping of name.

Comment on lines +21 to +37
ForEach(tabs, id: \.0) { tab, icon in
Button {
selected = tab
} label: {
Image(systemName: icon)
.font(.system(size: 12, weight: selected == tab ? .semibold : .regular))
.frame(maxWidth: .infinity)
.frame(height: 24)
.foregroundStyle(selected == tab ? .primary : .tertiary)
.background(
selected == tab
? RoundedRectangle(cornerRadius: 4).fill(Color.white.opacity(0.08))
: nil
)
.contentShape(Rectangle())
}
.buttonStyle(.plain)

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

Give the icon-only sidebar tabs accessible names and selected semantics.

Right now these buttons expose only SF Symbols, so VoiceOver users cannot tell Workspaces / Explorer / Search apart or which tab is active. Add localized accessibilityLabel/help text and mark the selected tab as selected on the Button itself.

As per coding guidelines, "All user-facing strings must be localized using String(localized: "key.name", defaultValue: "English text") for every string shown in the UI (labels, buttons, menus, dialogs, tooltips, error messages)."

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

In `@Sources/SidebarTabSelector.swift` around lines 21 - 37, The icon-only tab
buttons created in ForEach(tabs, id: \.0) { tab, icon in ... } need localized
accessible names and a selected trait so VoiceOver can identify each tab and
which is active; update the Button (not just the Image) to call
.accessibilityLabel(String(localized: "sidebar.<tabKey>", defaultValue:
"<English name>")) and .help(String(localized: "sidebar.<tabKey>.help",
defaultValue: "<English name>")) (one key per tab like "workspaces", "explorer",
"search"), and when selected == tab also add
.accessibilityAddTraits(.isSelected) (or remove that trait when not selected) so
the selected state is announced. Ensure all user-facing strings use
String(localized:..., defaultValue:...) per guidelines and attach these
modifiers to the Button that sets selected.

@imadbz

imadbz commented Mar 25, 2026

Copy link
Copy Markdown
Author

@lawrencecchen @austinywang

got your suggestions implemented.

Screenshot 2026-03-25 at 1 50 17 PM

[x] new files open in cmux tabs
[x] explorer handles multi folders
[x] added a tabbed sidebar, projects, explorer, search
[x] explorer and search implemented with swift
[x] new files open the same way vscode does, new files with one click replaces previously opened editor panel. if you double click on a file, or edit a file it will consider it pinned and will use a new tab for future file opens

- Add commandPalette.kind.editor to xcstrings (en: Editor, ja: エディタ)
- Cmd+Shift+E now opens file explorer sidebar tab instead of creating
  editor panel (files are opened from explorer now)
- Add cmuxSidebarSwitchToExplorer notification

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@imadbz

imadbz commented Mar 25, 2026

Copy link
Copy Markdown
Author

PR Review Issues — Status Update

All 10 issues from the review have been addressed:

Already resolved by refactoring (single-file editor, native explorer)

  • P1 Bundle Monaco — By design: WebView pool pre-loads Monaco from CDN at startup (2 instances ready before any user interaction). Bundling would add ~15MB to the app. The pool eliminates the latency concern.
  • P1 Dirty tracking after rename — No longer applicable. Editor is now single-file, no rename/move within editor.
  • P1 Close dirty tab without confirmation — No longer applicable. Single-file editor, tab closing handled by cmux's own close flow.
  • P2 codicon.css from CDN — Old editor.css removed entirely. Explorer is native SwiftUI now.
  • P2 Overlapping async polls — Removed JS polling entirely. Uses native FSEvents file watching.
  • P2 createFile return value — createFile/createDir/readDir handlers removed from EditorMessageHandler. Only writeFile remains.
  • P2 Drag-move nested selection — Drag-and-drop code removed (was in old WebView explorer).
  • P2 Git conflict check order — Old git status parsing in EditorMessageHandler removed. ExplorerSidebarPanel has correct conflict-first ordering.

Fixed in latest push (f11cbea)

  • P3 commandPalette.kind.editor localization — Added to Localizable.xcstrings with English and Japanese translations.
  • P2 open_editor socket focus gating — Was already correct (socketCommandAllowsInAppFocusMutations() check at line 13447). Additionally, Cmd+Shift+E now opens the explorer sidebar tab instead of creating an editor panel.

@cubic-dev-ai cubic-dev-ai 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.

2 issues found across 5 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="docs/README.bs.md">

<violation number="1" location="docs/README.bs.md:8">
P2: Relative links in docs/README.bs.md point to non-existent paths (e.g., ./docs/assets/... and README.md/LICENSE resolve under docs/), which will break images and navigation for this doc.</violation>

<violation number="2" location="docs/README.bs.md:11">
P3: Stray `fadsd` text will render in the README; remove the accidental line.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread docs/README.bs.md

<p align="center">
<a href="https://github.com/manaflow-ai/cmux/releases/latest/download/cmux-macos.dmg">
<img src="./docs/assets/macos-badge.png" alt="Preuzmi cmux za macOS" width="180" />

@cubic-dev-ai cubic-dev-ai Bot Mar 25, 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.

P2: Relative links in docs/README.bs.md point to non-existent paths (e.g., ./docs/assets/... and README.md/LICENSE resolve under docs/), which will break images and navigation for this doc.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/README.bs.md, line 8:

<comment>Relative links in docs/README.bs.md point to non-existent paths (e.g., ./docs/assets/... and README.md/LICENSE resolve under docs/), which will break images and navigation for this doc.</comment>

<file context>
@@ -0,0 +1,273 @@
+
+<p align="center">
+  <a href="https://github.com/manaflow-ai/cmux/releases/latest/download/cmux-macos.dmg">
+    <img src="./docs/assets/macos-badge.png" alt="Preuzmi cmux za macOS" width="180" />
+  </a>
+</p>
</file context>
Fix with Cubic

Comment thread docs/README.bs.md
<img src="./docs/assets/macos-badge.png" alt="Preuzmi cmux za macOS" width="180" />
</a>
</p>
fadsd

@cubic-dev-ai cubic-dev-ai Bot Mar 25, 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.

P3: Stray fadsd text will render in the README; remove the accidental line.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/README.bs.md, line 11:

<comment>Stray `fadsd` text will render in the README; remove the accidental line.</comment>

<file context>
@@ -0,0 +1,273 @@
+    <img src="./docs/assets/macos-badge.png" alt="Preuzmi cmux za macOS" width="180" />
+  </a>
+</p>
+fadsd
+<p align="center">
+  <a href="README.md">English</a> | <a href="README.ja.md">日本語</a> | <a href="README.zh-CN.md">简体中文</a> | <a href="README.zh-TW.md">繁體中文</a> | <a href="README.ko.md">한국어</a> | <a href="README.de.md">Deutsch</a> | <a href="README.es.md">Español</a> | <a href="README.fr.md">Français</a> | <a href="README.it.md">Italiano</a> | <a href="README.da.md">Dansk</a> | <a href="README.pl.md">Polski</a> | <a href="README.ru.md">Русский</a> | Bosanski | <a href="README.ar.md">العربية</a> | <a href="README.no.md">Norsk</a> | <a href="README.pt-BR.md">Português (Brasil)</a> | <a href="README.th.md">ไทย</a> | <a href="README.tr.md">Türkçe</a> | <a href="README.km.md">ភាសាខ្មែរ</a>
</file context>
Fix with Cubic

Copy link
Copy Markdown

Implemented the unresolved review fixes locally and pushed them to my fork because this account cannot push to manaflow-ai/cmux (git push origin HEAD:feature/monaco-editor-panel returned 403).

Commit:

What’s covered:

  • editor root-path selection now uses the workspace project root / matching file root instead of drifting currentDirectory
  • detached editor panels reinstall their subscriptions when reattached
  • explorer/native explorer FSEvents no longer keep unsafe unretained callbacks alive
  • explorer openFileExternal / pinFileExternal now validate paths through the same traversal guard as other file actions
  • explorer roots are sent to JS via JSONSerialization instead of manual string-building
  • Monaco now caches the pending theme, clears dirty state correctly on buffer replacement, disposes old models, and surfaces save failures inline
  • Codicons are bundled locally instead of being fetched from jsDelivr at runtime
  • Bosnian README broken links / stray text are fixed
  • the unpublished vendor/bonsplit dependency was removed by dropping the preview-tab italic wiring and pinning the submodule back to published commit 447ac42b45256bdf333659d2dbe955afcaa87f6b

Validation I ran:

  • git diff --check
  • node --check Resources/editor/editor.js
  • xcodebuild -list -project GhosttyTabs.xcodeproj
  • xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-ci -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO build
    • this still fails in this checkout because GhosttyKit.xcframework is missing locally, not because of the above patch set.

@neno-is-ooo

Copy link
Copy Markdown

Follow-up: since I can't push directly to imadbz/cmux, I opened a helper PR against the actual head branch here:

If imadbz/cmux#1 is merged into imadbz:feature/monaco-editor-panel, this original PR will pick up commit 561acb312fdbcfdddd9e448f3a6c6e83bf8f1997 automatically.

@teamleaderleo teamleaderleo added the needs-triage Auto-triage could not pick an area; a person decides 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

needs-triage Auto-triage could not pick an area; a person decides

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants