feat: cmux.json for custom commands#2011
Merged
lawrencecchen merged 13 commits intomanaflow-ai:mainfrom Mar 25, 2026
Merged
Conversation
|
@pratikpakhale is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
Author
|
@codex review |
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CMUX support: new Codable models, a MainActor store that discovers/merges/watches local+global cmux.json, dynamic command-palette contributions and executor for commands/workspaces, layout application into workspaces, terminal input routing, unit tests, and localized docs + nav entries. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/Scene
participant Store as CmuxConfigStore
participant Watcher as FileWatcher
participant Loader as ConfigLoader
participant UI as ContentView/CmdPalette
participant Executor as CmuxConfigExecutor
participant TabMgr as TabManager
participant Workspace as Workspace
participant Panel as TerminalPanel
participant Ghostty as GhosttyTerminalView
App->>Store: wireDirectoryTracking(tabManager)
App->>Store: loadAll()
Store->>Watcher: start watching local/global paths
Watcher-->>Store: file change event
Store->>Loader: parse local then global cmux.json
Loader-->>Store: update loadedCommands & bump configRevision
Store-->>UI: publish commands
UI->>Executor: user invokes command (id)
Executor->>Store: resolveCwd(if needed)
Executor->>TabMgr: find or create workspace (apply restart policy)
Executor->>Workspace: applyCustomLayout(layoutNode, baseCwd)
Workspace->>Panel: create/configure panes & surfaces
Panel->>Ghostty: sendInput() / key events for initial commands
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@pratikpakhale I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
|
To use Codex here, create a Codex account and connect to github. |
1 similar comment
|
To use Codex here, create a Codex account and connect to github. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 32 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="web/messages/da.json">
<violation number="1" location="web/messages/da.json:352">
P2: Danish translation typo in `surfaceDefinition` produces malformed user-facing docs text.</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-aiwith guidance or docs links (includingllms.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.
Contributor
Greptile SummaryThis PR introduces Key issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant App as cmuxApp (onAppear)
participant Store as CmuxConfigStore
participant FS as FileSystem (DispatchSource)
participant CV as ContentView
participant Exec as CmuxConfigExecutor
participant WS as Workspace
App->>Store: wireDirectoryTracking(tabManager)
Store->>Store: updateLocalConfigPath(directory)
Store->>Store: loadAll() — parses local + global cmux.json
App->>Store: loadAll() (redundant second call)
FS-->>Store: file write/delete/rename event
Store->>Store: loadAll() — reloads config
Store->>CV: @Published loadedCommands / configRevision changes
CV->>CV: commandPaletteCommandsFingerprint changes
CV->>CV: registerCommandPaletteCommands() — re-registers handlers
note over CV: User selects custom command in palette
CV->>Exec: CmuxConfigExecutor.execute(command, tabManager, baseCwd)
alt has workspace definition
Exec->>Exec: check for existing workspace by name
alt restart == ignore
Exec->>WS: selectWorkspace(existing)
else restart == recreate
Exec->>WS: closeWorkspace(existing)
Exec->>WS: addWorkspace + applyCustomLayout
else restart == confirm
Exec->>Exec: show NSAlert
Exec->>WS: closeWorkspace + addWorkspace + applyCustomLayout
end
else has shell command
Exec->>Exec: optionally show confirm NSAlert
Exec->>WS: focusedTerminalPanel.sendInput(command + newline)
end
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
Sources/CmuxConfig.swift (2)
118-148: Singleton deinit is effectively dead code.Since
CmuxConfigStore.sharedis a static singleton,deinitwill never be called during normal app lifecycle. The cleanup code in lines 146-148 is unreachable.This isn't a bug—it's just unnecessary code. You could remove the deinit entirely, or keep it for completeness if the singleton pattern might change in the future.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 118 - 148, The deinit in CmuxConfigStore is unreachable because the singleton instance CmuxConfigStore.shared never deallocates; remove the deinit block (the cancel calls for localFileWatchSource and globalFileWatchSource) to avoid dead code, or alternatively replace it by a public cleanup method (e.g., shutdown or tearDown) that explicitly cancels localFileWatchSource, globalFileWatchSource and closes descriptors so callers can perform cleanup if the lifecycle ever changes; update any docs/comments to reference CmuxConfigStore.shared and the new cleanup method if added.
226-238: Consider surfacing parse errors to users.Currently, JSON parse errors are only logged via
NSLog(line 235). Users editingcmux.jsonwon't see feedback if their configuration is malformed—commands simply won't appear in the palette.Consider adding a mechanism to surface configuration errors (e.g., a notification, status indicator, or diagnostic command) so users can troubleshoot issues with their
cmux.jsonfiles.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 226 - 238, parseConfig currently swallows JSON decode failures (only NSLog) so users get no visible feedback for malformed cmux.json; change parseConfig(at:) to surface errors by emitting a user-facing event instead of just logging: keep the NSLog, then post a Notification (e.g. Notification.Name("CmuxConfigParseError")) or call a new CmuxConfigDelegate method (e.g. cmuxConfig(_:didFailParsingAtPath:error:)) with the path and the caught error so the UI/command layer can show a notification/status/diagnostic or populate a diagnostics command; reference the parseConfig(at:), CmuxConfigFile, and the catch error to include in the notification/delegate payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Line 142: The CmuxConfigStore is shared across windows
(CmuxConfigStore.shared) but only the primary window's TabManager is wired via
wireDirectoryTracking(tabManager:), causing secondary windows to show the
primary window's loadedCommands; fix by ensuring per-window wiring or per-window
command isolation—either update createMainWindow() to call
CmuxConfigStore.shared.wireDirectoryTracking(tabManager:) for every new window's
TabManager (reference createMainWindow(), TabManager, and
wireDirectoryTracking(tabManager:)), or change CmuxConfigStore to key
loadedCommands by window/workspace ID so each window uses its own cmux.json
state (reference CmuxConfigStore, loadedCommands).
In `@Sources/ContentView.swift`:
- Around line 6314-6320: The code uses
tabManager.selectedWorkspace?.currentDirectory directly for baseCwd, but
Workspace.currentDirectory can be an empty or whitespace string so the
nil-coalescing (??) won't substitute the home path; before calling
CmuxConfigExecutor.execute ensure you treat empty/whitespace as missing by
trimming and checking isEmpty (e.g., compute baseCwd from
tabManager.selectedWorkspace?.currentDirectory?.trimmingCharacters(in:
.whitespacesAndNewlines) and if that result is empty or nil fall back to
FileManager.default.homeDirectoryForCurrentUser.path) so
CmuxConfigExecutor.execute receives a valid non-empty working directory.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 3725-3730: The loop over text.unicodeScalars currently treats CR
(0x0D) and LF (0x0A) separately causing "\r\n" to produce two Return events;
modify the scalar handling in the loop inside GhosttyTerminalView (where
bufferedText is built, flushText(&bufferedText, surface:) is called and
sendKeyEvent(surface:keycode:) is invoked) to coalesce a CR followed immediately
by an LF into a single Return: when you see 0x0D, peek the next scalar (or
advance an index) and if it is 0x0A consume it without emitting a second Return,
otherwise emit the single Return as before, ensuring flushText is only called
once for the CR+LF pair.
- Around line 3723-3724: CmuxConfigExecutor.executeSimpleCommand currently calls
GhosttyTerminalView.sendInput(_:) directly which drops input if surface is nil;
change the call to follow the Workspace.sendInputWhenReady pattern: check
panel.surface.surface != nil before calling sendInput, and if nil register/defer
the input to be sent when the .terminalSurfaceDidBecomeReady event fires (or
invoke the same queuing used by sendText). Ensure you reference
CmuxConfigExecutor.executeSimpleCommand, GhosttyTerminalView.sendInput(_:),
Workspace.sendInputWhenReady, panel.surface.surface and the
.terminalSurfaceDidBecomeReady notification so the command is queued and
delivered when the terminal surface becomes ready.
In `@Sources/Workspace.swift`:
- Around line 891-916: In sendInputWhenReady(_:, to:) remove the 3s
DispatchQueue.main.asyncAfter timeout and the NSLog that prints command text;
instead keep the input queued until panel.surface becomes ready or the panel is
closed/destroyed, and ensure the NotificationCenter observer is removed on
close; when emitting any failure log, log only non-secret identifiers (e.g.
panel.identifier or panel.surface.identifier) rather than the command text.
Locate the observer setup in sendInputWhenReady, delete the timeout block, add a
cleanup path triggered by panel/surface teardown (listen for a close/didDestroy
notification or implement deinit/close callback) that removes the observer and
drops the queued input without logging secrets.
In `@web/messages/da.json`:
- Line 352: The Danish translation for the key "surfaceDefinition" contains an
extraneous space ("Overfladedef inition"); remove the space so the value reads
"Overfladedefinition" by updating the value for the "surfaceDefinition" key in
the da.json translations so it matches the correct single-word spelling.
In `@web/messages/tr.json`:
- Around line 305-368: The Turkish translations put the customCommands object
under docs.configuration.customCommands but the page expects docs.customCommands
(see useTranslations("docs.customCommands") and getTranslations({ locale,
namespace: "docs.customCommands" })); move the entire "customCommands" object
out of the "configuration" object so it sits as a sibling of "configuration"
under "docs" (matching the structure used in en.json) so lookup by
docs.customCommands resolves correctly.
---
Nitpick comments:
In `@Sources/CmuxConfig.swift`:
- Around line 118-148: The deinit in CmuxConfigStore is unreachable because the
singleton instance CmuxConfigStore.shared never deallocates; remove the deinit
block (the cancel calls for localFileWatchSource and globalFileWatchSource) to
avoid dead code, or alternatively replace it by a public cleanup method (e.g.,
shutdown or tearDown) that explicitly cancels localFileWatchSource,
globalFileWatchSource and closes descriptors so callers can perform cleanup if
the lifecycle ever changes; update any docs/comments to reference
CmuxConfigStore.shared and the new cleanup method if added.
- Around line 226-238: parseConfig currently swallows JSON decode failures (only
NSLog) so users get no visible feedback for malformed cmux.json; change
parseConfig(at:) to surface errors by emitting a user-facing event instead of
just logging: keep the NSLog, then post a Notification (e.g.
Notification.Name("CmuxConfigParseError")) or call a new CmuxConfigDelegate
method (e.g. cmuxConfig(_:didFailParsingAtPath:error:)) with the path and the
caught error so the UI/command layer can show a notification/status/diagnostic
or populate a diagnostics command; reference the parseConfig(at:),
CmuxConfigFile, and the catch error to include in the notification/delegate
payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f0c8508f-119e-4cec-b255-4f40f887259b
📒 Files selected for processing (32)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigExecutor.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/CmuxConfigTests.swiftweb/app/[locale]/components/docs-nav-items.tsweb/app/[locale]/docs/custom-commands/page.tsxweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (6)
Sources/cmuxApp.swift (1)
142-142:⚠️ Potential issue | 🟠 MajorMake
cmux.jsonstate window-scoped, not app-scoped.Line 142 shares one
CmuxConfigStoreacross the app, but Lines 351-352 wire and load it only from the primary window’stabManager. In a multi-window session, secondary windows can only see whichever repo the primary window last loaded, so their command palette can surface the wrongcmux.jsoncommands. Please give each main window its own store, or key the store’s loaded commands/watch state by window/workspace and wire every window’sTabManager.Also applies to: 351-352
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` at line 142, The app currently uses a single shared CmuxConfigStore (cmuxConfigStore = CmuxConfigStore.shared) which makes cmux.json state app-scoped; change it so each main window gets its own store instance or a store keyed by window/workspace and ensure each window's TabManager wires/loads its own store (instead of only the primary window calling TabManager.load on the shared store). Concretely, replace the app-scoped `@StateObject` cmuxConfigStore with a window-local StateObject (or a factory that returns CmuxConfigStore(windowID)) and update where TabManager.load/TabManager wires the store (the code around the TabManager wiring at the previous load calls) so every window calls the load/watch methods for its own CmuxConfigStore keyed to that window/workspace.web/messages/da.json (1)
352-352:⚠️ Potential issue | 🟡 MinorFix typo in Danish translation string.
Line 352 still contains a split word:
"Overfladedef inition"should be"Overfladedefinition".✏️ Suggested fix
- "surfaceDefinition": "Overfladedef inition", + "surfaceDefinition": "Overfladedefinition",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/messages/da.json` at line 352, The translation value for the key "surfaceDefinition" contains a split word "Overfladedef inition"; update the Danish JSON entry for "surfaceDefinition" to the correct single word "Overfladedefinition" so the string reads "surfaceDefinition": "Overfladedefinition".Sources/ContentView.swift (1)
6314-6320:⚠️ Potential issue | 🟡 MinorTreat blank working directories as missing before falling back.
Workspace.currentDirectoryis already normalized elsewhere in this file. The??only handlesnil, so""or whitespace can still flow intoCmuxConfigExecutor.execute(...)and break relative-path resolution.🛠️ Suggested fix
- let baseCwd = tabManager.selectedWorkspace?.currentDirectory - ?? FileManager.default.homeDirectoryForCurrentUser.path + let baseCwd: String = { + let candidate = tabManager.selectedWorkspace?.currentDirectory + .trimmingCharacters(in: .whitespacesAndNewlines) + guard let candidate, !candidate.isEmpty else { + return FileManager.default.homeDirectoryForCurrentUser.path + } + return candidate + }() CmuxConfigExecutor.execute( command: captured, tabManager: tabManager,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 6314 - 6320, The selected workspace's currentDirectory can be an empty or whitespace string and should be treated as missing before falling back; when building baseCwd (from tabManager.selectedWorkspace?.currentDirectory) trim whitespace and check for emptiness and if empty use FileManager.default.homeDirectoryForCurrentUser.path, then pass that sanitized baseCwd into CmuxConfigExecutor.execute to avoid propagating blank paths into relative-path resolution.Sources/GhosttyTerminalView.swift (2)
3723-3724:⚠️ Potential issue | 🔴 Critical
sendInputcurrently drops commands when the surface is not readyAt Line 3724, returning immediately on
surface == nilcan lose config-driven/auto-run input during terminal startup. This path should queue/defer likesendText(_:)instead of dropping.💡 Suggested fix
func sendInput(_ text: String) { - guard let surface = surface else { return } + guard let surface = surface else { + enqueuePendingInput(text) + return + } var bufferedText = "" for scalar in text.unicodeScalars { switch scalar.value {// Add a pending input queue (parallel to pendingTextQueue) and flush it // after surface creation (near flushPendingTextIfNeeded()) so control-key // semantics are preserved once the runtime surface is ready.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 3723 - 3724, sendInput currently returns when surface is nil and drops commands; instead create a pendingInputQueue (parallel to pendingTextQueue) and push incoming text into it when surface == nil inside sendInput, then flush that queue from the same place you flush pendingText (e.g. inside flushPendingTextIfNeeded() or immediately after surface is created) to preserve control-key semantics; update sendInput, where you reference surface and sendText, to dequeue and send pending inputs once flushPendingTextIfNeeded() runs and ensure ordering with pendingTextQueue is maintained.
3726-3730:⚠️ Potential issue | 🟡 MinorCoalesce CRLF into a single Return key event
Line 3728 treats
\rand\nindependently, so"\r\n"emits two Return presses.💡 Suggested fix
- for scalar in text.unicodeScalars { + let scalars = Array(text.unicodeScalars) + var index = 0 + while index < scalars.count { + let scalar = scalars[index] switch scalar.value { - case 0x0A, 0x0D: + case 0x0D: flushText(&bufferedText, surface: surface) sendKeyEvent(surface: surface, keycode: 0x24) // kVK_Return + if index + 1 < scalars.count, scalars[index + 1].value == 0x0A { + index += 1 // consume LF in CRLF + } + case 0x0A: + flushText(&bufferedText, surface: surface) + sendKeyEvent(surface: surface, keycode: 0x24) // kVK_Return case 0x09: flushText(&bufferedText, surface: surface) sendKeyEvent(surface: surface, keycode: 0x30) // kVK_Tab case 0x1B: flushText(&bufferedText, surface: surface) sendKeyEvent(surface: surface, keycode: 0x35) // kVK_Escape default: bufferedText.unicodeScalars.append(scalar) } + index += 1 }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 3726 - 3730, The loop over text.unicodeScalars in GhosttyTerminalView is treating 0x0D (CR) and 0x0A (LF) as separate Return key presses; change the logic in the loop that calls flushText(&bufferedText, surface: surface) and sendKeyEvent(surface: surface, keycode: 0x24) so that a CR immediately followed by an LF only emits one Return event. Implement a small state flag (e.g., lastWasCR) or remember the previous scalar in the loop that, when you see LF (0x0A), checks if lastWasCR is true and if so only performs the flush without calling sendKeyEvent again; ensure you still flush once and reset the flag appropriately; update references in the loop that use text.unicodeScalars, bufferedText, flushText, and sendKeyEvent.Sources/Workspace.swift (1)
891-916:⚠️ Potential issue | 🟠 MajorDon't time out and log queued startup commands.
The 3s timeout on Line 911 still makes auto-run nondeterministic for
focus: falseterminals that stay cold until selected, and Line 915 still writes user-provided command text to Console. Keep the input queued until the terminal becomes ready or the panel is torn down, trigger a background surface start for hidden terminals, and log only opaque identifiers if cleanup fails.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 891 - 916, sendInputWhenReady currently gives up after 3s and logs the user command text; instead keep the input queued until the surface becomes ready or the panel is torn down, trigger a background start for hidden surfaces, and avoid logging sensitive command text. Update sendInputWhenReady to remove the DispatchQueue.main.asyncAfter timeout, add an observer for panel/ surface teardown (so queued input is discarded when the panel is removed), and proactively request the surface to start in background if hidden (call the surface's background-start API on panel.surface). When cleanup fails, log only an opaque identifier (e.g. panel.identifier or panel.surface.identifier) rather than the command text, and ensure observer removal still happens in both success and teardown paths (references: sendInputWhenReady, TerminalPanel, panel.surface, .terminalSurfaceDidBecomeReady, panel.sendInput).
🧹 Nitpick comments (1)
web/app/[locale]/docs/custom-commands/page.tsx (1)
38-290: Consider extracting large inline JSON examples into constants.The page is readable now, but moving long literals out of JSX will make future edits and reviews easier.
♻️ Minimal refactor pattern
+const SCHEMA_EXAMPLE = `{ + "commands": [ + { + "name": "Start Dev", + "keywords": ["dev", "start"], + "workspace": { ... } + } + ] +}`; + export default function CustomCommandsPage() { const t = useTranslations("docs.customCommands"); @@ - <CodeBlock title="cmux.json" lang="json">{`{ - "commands": [ - { - "name": "Start Dev", - "keywords": ["dev", "start"], - "workspace": { ... } - }, - { - "name": "Run Tests", - "command": "npm test", - "confirm": true - } - ] -}`}</CodeBlock> + <CodeBlock title="cmux.json" lang="json">{SCHEMA_EXAMPLE}</CodeBlock>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/`[locale]/docs/custom-commands/page.tsx around lines 38 - 290, The JSX contains multiple large inline JSON examples inside CodeBlock elements (e.g., the long cmux.json literals rendered by <CodeBlock title="cmux.json" lang="json">); extract each large JSON string into named top-level constants (for example FULL_EXAMPLE_JSON, WORKSPACE_EXAMPLE_JSON, SIMPLE_COMMANDS_JSON, etc.) defined near the top of the module and replace the inline template strings with those constant references while keeping the existing <CodeBlock ...> props (title/lang) and translations (t(...)) unchanged so editors/reviewers can edit examples more easily.
🤖 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/Localizable.xcstrings`:
- Around line 15256-15368: The localized key command.cmuxConfig.customTitle
contains a % @ placeholder but the usage in ContentView (where the command title
is composed) currently interpolates the localized string directly, causing the
literal placeholder to appear; wrap the localized lookup with
String.localizedStringWithFormat(String(localized:
"command.cmuxConfig.customTitle", defaultValue: "Custom: %@"), commandName) so
the placeholder is substituted — update the code that builds the custom title in
ContentView to use String.localizedStringWithFormat with String(localized:) and
pass the command name (e.g., commandName) as the formatting argument.
In `@Sources/CmuxConfig.swift`:
- Around line 118-123: CmuxConfigStore currently uses a global singleton
(CmuxConfigStore.shared) holding localConfigPath, loadedCommands and
subscriptions which causes per-window configs to collide; change it to be a
per-window instance instead: remove/stop using the static shared, add an init
that accepts the window-scoped owner (e.g., FileBrowserDrawerState or
TabManager) or a localConfigPath, move wiring logic in
wireDirectoryTracking(tabManager:) to operate on instance properties
(localConfigPath, loadedCommands, subscription set) and ensure each window
creates and owns its own CmuxConfigStore (ContentView.onAppear /
AppDelegate.createMainWindow should instantiate and pass the store into views)
so subscriptions are not torn down by other windows and each window shows its
own command set.
- Around line 9-16: CmuxCommandDefinition currently allows ambiguous configs
because workspace and command are both optional; update its Decodable
implementation (implement init(from decoder:) for CmuxCommandDefinition) to
validate that exactly one of workspace or command is present and throw a
decoding error otherwise, rejecting entries where both are nil or both are
non-nil; keep the type conformances (Codable/Sendable/Identifiable) but perform
the cross-field check during decode and surface a clear decoding failure message
referencing the name (or index) to fail fast on hybrid/empty command
definitions.
- Around line 155-169: The code currently reconstructs local config from
workspace.$currentDirectory (in the tabManager.$selectedTabId pipeline and
similarly at 177-191), causing config to follow transient cd changes; change
updateLocalConfigPath usage to resolve a stable project root instead: when
reacting to workspace selection (tabManager.$selectedTabId -> workspace), call a
helper that walks the directory ancestors starting from
workspace.currentDirectory (or a stable workspace.initialDirectory/projectRoot
if available) up to filesystem root looking for "cmux.json" and set
localConfigPath to the nearest ancestor containing that file; update the logic
in updateLocalConfigPath (or replace its caller) to perform this ancestor search
(referencing workspace.$currentDirectory, updateLocalConfigPath(_:), and the
tabManager.$selectedTabId pipeline) so transient shell cd does not drop the
project-local config.
In `@Sources/ContentView.swift`:
- Around line 5967-5974: The code currently uses user-controlled command.id from
cmuxConfigStore.loadedCommands directly when creating
CommandPaletteCommandContribution, which can collide with built-in commands or
other custom IDs and later crash in
ContentView.refreshCommandPaletteSearchCorpus(force:query:); change the logic to
either prefix/namespace the stored IDs (e.g., "cmuxConfig.\(command.id)") before
passing them into CommandPaletteCommandContribution or validate/reject IDs that
collide with existing command IDs at the point where commands are
appended/registered, and ensure the same namespacing/validation is applied when
rebuilding the corpus in
ContentView.refreshCommandPaletteSearchCorpus(force:query:).
In `@web/messages/no.json`:
- Line 309: The "intro" message value contains a Norwegian typo: change the word
"rotmapp" to "rotmappe" in the JSON entry with key "intro" (the string
mentioning "cmux.json" and "~/.config/cmux/") so the sentence reads "...ved å
legge til en cmux.json-fil i prosjektets rotmappe eller ~/.config/cmux/....".
---
Duplicate comments:
In `@Sources/cmuxApp.swift`:
- Line 142: The app currently uses a single shared CmuxConfigStore
(cmuxConfigStore = CmuxConfigStore.shared) which makes cmux.json state
app-scoped; change it so each main window gets its own store instance or a store
keyed by window/workspace and ensure each window's TabManager wires/loads its
own store (instead of only the primary window calling TabManager.load on the
shared store). Concretely, replace the app-scoped `@StateObject` cmuxConfigStore
with a window-local StateObject (or a factory that returns
CmuxConfigStore(windowID)) and update where TabManager.load/TabManager wires the
store (the code around the TabManager wiring at the previous load calls) so
every window calls the load/watch methods for its own CmuxConfigStore keyed to
that window/workspace.
In `@Sources/ContentView.swift`:
- Around line 6314-6320: The selected workspace's currentDirectory can be an
empty or whitespace string and should be treated as missing before falling back;
when building baseCwd (from tabManager.selectedWorkspace?.currentDirectory) trim
whitespace and check for emptiness and if empty use
FileManager.default.homeDirectoryForCurrentUser.path, then pass that sanitized
baseCwd into CmuxConfigExecutor.execute to avoid propagating blank paths into
relative-path resolution.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 3723-3724: sendInput currently returns when surface is nil and
drops commands; instead create a pendingInputQueue (parallel to
pendingTextQueue) and push incoming text into it when surface == nil inside
sendInput, then flush that queue from the same place you flush pendingText (e.g.
inside flushPendingTextIfNeeded() or immediately after surface is created) to
preserve control-key semantics; update sendInput, where you reference surface
and sendText, to dequeue and send pending inputs once flushPendingTextIfNeeded()
runs and ensure ordering with pendingTextQueue is maintained.
- Around line 3726-3730: The loop over text.unicodeScalars in
GhosttyTerminalView is treating 0x0D (CR) and 0x0A (LF) as separate Return key
presses; change the logic in the loop that calls flushText(&bufferedText,
surface: surface) and sendKeyEvent(surface: surface, keycode: 0x24) so that a CR
immediately followed by an LF only emits one Return event. Implement a small
state flag (e.g., lastWasCR) or remember the previous scalar in the loop that,
when you see LF (0x0A), checks if lastWasCR is true and if so only performs the
flush without calling sendKeyEvent again; ensure you still flush once and reset
the flag appropriately; update references in the loop that use
text.unicodeScalars, bufferedText, flushText, and sendKeyEvent.
In `@Sources/Workspace.swift`:
- Around line 891-916: sendInputWhenReady currently gives up after 3s and logs
the user command text; instead keep the input queued until the surface becomes
ready or the panel is torn down, trigger a background start for hidden surfaces,
and avoid logging sensitive command text. Update sendInputWhenReady to remove
the DispatchQueue.main.asyncAfter timeout, add an observer for panel/ surface
teardown (so queued input is discarded when the panel is removed), and
proactively request the surface to start in background if hidden (call the
surface's background-start API on panel.surface). When cleanup fails, log only
an opaque identifier (e.g. panel.identifier or panel.surface.identifier) rather
than the command text, and ensure observer removal still happens in both success
and teardown paths (references: sendInputWhenReady, TerminalPanel,
panel.surface, .terminalSurfaceDidBecomeReady, panel.sendInput).
In `@web/messages/da.json`:
- Line 352: The translation value for the key "surfaceDefinition" contains a
split word "Overfladedef inition"; update the Danish JSON entry for
"surfaceDefinition" to the correct single word "Overfladedefinition" so the
string reads "surfaceDefinition": "Overfladedefinition".
---
Nitpick comments:
In `@web/app/`[locale]/docs/custom-commands/page.tsx:
- Around line 38-290: The JSX contains multiple large inline JSON examples
inside CodeBlock elements (e.g., the long cmux.json literals rendered by
<CodeBlock title="cmux.json" lang="json">); extract each large JSON string into
named top-level constants (for example FULL_EXAMPLE_JSON,
WORKSPACE_EXAMPLE_JSON, SIMPLE_COMMANDS_JSON, etc.) defined near the top of the
module and replace the inline template strings with those constant references
while keeping the existing <CodeBlock ...> props (title/lang) and translations
(t(...)) unchanged so editors/reviewers can edit examples more easily.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b583a47e-d56f-48b5-8782-d2f645513dc6
📒 Files selected for processing (32)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigExecutor.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/CmuxConfigTests.swiftweb/app/[locale]/components/docs-nav-items.tsweb/app/[locale]/docs/custom-commands/page.tsxweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
Sources/ContentView.swift (1)
6314-6317:⚠️ Potential issue | 🟡 MinorTrim whitespace-only working directories before falling back.
Line 6315 still only checks
isEmpty, so" "bypasses the fallback and gets forwarded intoCmuxConfigExecutor.execute. That leaves relative custom commands resolving against an invalid base path even though othercurrentDirectoryreads in this file trim before checking emptiness.Suggested fix
- let rawCwd = tabManager.selectedWorkspace?.currentDirectory - let baseCwd = (rawCwd?.isEmpty == false) ? rawCwd! - : FileManager.default.homeDirectoryForCurrentUser.path + let baseCwd: String = { + let candidate = tabManager.selectedWorkspace?.currentDirectory? + .trimmingCharacters(in: .whitespacesAndNewlines) + guard let candidate, !candidate.isEmpty else { + return FileManager.default.homeDirectoryForCurrentUser.path + } + return candidate + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 6314 - 6317, The code registers a command and sets baseCwd from rawCwd but only checks rawCwd.isEmpty, so whitespace-only values like " " bypass the fallback; update the registry.register closure that reads let rawCwd = tabManager.selectedWorkspace?.currentDirectory to trim the value (e.g., use rawCwd?.trimmingCharacters(in: .whitespacesAndNewlines)) and then check the trimmed string's isEmpty; set baseCwd to the trimmed path when non-empty, otherwise fall back to FileManager.default.homeDirectoryForCurrentUser.path so CmuxConfigExecutor.execute receives a valid base path.Sources/CmuxConfig.swift (2)
216-232:⚠️ Potential issue | 🔴 Critical
CmuxConfigStore.sharedis still cross-window state.
sharedowns onelocalConfigPath, oneloadedCommands, and one subscription set. EverywireDirectoryTracking(tabManager:)call replaces the previous window’s tracking, so multi-window sessions can show project-A commands in project-B and execute them in the wrong workspace. Based on learnings:AppDelegate.registerMainWindowownership is window-scoped; primary and secondary windows each own their passed state to avoid re-registration mismatches.Also applies to: 248-266
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 216 - 232, CmuxConfigStore.shared currently holds window-scoped state (localConfigPath, loadedCommands, cancellables, file watch descriptors, and subscriptions) which leads to cross-window leaks when wireDirectoryTracking(tabManager:) is called; change ownership so each window gets its own CmuxConfigStore instance instead of using the global static shared: remove or stop using the static let shared singleton for window-specific flows, make the API accept/produce a per-window CmuxConfigStore, ensure wireDirectoryTracking(tabManager:) is an instance method on CmuxConfigStore (or accept a store instance) so each window maintains its own localConfigPath, loadedCommands, cancellables, localFileWatchSource/localFileDescriptor and globalFileWatchSource/globalFileDescriptor, and update registration code (where AppDelegate.registerMainWindow or callers use CmuxConfigStore.shared) to create and retain a new CmuxConfigStore per window.
251-265:⚠️ Potential issue | 🟠 MajorProject-local config still follows shell
cd.This pipeline recomputes
localConfigPathfromworkspace.currentDirectoryon every directory change, so a workspace that starts at/repodrops/repo/cmux.jsonas soon as the shell moves into/repo/subdir. Resolve from a stable project root, or walk ancestors to the nearestcmux.json, instead of rebinding to transient cwd.Also applies to: 273-287
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 251 - 265, The pipeline currently recomputes localConfigPath directly from workspace.$currentDirectory (in the tabManager.$selectedTabId subscription and the similar block at lines 273-287), which follows transient shell cd; change the logic so you resolve a stable project root instead of rebinding to cwd: when a tab becomes selected (the tabManager.$selectedTabId subscription) use the tab's workspace to locate the repository/project root by walking ancestors from workspace.currentDirectory until you find the nearest "cmux.json" (or a VCS root), and set localConfigPath to that discovered path (falling back to the original workspace path if nothing found). Update the code that calls updateLocalConfigPath to accept the stable project-root result (or add a new helper like findNearestProjectRoot(from:) used by updateLocalConfigPath) and apply the same change to the other similar subscription block so both use ancestor-walking resolution rather than the transient cwd.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/CmuxConfig.swift`:
- Around line 40-65: The decoder allows blank strings for required name and
optional command; update init(from decoder: Decoder) to validate that
name.trimmingCharacters(in: .whitespacesAndNewlines) is not empty and reject it
by throwing DecodingError.dataCorrupted (with appropriate Context) if it is, and
likewise treat command as invalid when decoded if
command?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty — either set it
to nil or throw (prefer throwing) with a clear debugDescription; adjust the
checks around workspace/command after trimming so the existing mutual-exclusion
and presence validation uses the trimmed values.
In `@Sources/ContentView.swift`:
- Around line 5967-5975: Sanitize command.name and command.description from
cmuxConfigStore.loadedCommands using the same dangerousScalars/scalar-filtering
utility used in TerminalController.swift and BrowserPanel.swift before creating
CommandPaletteCommandContribution; replace usages of command.name and
command.description with their sanitized equivalents and if the sanitized
description is empty, use cmuxConfigDefaultSubtitle as the subtitle. Ensure the
title still includes the sanitized commandName in the localized
"command.cmuxConfig.customTitle" call and preserve keywords and command.id
unchanged.
---
Duplicate comments:
In `@Sources/CmuxConfig.swift`:
- Around line 216-232: CmuxConfigStore.shared currently holds window-scoped
state (localConfigPath, loadedCommands, cancellables, file watch descriptors,
and subscriptions) which leads to cross-window leaks when
wireDirectoryTracking(tabManager:) is called; change ownership so each window
gets its own CmuxConfigStore instance instead of using the global static shared:
remove or stop using the static let shared singleton for window-specific flows,
make the API accept/produce a per-window CmuxConfigStore, ensure
wireDirectoryTracking(tabManager:) is an instance method on CmuxConfigStore (or
accept a store instance) so each window maintains its own localConfigPath,
loadedCommands, cancellables, localFileWatchSource/localFileDescriptor and
globalFileWatchSource/globalFileDescriptor, and update registration code (where
AppDelegate.registerMainWindow or callers use CmuxConfigStore.shared) to create
and retain a new CmuxConfigStore per window.
- Around line 251-265: The pipeline currently recomputes localConfigPath
directly from workspace.$currentDirectory (in the tabManager.$selectedTabId
subscription and the similar block at lines 273-287), which follows transient
shell cd; change the logic so you resolve a stable project root instead of
rebinding to cwd: when a tab becomes selected (the tabManager.$selectedTabId
subscription) use the tab's workspace to locate the repository/project root by
walking ancestors from workspace.currentDirectory until you find the nearest
"cmux.json" (or a VCS root), and set localConfigPath to that discovered path
(falling back to the original workspace path if nothing found). Update the code
that calls updateLocalConfigPath to accept the stable project-root result (or
add a new helper like findNearestProjectRoot(from:) used by
updateLocalConfigPath) and apply the same change to the other similar
subscription block so both use ancestor-walking resolution rather than the
transient cwd.
In `@Sources/ContentView.swift`:
- Around line 6314-6317: The code registers a command and sets baseCwd from
rawCwd but only checks rawCwd.isEmpty, so whitespace-only values like " "
bypass the fallback; update the registry.register closure that reads let rawCwd
= tabManager.selectedWorkspace?.currentDirectory to trim the value (e.g., use
rawCwd?.trimmingCharacters(in: .whitespacesAndNewlines)) and then check the
trimmed string's isEmpty; set baseCwd to the trimmed path when non-empty,
otherwise fall back to FileManager.default.homeDirectoryForCurrentUser.path so
CmuxConfigExecutor.execute receives a valid base path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bb6dc663-ffb6-4ea3-b340-c343f620d313
📒 Files selected for processing (7)
Sources/CmuxConfig.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Workspace.swiftcmuxTests/CmuxConfigTests.swiftweb/messages/da.jsonweb/messages/no.json
✅ Files skipped from review due to trivial changes (4)
- web/messages/da.json
- web/messages/no.json
- Sources/Workspace.swift
- cmuxTests/CmuxConfigTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/GhosttyTerminalView.swift
Contributor
Author
|
@coderabbitai review further |
|
🧠 Learnings used✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
Sources/ContentView.swift (1)
6329-6331:⚠️ Potential issue | 🟡 MinorTrim
currentDirectorybefore using it asbaseCwd.This only checks
isEmpty, so a whitespace-onlycurrentDirectorystill reachesCmuxConfigExecutorand breaks relative-path resolution.🛠️ Suggested fix
- let rawCwd = tabManager.selectedWorkspace?.currentDirectory - let baseCwd = (rawCwd?.isEmpty == false) ? rawCwd! - : FileManager.default.homeDirectoryForCurrentUser.path + let baseCwd: String = { + let candidate = tabManager.selectedWorkspace?.currentDirectory? + .trimmingCharacters(in: .whitespacesAndNewlines) + guard let candidate, !candidate.isEmpty else { + return FileManager.default.homeDirectoryForCurrentUser.path + } + return candidate + }()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 6329 - 6331, Trim the workspace currentDirectory before using it: obtain tabManager.selectedWorkspace?.currentDirectory, call trimmingCharacters(in: .whitespacesAndNewlines) and check the trimmed string's isEmpty; if non-empty use the trimmed value for baseCwd, otherwise fall back to FileManager.default.homeDirectoryForCurrentUser.path so CmuxConfigExecutor never receives a whitespace-only path.Sources/CmuxConfig.swift (1)
111-138:⚠️ Potential issue | 🟠 MajorReject split-specific keys on pane nodes too.
The current guard only catches
pane + direction. A node like{ "pane": ..., "children": [...] }or{ "pane": ..., "split": 0.3 }still decodes as.paneand silently drops the split fields, so malformed layouts load with a different shape than the JSON describes.Possible guard
let container = try decoder.container(keyedBy: CodingKeys.self) let hasPane = container.contains(.pane) let hasDirection = container.contains(.direction) + let hasSplit = container.contains(.split) + let hasChildren = container.contains(.children) - if hasPane && hasDirection { + if hasPane && (hasDirection || hasSplit || hasChildren) { throw DecodingError.dataCorrupted( DecodingError.Context( codingPath: decoder.codingPath, - debugDescription: "CmuxLayoutNode must not contain both 'pane' and 'direction' keys" + debugDescription: "Pane nodes must not contain split-specific keys" ) ) }
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/CmuxConfig.swift`:
- Around line 289-304: The early-return in updateLocalConfigPath compares
newPath to localConfigPath and skips starting a watcher when both are nil, so
creating a first-time cmux.json (or a nearer one) under the watched subtree is
never observed; modify updateLocalConfigPath to treat the "both nil" case
specially: if newPath == localConfigPath then only return early when the
incoming directory parameter is nil (i.e., no new subtree to watch); otherwise,
when directory is provided and localConfigPath is nil, do not
return—stopLocalFileWatcher(), set localConfigPath (still nil),
startLocalFileWatcher() for the provided directory so creation of a new
cmux.json will be detected, then call loadAll(); apply the same logic to the
analogous block around the 409-440 region (the other updateLocalConfigPath-like
watcher function) using the same symbols (findCmuxConfig, localConfigPath,
stopLocalFileWatcher, startLocalFileWatcher, loadAll).
---
Duplicate comments:
In `@Sources/ContentView.swift`:
- Around line 6329-6331: Trim the workspace currentDirectory before using it:
obtain tabManager.selectedWorkspace?.currentDirectory, call
trimmingCharacters(in: .whitespacesAndNewlines) and check the trimmed string's
isEmpty; if non-empty use the trimmed value for baseCwd, otherwise fall back to
FileManager.default.homeDirectoryForCurrentUser.path so CmuxConfigExecutor never
receives a whitespace-only path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ab11b2c7-8d1a-449c-9c58-a9ca482825f6
📒 Files selected for processing (5)
Sources/AppDelegate.swiftSources/CmuxConfig.swiftSources/ContentView.swiftSources/cmuxApp.swiftcmuxTests/CmuxConfigTests.swift
✅ Files skipped from review due to trivial changes (1)
- Sources/AppDelegate.swift
There was a problem hiding this comment.
♻️ Duplicate comments (5)
Sources/ContentView.swift (2)
6326-6337:⚠️ Potential issue | 🟡 MinorTreat whitespace-only working directories as missing.
Line 6330 only guards
isEmpty, so" "still passes through asbaseCwd. That leaves relativecwdresolution broken for fresh or not-yet-synced workspaces.🛠️ Suggested fix
for command in cmuxConfigStore.loadedCommands { let captured = command registry.register(commandId: command.id) { - let rawCwd = tabManager.selectedWorkspace?.currentDirectory - let baseCwd = (rawCwd?.isEmpty == false) ? rawCwd! - : FileManager.default.homeDirectoryForCurrentUser.path + let baseCwd: String = { + let candidate = tabManager.selectedWorkspace?.currentDirectory? + .trimmingCharacters(in: .whitespacesAndNewlines) + guard let candidate, !candidate.isEmpty else { + return FileManager.default.homeDirectoryForCurrentUser.path + } + return candidate + }() CmuxConfigExecutor.execute( command: captured, tabManager: tabManager, baseCwd: baseCwd )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 6326 - 6337, The code treats a workspace currentDirectory that contains only whitespace as valid; update the baseCwd determination inside the registry.register closure (where rawCwd is taken from tabManager.selectedWorkspace?.currentDirectory and baseCwd is computed) to consider whitespace-only strings as empty by trimming whitespace/newlines before the isEmpty check and use the trimmed value when non-empty; if the trimmed string is empty or nil, fall back to FileManager.default.homeDirectoryForCurrentUser.path so relative cwd resolution works for fresh or unsynced workspaces.
5966-5981:⚠️ Potential issue | 🟠 MajorStrip control characters from config-sourced palette text too.
Line 5993 only removes the BiDi/zero-width set. Embedded
\r,\n, and other control scalars still survive into a one-line palette row, so a repo-controlledcmux.jsoncan still hide or reshape the visible label/subtitle.Based on learnings, in `Sources/TerminalController.swift` and `Sources/Panels/BrowserPanel.swift`, untrusted-text sanitizers strip control characters together with the dangerous BiDi/zero-width scalar set.🛡️ Possible hardening
private func sanitizeCmuxConfigPaletteText(_ text: String) -> String { let dangerous: Set<Unicode.Scalar> = [ "\u{200B}", "\u{200C}", "\u{200D}", "\u{200E}", "\u{200F}", "\u{202A}", "\u{202B}", "\u{202C}", "\u{202D}", "\u{202E}", "\u{2066}", "\u{2067}", "\u{2068}", "\u{2069}", "\u{FEFF}", ] - let filtered = String(text.unicodeScalars.filter { !dangerous.contains($0) }) + let filtered = String( + text.unicodeScalars.filter { scalar in + !dangerous.contains(scalar) + && !CharacterSet.controlCharacters.contains(scalar) + } + ) return filtered.trimmingCharacters(in: .whitespacesAndNewlines) }Also applies to: 5986-5995
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 5966 - 5981, The cmux config text sanitizer currently only strips BiDi/zero-width scalars; update the implementation of sanitizeCmuxConfigPaletteText so it also removes control characters (e.g., Cc like \r, \n and other control scalars) the same way the untrusted-text sanitizers in TerminalController.swift and BrowserPanel.swift do, ensuring both commandName and subtitle produced in the loop are cleaned of line breaks and control characters before being used in CommandPaletteCommandContribution.Sources/Workspace.swift (1)
891-916:⚠️ Potential issue | 🟠 MajorKeep startup commands queued until the terminal is ready or closed.
This still drops layout
commandexecution after an arbitrary 3s window.focus: falseterminals created during layout population can stay cold longer than that, so hidden-pane auto-run becomes race-dependent and silently breaks. Please remove the timeout/drop path and clean the observer up on panel teardown instead.Suggested direction
private func sendInputWhenReady(_ text: String, to panel: TerminalPanel) { if panel.surface.surface != nil { panel.sendInput(text) return } - var resolved = false - var observer: NSObjectProtocol? - - observer = NotificationCenter.default.addObserver( + if let existing = pendingStartupInputObserverByPanelId[panel.id] { + NotificationCenter.default.removeObserver(existing) + pendingStartupInputObserverByPanelId.removeValue(forKey: panel.id) + } + + let observer = NotificationCenter.default.addObserver( forName: .terminalSurfaceDidBecomeReady, object: panel.surface, queue: .main - ) { [weak panel] _ in - guard !resolved, let panel else { return } - resolved = true - if let observer { NotificationCenter.default.removeObserver(observer) } + ) { [weak self, weak panel] _ in + guard let self, let panel else { return } + if let observer = self.pendingStartupInputObserverByPanelId.removeValue(forKey: panel.id) { + NotificationCenter.default.removeObserver(observer) + } panel.sendInput(text) } - - DispatchQueue.main.asyncAfter(deadline: .now() + 3.0) { - guard !resolved else { return } - resolved = true - if let observer { NotificationCenter.default.removeObserver(observer) } - NSLog("[CmuxConfig] surface not ready after 3s, dropping command (%d chars)", text.count) - } + pendingStartupInputObserverByPanelId[panel.id] = observer }// Workspace private state private var pendingStartupInputObserverByPanelId: [UUID: NSObjectProtocol] = [:] // splitTabBar(_:didCloseTab:fromPane:) if let observer = pendingStartupInputObserverByPanelId.removeValue(forKey: panelId) { NotificationCenter.default.removeObserver(observer) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 891 - 916, The current sendInputWhenReady(_:to:) uses a 3s DispatchQueue timeout and drops commands; instead remove the timeout and keep the observer until the panel is closed or the surface becomes ready. Change sendInputWhenReady to create the NotificationCenter observer for .terminalSurfaceDidBecomeReady, store that observer in a workspace-level dictionary (e.g. pendingStartupInputObserverByPanelId: [UUID: NSObjectProtocol]) keyed by panel.id, and on notification call panel.sendInput(text), remove the observer from NotificationCenter and remove the entry from pendingStartupInputObserverByPanelId; remove the DispatchQueue.main.asyncAfter block and the "resolved" drop path. In the tab/pane teardown path (splitTabBar(_:didCloseTab:fromPane:) or equivalent panel-close handler) check pendingStartupInputObserverByPanelId for the panelId and, if present, removeObserver and remove the dictionary entry to ensure cleanup when the panel is closed.Sources/CmuxConfig.swift (2)
232-248:⚠️ Potential issue | 🟠 MajorPer-window config isolation is still missing.
The store uses instance properties (
localConfigPath,loadedCommands,cancellables) butwireDirectoryTrackingcallscancellables.removeAll(), tearing down any previous window's subscriptions. In a multi-window session, the last window to callwireDirectoryTrackingdictates which project's commands appear in all windows' palettes.This was flagged in a previous review and remains unresolved. Consider either:
- Making
CmuxConfigStorea per-window instance (owned by eachMainWindowContext)- Keying subscriptions and loaded commands by window/tabManager ID
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 232 - 248, CmuxConfigStore currently holds shared instance properties (localConfigPath, loadedCommands, cancellables) while wireDirectoryTracking mutates cancellables.removeAll(), causing different windows to clobber each other's subscriptions; fix by making CmuxConfigStore per-window (owned by MainWindowContext) or by namespacing state and subscriptions by window/tab ID: move localConfigPath/loadedCommands/cancellables into a window-scoped struct keyed by MainWindowContext or tabManager ID, update wireDirectoryTracking to operate on that keyed entry (attach/detach only that window's DispatchSource/FileDescriptor) and remove global shared mutations so each window manages its own subscriptions and loaded commands.
289-304:⚠️ Potential issue | 🟠 MajorFirst-time local
cmux.jsoncreation still not observed.When
findCmuxConfigreturnsnilandlocalConfigPathis alreadynil, Line 297 returns early. When they differ (e.g.,localConfigPathwas non-nil and nownewPathisnil), the code proceeds but Line 300 only starts a watcher ifnewPath != nil. Neither path starts a directory watcher to detect first-time file creation.A user who opens a workspace without
cmux.jsonand later creates one won't see their commands until they trigger a directory change or restart.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 289 - 304, updateLocalConfigPath currently skips starting a watcher when both newPath and localConfigPath are nil, so the extension never notices a first-time cmux.json creation; change updateLocalConfigPath to start a directory-level file watcher when findCmuxConfig returns nil but you still need to observe the directory for future cmux.json creation (i.e., when newPath == nil and previous localConfigPath == nil or when transitioning to nil), by calling startLocalFileWatcher (or a new startDirectoryWatcher helper) on the workspace/parent directory before returning, ensure stopLocalFileWatcher is still called when switching paths, and keep calling loadAll() only after watcher state and localConfigPath are updated (references: updateLocalConfigPath, findCmuxConfig, localConfigPath, stopLocalFileWatcher, startLocalFileWatcher, loadAll).
🧹 Nitpick comments (5)
Sources/AppDelegate.swift (1)
5789-5798: Avoid duplicating global config watchers per window.Line 5789 creates a fresh
CmuxConfigStorefor every main window. SinceCmuxConfigStore.init()starts a global watcher (Sources/CmuxConfig.swift:233-260), N windows means N watchers on the same~/.config/cmux/cmux.json, which can cause redundant reload work and extra FD usage. Consider sharing a single global watcher and fan-out updates to per-window stores (while keeping per-window local-directory tracking/state).Possible direction
- let cmuxConfigStore = CmuxConfigStore() + let cmuxConfigStore = CmuxConfigStore(globalWatcher: CmuxGlobalConfigWatcher.shared) cmuxConfigStore.wireDirectoryTracking(tabManager: tabManager) cmuxConfigStore.loadAll()Then keep local watchers in each store, but centralize only the global-file observer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/AppDelegate.swift` around lines 5789 - 5798, The code instantiates a new CmuxConfigStore per window which starts a global file watcher in CmuxConfigStore.init(), causing duplicate watchers; instead create or reuse a single shared/global CmuxConfigStore instance and inject that into each window’s view while keeping per-window local directory tracking via wireDirectoryTracking(tabManager:). Concretely, stop calling CmuxConfigStore() inside the window creation block and obtain a shared instance (e.g. CmuxConfigStore.shared or an AppDelegate-held cmuxConfigStoreGlobal) to .environmentObject into ContentView, and call wireDirectoryTracking(...) on a per-window store wrapper or on the tabManager-level per-window store so only the global watcher in CmuxConfigStore is created once while per-window state/tracking remains local.Sources/cmuxApp.swift (1)
351-352: Guard cmux config bootstrap so it only runs once.Line 351 and Line 352 are inside
onAppear; re-appearance can rewire subscriptions and reload configs again, causing unnecessaryconfigRevisionbumps and avoidable palette churn.♻️ Proposed change
`@StateObject` private var cmuxConfigStore = CmuxConfigStore() +@State private var didInitializeCmuxConfigStore = false ... .onAppear { ... - cmuxConfigStore.wireDirectoryTracking(tabManager: tabManager) - cmuxConfigStore.loadAll() + if !didInitializeCmuxConfigStore { + didInitializeCmuxConfigStore = true + cmuxConfigStore.wireDirectoryTracking(tabManager: tabManager) + cmuxConfigStore.loadAll() + } applyAppearance() ... }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` around lines 351 - 352, The cmux config bootstrap in onAppear (calls to cmuxConfigStore.wireDirectoryTracking(tabManager:) and cmuxConfigStore.loadAll()) must be guarded so it only runs once; add a stored Bool flag (e.g. hasBootstrappedCmuxConfig) at the view or app state level, check the flag in onAppear and only call wireDirectoryTracking(...) and loadAll() when the flag is false, then set the flag to true immediately after running to prevent re-wiring subscriptions and repeated configRevision bumps on subsequent appearances.web/app/[locale]/docs/custom-commands/page.tsx (1)
38-51: Consider using raw strings or escaping for JSON ellipsis.The JSON example uses
{ ... }which isn't valid JSON syntax. While this is intentional for documentation purposes to indicate "more content here," consider using a comment-style placeholder or a note to clarify this is illustrative.💡 Alternative: Use a more explicit placeholder
<CodeBlock title="cmux.json" lang="json">{`{ "commands": [ { "name": "Start Dev", "keywords": ["dev", "start"], - "workspace": { ... } + "workspace": { "..." : "see workspace commands below" } },Or simply add a note that the ellipsis represents omitted content.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/`[locale]/docs/custom-commands/page.tsx around lines 38 - 51, The JSON example in the CodeBlock using `{ ... }` is invalid; update the snippet inside the CodeBlock (title "cmux.json") to show valid JSON or a clear placeholder (e.g., "workspace": { "/* fields omitted */": true } or "workspace": { "placeholder": "…" }), or add an inline comment/note above the CodeBlock clarifying the ellipsis is illustrative so readers won’t copy invalid JSON; modify the CodeBlock content and/or nearby explanatory text accordingly.Sources/GhosttyTerminalView.swift (1)
3755-3781: Factor shared key-event initialization to reduce drift risk.Line 3757–3763 and Line 3772–3779 duplicate
ghostty_input_key_ssetup. A tiny shared initializer/helper would keep future field changes consistent.♻️ Suggested refactor
+ private func makeBaseKeyEvent(keycode: UInt32) -> ghostty_input_key_s { + var keyEvent = ghostty_input_key_s() + keyEvent.action = GHOSTTY_ACTION_PRESS + keyEvent.keycode = keycode + keyEvent.mods = GHOSTTY_MODS_NONE + keyEvent.consumed_mods = GHOSTTY_MODS_NONE + keyEvent.unshifted_codepoint = 0 + keyEvent.composing = false + return keyEvent + } + private func flushText(_ buffer: inout String, surface: ghostty_surface_t) { guard !buffer.isEmpty else { return } - var keyEvent = ghostty_input_key_s() - keyEvent.action = GHOSTTY_ACTION_PRESS - keyEvent.keycode = 0 - keyEvent.mods = GHOSTTY_MODS_NONE - keyEvent.consumed_mods = GHOSTTY_MODS_NONE - keyEvent.unshifted_codepoint = 0 - keyEvent.composing = false + var keyEvent = makeBaseKeyEvent(keycode: 0) buffer.withCString { ptr in keyEvent.text = ptr _ = ghostty_surface_key(surface, keyEvent) } buffer.removeAll(keepingCapacity: true) } private func sendKeyEvent(surface: ghostty_surface_t, keycode: UInt32) { - var keyEvent = ghostty_input_key_s() - keyEvent.action = GHOSTTY_ACTION_PRESS - keyEvent.keycode = keycode - keyEvent.mods = GHOSTTY_MODS_NONE - keyEvent.consumed_mods = GHOSTTY_MODS_NONE - keyEvent.unshifted_codepoint = 0 - keyEvent.composing = false + var keyEvent = makeBaseKeyEvent(keycode: keycode) keyEvent.text = nil _ = ghostty_surface_key(surface, keyEvent) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 3755 - 3781, Both flushText and sendKeyEvent duplicate initialization of ghostty_input_key_s; extract a small helper (e.g., makeKeyEvent or defaultKeyEvent) that returns a prefilled ghostty_input_key_s with action, keycode, mods, consumed_mods, unshifted_codepoint and composing set to the same defaults, then call that helper from flushText and sendKeyEvent and only override text or other per-call fields; reference the existing functions flushText(_:surface:) and sendKeyEvent(surface:keycode:) and the ghostty_input_key_s struct when adding the helper to ensure future field changes stay consistent.Sources/CmuxConfigExecutor.swift (1)
37-38: Silent no-op when no terminal is focused.When
focusedTerminalPanelreturnsnil(e.g., focused panel is a browser), the simple command silently does nothing. This is probably acceptable for a palette-invoked command, but consider whether a brief notification or sound would improve discoverability of the failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfigExecutor.swift` around lines 37 - 38, The guard that reads tabManager.selectedWorkspace?.focusedTerminalPanel silently returns when nil; update the else branch in CmuxConfigExecutor (the guard's failure path) to provide user feedback instead of a silent no-op — e.g., play a short sound with NSBeep() or present a brief UI notification/alert (NSUserNotification/NSAlert or your app's notification helper) informing the user that no terminal is focused before returning; keep the existing early-return behavior after notifying.
🤖 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/CmuxConfig.swift`:
- Around line 232-248: CmuxConfigStore currently holds shared instance
properties (localConfigPath, loadedCommands, cancellables) while
wireDirectoryTracking mutates cancellables.removeAll(), causing different
windows to clobber each other's subscriptions; fix by making CmuxConfigStore
per-window (owned by MainWindowContext) or by namespacing state and
subscriptions by window/tab ID: move localConfigPath/loadedCommands/cancellables
into a window-scoped struct keyed by MainWindowContext or tabManager ID, update
wireDirectoryTracking to operate on that keyed entry (attach/detach only that
window's DispatchSource/FileDescriptor) and remove global shared mutations so
each window manages its own subscriptions and loaded commands.
- Around line 289-304: updateLocalConfigPath currently skips starting a watcher
when both newPath and localConfigPath are nil, so the extension never notices a
first-time cmux.json creation; change updateLocalConfigPath to start a
directory-level file watcher when findCmuxConfig returns nil but you still need
to observe the directory for future cmux.json creation (i.e., when newPath ==
nil and previous localConfigPath == nil or when transitioning to nil), by
calling startLocalFileWatcher (or a new startDirectoryWatcher helper) on the
workspace/parent directory before returning, ensure stopLocalFileWatcher is
still called when switching paths, and keep calling loadAll() only after watcher
state and localConfigPath are updated (references: updateLocalConfigPath,
findCmuxConfig, localConfigPath, stopLocalFileWatcher, startLocalFileWatcher,
loadAll).
In `@Sources/ContentView.swift`:
- Around line 6326-6337: The code treats a workspace currentDirectory that
contains only whitespace as valid; update the baseCwd determination inside the
registry.register closure (where rawCwd is taken from
tabManager.selectedWorkspace?.currentDirectory and baseCwd is computed) to
consider whitespace-only strings as empty by trimming whitespace/newlines before
the isEmpty check and use the trimmed value when non-empty; if the trimmed
string is empty or nil, fall back to
FileManager.default.homeDirectoryForCurrentUser.path so relative cwd resolution
works for fresh or unsynced workspaces.
- Around line 5966-5981: The cmux config text sanitizer currently only strips
BiDi/zero-width scalars; update the implementation of
sanitizeCmuxConfigPaletteText so it also removes control characters (e.g., Cc
like \r, \n and other control scalars) the same way the untrusted-text
sanitizers in TerminalController.swift and BrowserPanel.swift do, ensuring both
commandName and subtitle produced in the loop are cleaned of line breaks and
control characters before being used in CommandPaletteCommandContribution.
In `@Sources/Workspace.swift`:
- Around line 891-916: The current sendInputWhenReady(_:to:) uses a 3s
DispatchQueue timeout and drops commands; instead remove the timeout and keep
the observer until the panel is closed or the surface becomes ready. Change
sendInputWhenReady to create the NotificationCenter observer for
.terminalSurfaceDidBecomeReady, store that observer in a workspace-level
dictionary (e.g. pendingStartupInputObserverByPanelId: [UUID: NSObjectProtocol])
keyed by panel.id, and on notification call panel.sendInput(text), remove the
observer from NotificationCenter and remove the entry from
pendingStartupInputObserverByPanelId; remove the DispatchQueue.main.asyncAfter
block and the "resolved" drop path. In the tab/pane teardown path
(splitTabBar(_:didCloseTab:fromPane:) or equivalent panel-close handler) check
pendingStartupInputObserverByPanelId for the panelId and, if present,
removeObserver and remove the dictionary entry to ensure cleanup when the panel
is closed.
---
Nitpick comments:
In `@Sources/AppDelegate.swift`:
- Around line 5789-5798: The code instantiates a new CmuxConfigStore per window
which starts a global file watcher in CmuxConfigStore.init(), causing duplicate
watchers; instead create or reuse a single shared/global CmuxConfigStore
instance and inject that into each window’s view while keeping per-window local
directory tracking via wireDirectoryTracking(tabManager:). Concretely, stop
calling CmuxConfigStore() inside the window creation block and obtain a shared
instance (e.g. CmuxConfigStore.shared or an AppDelegate-held
cmuxConfigStoreGlobal) to .environmentObject into ContentView, and call
wireDirectoryTracking(...) on a per-window store wrapper or on the
tabManager-level per-window store so only the global watcher in CmuxConfigStore
is created once while per-window state/tracking remains local.
In `@Sources/cmuxApp.swift`:
- Around line 351-352: The cmux config bootstrap in onAppear (calls to
cmuxConfigStore.wireDirectoryTracking(tabManager:) and
cmuxConfigStore.loadAll()) must be guarded so it only runs once; add a stored
Bool flag (e.g. hasBootstrappedCmuxConfig) at the view or app state level, check
the flag in onAppear and only call wireDirectoryTracking(...) and loadAll() when
the flag is false, then set the flag to true immediately after running to
prevent re-wiring subscriptions and repeated configRevision bumps on subsequent
appearances.
In `@Sources/CmuxConfigExecutor.swift`:
- Around line 37-38: The guard that reads
tabManager.selectedWorkspace?.focusedTerminalPanel silently returns when nil;
update the else branch in CmuxConfigExecutor (the guard's failure path) to
provide user feedback instead of a silent no-op — e.g., play a short sound with
NSBeep() or present a brief UI notification/alert (NSUserNotification/NSAlert or
your app's notification helper) informing the user that no terminal is focused
before returning; keep the existing early-return behavior after notifying.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 3755-3781: Both flushText and sendKeyEvent duplicate
initialization of ghostty_input_key_s; extract a small helper (e.g.,
makeKeyEvent or defaultKeyEvent) that returns a prefilled ghostty_input_key_s
with action, keycode, mods, consumed_mods, unshifted_codepoint and composing set
to the same defaults, then call that helper from flushText and sendKeyEvent and
only override text or other per-call fields; reference the existing functions
flushText(_:surface:) and sendKeyEvent(surface:keycode:) and the
ghostty_input_key_s struct when adding the helper to ensure future field changes
stay consistent.
In `@web/app/`[locale]/docs/custom-commands/page.tsx:
- Around line 38-51: The JSON example in the CodeBlock using `{ ... }` is
invalid; update the snippet inside the CodeBlock (title "cmux.json") to show
valid JSON or a clear placeholder (e.g., "workspace": { "/* fields omitted */":
true } or "workspace": { "placeholder": "…" }), or add an inline comment/note
above the CodeBlock clarifying the ellipsis is illustrative so readers won’t
copy invalid JSON; modify the CodeBlock content and/or nearby explanatory text
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 133b6e65-17b3-4548-bc01-19e002c803e4
📒 Files selected for processing (32)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigExecutor.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/CmuxConfigTests.swiftweb/app/[locale]/components/docs-nav-items.tsweb/app/[locale]/docs/custom-commands/page.tsxweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/CmuxConfig.swift (1)
217-230: Consider validating surface type constraints.
CmuxSurfaceDefinitionaccepts all optional fields regardless oftype. This could lead to confusing configurations:
- Browser surface without
url- Terminal surface with
urlset (ignored?)- Browser surface with
commandset (ignored?)Validating type-specific constraints at decode time would fail fast on misconfigured surfaces rather than silently ignoring fields.
♻️ Example validation in a custom decoder
struct CmuxSurfaceDefinition: Codable, Sendable { // ... existing fields ... init(from decoder: Decoder) throws { let container = try decoder.container(keyedBy: CodingKeys.self) type = try container.decode(CmuxSurfaceType.self, forKey: .type) name = try container.decodeIfPresent(String.self, forKey: .name) command = try container.decodeIfPresent(String.self, forKey: .command) cwd = try container.decodeIfPresent(String.self, forKey: .cwd) env = try container.decodeIfPresent([String: String].self, forKey: .env) url = try container.decodeIfPresent(String.self, forKey: .url) focus = try container.decodeIfPresent(Bool.self, forKey: .focus) switch type { case .browser: if url == nil || url?.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty == true { throw DecodingError.dataCorrupted( DecodingError.Context( codingPath: decoder.codingPath, debugDescription: "Browser surface requires a non-empty 'url'" ) ) } case .terminal: break // terminal surfaces are valid without url } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 217 - 230, CmuxSurfaceDefinition currently decodes all fields as optional regardless of CmuxSurfaceType; implement a custom init(from decoder: Decoder) on CmuxSurfaceDefinition to decode type and all fields then validate type-specific constraints (e.g., for CmuxSurfaceType.browser require a non-empty url, and for CmuxSurfaceType.terminal reject or warn if url is present or ensure ignored fields are explicit) and throw a DecodingError.dataCorrupted with a clear message when constraints fail; update CodingKeys and ensure the struct still conforms to Codable and Sendable so misconfigured surfaces fail fast at decode time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/CmuxConfig.swift`:
- Around line 217-230: CmuxSurfaceDefinition currently decodes all fields as
optional regardless of CmuxSurfaceType; implement a custom init(from decoder:
Decoder) on CmuxSurfaceDefinition to decode type and all fields then validate
type-specific constraints (e.g., for CmuxSurfaceType.browser require a non-empty
url, and for CmuxSurfaceType.terminal reject or warn if url is present or ensure
ignored fields are explicit) and throw a DecodingError.dataCorrupted with a
clear message when constraints fail; update CodingKeys and ensure the struct
still conforms to Codable and Sendable so misconfigured surfaces fail fast at
decode time.
2 tasks
XCUIApplication.launch() blocks ~60s then fails on headless WarpBuild runners because foreground activation requires a GUI login session. Apply the same pre-launch strategy used for the display resolution test: - CI shell launches the app with env vars before running xcodebuild - Test detects pre-launched app via manifest, uses activate() instead of launch() to avoid killing and relaunching the app - Falls back to clicking the window for focus via accessibility framework Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit a540e2f.
…d palette improvements
- Fix Danish ("Overfladedef inition") and Norwegian ("rotmapp") translation typos
- Add empty-string check for baseCwd fallback in command palette handlers
- Coalesce \r\n into single Return keypress in sendInput
- Redact command text from timeout log to prevent secret leakage
- Add decode-time validation: reject hybrid/empty commands, ambiguous layout
nodes, wrong split children count, and empty pane surfaces
- Namespace custom command IDs with "cmux.config.command." prefix
- Forward command description to palette subtitle when available
- Update tests for new validation rules and ID prefix
…validation, ancestor walk, palette sanitization
…ound in full acesor walk
lawrencecchen
force-pushed
the
cmux-json
branch
from
5219bf3 to
c42a4a4
Compare
The confirm dialog now shows the actual command text and has an "Always trust commands from this folder" checkbox. When checked, future confirm commands from that directory skip the dialog. Trust is scoped to the git repo root if the cmux.json is inside a repo, so trusting once covers all subdirectories. Non-git directories are trusted by exact path. Global config is always trusted. Trusted directories are persisted in ~/Library/Application Support/cmux/ trusted-directories.json.
Shows all trusted directories with per-directory revoke buttons and a Clear All option. Placed in a "Custom Commands" section between Automation and Browser in Settings.
There was a problem hiding this comment.
1 issue found across 2 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="Sources/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:3885">
P2: Trusted directories in Settings are backed by one-time local @State and are not synchronized with external trust-store updates, so the security list can become stale.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
One path per line, with a Save button that activates on changes. Users can add, remove, or edit paths directly.
Matches the behavior of other textarea settings (browser host whitelist, external URL patterns) which auto-save via @AppStorage.
There was a problem hiding this comment.
1 issue found across 2 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="Sources/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:5085">
P2: Per-keystroke `replaceAll` from a stale draft can silently drop trusted directories added elsewhere in the app.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .padding(.horizontal, 16) | ||
| .padding(.bottom, 12) | ||
| .onChange(of: trustedDirectoriesDraft) { _ in | ||
| saveTrustedDirectories() |
There was a problem hiding this comment.
P2: Per-keystroke replaceAll from a stale draft can silently drop trusted directories added elsewhere in the app.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/cmuxApp.swift, line 5085:
<comment>Per-keystroke `replaceAll` from a stale draft can silently drop trusted directories added elsewhere in the app.</comment>
<file context>
@@ -5052,44 +5060,34 @@ struct SettingsView: View {
+ .padding(.horizontal, 16)
+ .padding(.bottom, 12)
+ .onChange(of: trustedDirectoriesDraft) { _ in
+ saveTrustedDirectories()
}
- .padding(.horizontal, 14)
</file context>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
Sources/ContentView.swift (2)
6330-6346:⚠️ Potential issue | 🟠 MajorTreat whitespace-only
currentDirectoryas missing.Line 6336 only checks
isEmpty, so" "still gets passed toCmuxConfigExecutorasbaseCwd. That breaks relative path resolution for config-defined commands/workspaces on blank-but-not-nil workspaces.🛠️ Suggested fix
registry.register(commandId: command.id) { - let rawCwd = tabManager.selectedWorkspace?.currentDirectory - let baseCwd = (rawCwd?.isEmpty == false) ? rawCwd! - : FileManager.default.homeDirectoryForCurrentUser.path + let baseCwd: String = { + let candidate = tabManager.selectedWorkspace?.currentDirectory + .trimmingCharacters(in: .whitespacesAndNewlines) + guard let candidate, !candidate.isEmpty else { + return FileManager.default.homeDirectoryForCurrentUser.path + } + return candidate + }() CmuxConfigExecutor.execute(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 6330 - 6346, Replace the currentDirectory emptiness check with a trimmed check so whitespace-only paths are treated as missing: when building baseCwd inside the registry closure (the block that registers commandId for each item from cmuxConfigStore.loadedCommands and calls CmuxConfigExecutor.execute), obtain rawCwd via tabManager.selectedWorkspace?.currentDirectory, trim it with .trimmingCharacters(in: .whitespacesAndNewlines), and if the trimmed string is empty use FileManager.default.homeDirectoryForCurrentUser.path; otherwise use the trimmed value as baseCwd before calling CmuxConfigExecutor.execute with configSourcePath and globalConfigPath.
5970-5999:⚠️ Potential issue | 🟠 MajorStrip control characters here too.
Line 5997 only drops the BiDi/zero-width set.
\r,\n, and other control characters can still survive intocommand.name, which means a repo-sourced command can render a benign first line in the palette while hiding the destructive suffix after a line break. Please mirror the full untrusted-text filter before these strings reach the title/search corpus.🛡️ Possible hardening
private func sanitizeCmuxConfigPaletteText(_ text: String) -> String { let dangerous: Set<Unicode.Scalar> = [ "\u{200B}", "\u{200C}", "\u{200D}", "\u{200E}", "\u{200F}", "\u{202A}", "\u{202B}", "\u{202C}", "\u{202D}", "\u{202E}", "\u{2066}", "\u{2067}", "\u{2068}", "\u{2069}", "\u{FEFF}", ] - let filtered = String(text.unicodeScalars.filter { !dangerous.contains($0) }) + let filteredScalars = text.unicodeScalars.filter { scalar in + switch scalar.value { + case 0x00...0x1F, 0x7F...0x9F: + return false + default: + return !dangerous.contains(scalar) + } + } + let filtered = String(String.UnicodeScalarView(filteredScalars)) return filtered.trimmingCharacters(in: .whitespacesAndNewlines) }Based on learnings, the repo’s browser/terminal sanitizers strip both control characters and the dangerous-scalar set before trimming.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/ContentView.swift` around lines 5970 - 5999, sanitizeCmuxConfigPaletteText currently only removes BiDi/zero-width scalars but leaves C0 control characters (e.g. \r, \n, tab) which can hide malicious suffixes; update sanitizeCmuxConfigPaletteText (and callers that build CommandPaletteCommandContribution titles/subtitles) to also strip control characters – e.g. filter out scalars where scalar.properties.isControl or remove characters in CharacterSet.controlCharacters/newlines before trimming so the full untrusted-text filter used elsewhere (browser/terminal sanitizers) is mirrored here.Sources/CmuxConfig.swift (1)
292-307:⚠️ Potential issue | 🟠 MajorRecompute the nearest
cmux.jsoninstead of reloading the stale path.After Line 301 sets
localConfigPath, the local watcher callbacks onlyloadAll()or reattach that same path. Two cases stay wrong until the cwd changes: deleting/repo/sub/cmux.jsonnever falls back to/repo/cmux.json, and creating a new nearer/repo/sub/cmux.jsonwhile/repo/cmux.jsonis active is never discovered. Keep the active cwd and rerunfindCmuxConfig(startingFrom:)on local create/delete/rename events before reattaching.Also applies to: 393-400, 430-438, 450-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 292 - 307, The file-watcher currently reattaches to the previously stored localConfigPath and calls loadAll(), which misses cases where a nearer cmux.json is created or the nearest one is deleted; update the watcher event handlers (the code paths that call stopLocalFileWatcher(), set localConfigPath, startLocalFileWatcher(), and loadAll(), e.g., in updateLocalConfigPath and the analogous blocks around lines 393-400, 430-438, 450-463) to recompute the nearest config by calling findCmuxConfig(startingFrom: currentWorkingDirectory) on create/delete/rename events before setting localConfigPath — if the recomputed path differs, stop the watcher, set localConfigPath to the recomputed value (or nil), restart the watcher only if non-nil, and then call loadAll() so the active config always reflects the nearest cmux.json.
🧹 Nitpick comments (1)
web/messages/bs.json (1)
305-368: Minor spelling inconsistency in Bosnian translation.Line 349 uses "Točno" which is the Croatian spelling. For consistency with the rest of the file (which uses Bosnian conventions like "funkcioniše" on line 75), consider using "Tačno":
- "splitChildren": "Točno dva dječja čvora (podjela ili panel)", + "splitChildren": "Tačno dva dječija čvora (podjela ili panel)",Otherwise, the Custom Commands documentation strings are complete and well-structured.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/messages/bs.json` around lines 305 - 368, The string value for the "splitChildren" key uses Croatian spelling "Točno dva dječja čvora (podjela ili panel)"; update it to Bosnian spelling "Tačno dva dječja čvora (podjela ili panel)" so it matches the rest of the Bosnian localization; locate the "splitChildren" entry in the provided diff and replace "Točno" with "Tačno".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/CmuxConfig.swift`:
- Around line 93-98: The CmuxWorkspaceDefinition decoder currently allows any
string for the color field, so update decoding/validation for
CmuxWorkspaceDefinition.color to reject values that are not accepted by
WorkspaceTabColorSettings.normalizedHex(...) (only allow 6-digit hex "#RRGGBB"
with no alpha); during init(from:) or a custom decoding step, call
WorkspaceTabColorSettings.normalizedHex(colorString) and throw a DecodingError
if it returns nil, so downstream code (e.g., CmuxConfigExecutor ->
newWorkspace.setCustomColor(...)) never receives invalid colors.
In `@Sources/CmuxConfigExecutor.swift`:
- Around line 40-44: The confirm dialog currently interpolates raw command text
into CmuxConfigExecutor (alert.informativeText = String(format: messageFormat,
command)); update this to first sanitize the command by removing BiDi override
and zero-width characters using the same dangerousScalars filter used in
TerminalController.swift and BrowserPanel.swift (refer to their dangerousScalars
Set<UInt32> and removal logic), then use the sanitized string in the
String(format:) call so the preview shown matches the actual run-time command.
---
Duplicate comments:
In `@Sources/CmuxConfig.swift`:
- Around line 292-307: The file-watcher currently reattaches to the previously
stored localConfigPath and calls loadAll(), which misses cases where a nearer
cmux.json is created or the nearest one is deleted; update the watcher event
handlers (the code paths that call stopLocalFileWatcher(), set localConfigPath,
startLocalFileWatcher(), and loadAll(), e.g., in updateLocalConfigPath and the
analogous blocks around lines 393-400, 430-438, 450-463) to recompute the
nearest config by calling findCmuxConfig(startingFrom: currentWorkingDirectory)
on create/delete/rename events before setting localConfigPath — if the
recomputed path differs, stop the watcher, set localConfigPath to the recomputed
value (or nil), restart the watcher only if non-nil, and then call loadAll() so
the active config always reflects the nearest cmux.json.
In `@Sources/ContentView.swift`:
- Around line 6330-6346: Replace the currentDirectory emptiness check with a
trimmed check so whitespace-only paths are treated as missing: when building
baseCwd inside the registry closure (the block that registers commandId for each
item from cmuxConfigStore.loadedCommands and calls CmuxConfigExecutor.execute),
obtain rawCwd via tabManager.selectedWorkspace?.currentDirectory, trim it with
.trimmingCharacters(in: .whitespacesAndNewlines), and if the trimmed string is
empty use FileManager.default.homeDirectoryForCurrentUser.path; otherwise use
the trimmed value as baseCwd before calling CmuxConfigExecutor.execute with
configSourcePath and globalConfigPath.
- Around line 5970-5999: sanitizeCmuxConfigPaletteText currently only removes
BiDi/zero-width scalars but leaves C0 control characters (e.g. \r, \n, tab)
which can hide malicious suffixes; update sanitizeCmuxConfigPaletteText (and
callers that build CommandPaletteCommandContribution titles/subtitles) to also
strip control characters – e.g. filter out scalars where
scalar.properties.isControl or remove characters in
CharacterSet.controlCharacters/newlines before trimming so the full
untrusted-text filter used elsewhere (browser/terminal sanitizers) is mirrored
here.
---
Nitpick comments:
In `@web/messages/bs.json`:
- Around line 305-368: The string value for the "splitChildren" key uses
Croatian spelling "Točno dva dječja čvora (podjela ili panel)"; update it to
Bosnian spelling "Tačno dva dječja čvora (podjela ili panel)" so it matches the
rest of the Bosnian localization; locate the "splitChildren" entry in the
provided diff and replace "Točno" with "Tačno".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 240094da-1a26-4f3d-a863-5af4a2c6448f
📒 Files selected for processing (33)
GhosttyTabs.xcodeproj/project.pbxprojResources/Localizable.xcstringsSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigExecutor.swiftSources/CmuxDirectoryTrust.swiftSources/ContentView.swiftSources/GhosttyTerminalView.swiftSources/Panels/TerminalPanel.swiftSources/Workspace.swiftSources/cmuxApp.swiftcmuxTests/CmuxConfigTests.swiftweb/app/[locale]/components/docs-nav-items.tsweb/app/[locale]/docs/custom-commands/page.tsxweb/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/en.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/ja.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
✅ Files skipped from review due to trivial changes (14)
- web/app/[locale]/components/docs-nav-items.ts
- web/messages/it.json
- web/messages/fr.json
- web/messages/ko.json
- web/messages/th.json
- web/messages/pt-BR.json
- web/messages/ar.json
- web/messages/ru.json
- Resources/Localizable.xcstrings
- web/messages/no.json
- web/messages/da.json
- web/messages/es.json
- cmuxTests/CmuxConfigTests.swift
- web/messages/ja.json
🚧 Files skipped from review as they are similar to previous changes (11)
- Sources/Panels/TerminalPanel.swift
- web/app/[locale]/docs/custom-commands/page.tsx
- web/messages/zh-CN.json
- web/messages/zh-TW.json
- GhosttyTabs.xcodeproj/project.pbxproj
- Sources/cmuxApp.swift
- Sources/AppDelegate.swift
- web/messages/tr.json
- Sources/GhosttyTerminalView.swift
- Sources/Workspace.swift
- web/messages/en.json
Comment on lines
+93
to
+98
| struct CmuxWorkspaceDefinition: Codable, Sendable { | ||
| var name: String? | ||
| var cwd: String? | ||
| var color: String? | ||
| var layout: CmuxLayoutNode? | ||
| } |
There was a problem hiding this comment.
Reject invalid workspace.color values during decode.
CmuxWorkspaceDefinition.color accepts any string, and Sources/CmuxConfigExecutor.swift Lines 112-113 later pass it straight to newWorkspace.setCustomColor(...). That creates a config-only path that is looser than the rest of the app’s workspace-color validation. Fail fast here unless WorkspaceTabColorSettings.normalizedHex(...) accepts the value.
Based on learnings: When validating or reporting workspace/tab colors in this repo, only accept and use 6-digit hex colors in the form #RRGGBB (no alpha, i.e., do not allow #RRGGBBAA).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/CmuxConfig.swift` around lines 93 - 98, The CmuxWorkspaceDefinition
decoder currently allows any string for the color field, so update
decoding/validation for CmuxWorkspaceDefinition.color to reject values that are
not accepted by WorkspaceTabColorSettings.normalizedHex(...) (only allow 6-digit
hex "#RRGGBB" with no alpha); during init(from:) or a custom decoding step, call
WorkspaceTabColorSettings.normalizedHex(colorString) and throw a DecodingError
if it returns nil, so downstream code (e.g., CmuxConfigExecutor ->
newWorkspace.setCustomColor(...)) never receives invalid colors.
Strip zero-width and BiDi override characters from the command preview so the dialog shows exactly what will be executed.
There was a problem hiding this comment.
1 issue found across 1 file (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="Sources/CmuxConfigExecutor.swift">
<violation number="1" location="Sources/CmuxConfigExecutor.swift:44">
P2: Confirmation dialog shows a sanitized command, but execution runs the raw command, creating a display/consent mismatch for untrusted config commands.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| localized: "dialog.cmuxConfig.confirmCommand.messageWithCommand", | ||
| defaultValue: "This will run the following command:\n\n%@" | ||
| ) | ||
| alert.informativeText = String(format: messageFormat, sanitizeForDisplay(command)) |
There was a problem hiding this comment.
P2: Confirmation dialog shows a sanitized command, but execution runs the raw command, creating a display/consent mismatch for untrusted config commands.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/CmuxConfigExecutor.swift, line 44:
<comment>Confirmation dialog shows a sanitized command, but execution runs the raw command, creating a display/consent mismatch for untrusted config commands.</comment>
<file context>
@@ -41,7 +41,7 @@ struct CmuxConfigExecutor {
defaultValue: "This will run the following command:\n\n%@"
)
- alert.informativeText = String(format: messageFormat, command)
+ alert.informativeText = String(format: messageFormat, sanitizeForDisplay(command))
alert.alertStyle = .warning
alert.addButton(withTitle: String(
</file context>
Contributor
|
Thank you for the contribution! |
Jesssullivan
added a commit
to Jesssullivan/cmux
that referenced
this pull request
Mar 26, 2026
Ingests all upstream fixes since 2026-03-22 including: - Fix Cmd+N crash: retain snapshot workspaces (manaflow-ai#2183, manaflow-ai#2181, manaflow-ai#2178, manaflow-ai#2173) - Fix browser pane restore after reopen (manaflow-ai#2141) - Fix Ghostty resize_split keybind (manaflow-ai#1899) - Reduce shell integration prompt latency (manaflow-ai#2109) - Fix command palette focus after terminal find (manaflow-ai#2089) - Add Codex CLI hooks (manaflow-ai#2103) - Add cmux.json custom commands (manaflow-ai#2011) - Fix window position restore on relaunch (manaflow-ai#2129) Conflict resolution: - BrowserPanel.swift: accepted upstream configureWebViewConfiguration() refactor (already includes our forMainFrameOnly:true CAPTCHA fix from PR manaflow-ai#1877) Fork-specific files preserved: - Sources/Panels/WebAuthn{Coordinator,BridgeJavaScript}.swift - Sources/FIDO2/module.modulemap - vendor/ctap2 submodule - cmux.entitlements (with camera/audio-input removed) - cmux.embedded.entitlements - .github/workflows/fork-{ci,release}.yml
6 tasks
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
* Pre-launch app for browser UI test on headless CI runners XCUIApplication.launch() blocks ~60s then fails on headless WarpBuild runners because foreground activation requires a GUI login session. Apply the same pre-launch strategy used for the display resolution test: - CI shell launches the app with env vars before running xcodebuild - Test detects pre-launched app via manifest, uses activate() instead of launch() to avoid killing and relaunching the app - Falls back to clicking the window for focus via accessibility framework Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert "Pre-launch app for browser UI test on headless CI runners" This reverts commit a540e2f. * feat: cmux.json for custom commands * tests: add cmux json tests * fix: pr review feedback: validation, translations, input handling, and palette improvements - Fix Danish ("Overfladedef inition") and Norwegian ("rotmapp") translation typos - Add empty-string check for baseCwd fallback in command palette handlers - Coalesce \r\n into single Return keypress in sendInput - Redact command text from timeout log to prevent secret leakage - Add decode-time validation: reject hybrid/empty commands, ambiguous layout nodes, wrong split children count, and empty pane surfaces - Namespace custom command IDs with "cmux.config.command." prefix - Forward command description to palette subtitle when available - Update tests for new validation rules and ID prefix * fix: address PR review feedback — per-window config isolation, blank validation, ancestor walk, palette sanitization * fix: fallback to current dir cmux.json watching if no any cmux.json found in full acesor walk * ci: trigger CI for fork PR * Add directory trust for cmux.json command confirmation The confirm dialog now shows the actual command text and has an "Always trust commands from this folder" checkbox. When checked, future confirm commands from that directory skip the dialog. Trust is scoped to the git repo root if the cmux.json is inside a repo, so trusting once covers all subdirectories. Non-git directories are trusted by exact path. Global config is always trusted. Trusted directories are persisted in ~/Library/Application Support/cmux/ trusted-directories.json. * Add trusted directories section to Settings Shows all trusted directories with per-directory revoke buttons and a Clear All option. Placed in a "Custom Commands" section between Automation and Browser in Settings. * Replace trusted directories list with editable textarea One path per line, with a Save button that activates on changes. Users can add, remove, or edit paths directly. * Auto-save trusted directories on edit, remove Save button Matches the behavior of other textarea settings (browser host whitelist, external URL patterns) which auto-save via @AppStorage. * Sanitize command text in confirm dialog against BiDi attacks Strip zero-width and BiDi override characters from the command preview so the dialog shows exactly what will be executed. --------- Co-authored-by: austinpower1258 <austinwang115@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cmux.jsonconfiguration support for defining custom commands and workspace layoutscmux.jsonin their project root (per-project) or~/.config/cmux/cmux.json(global) to define commands that appear in the command palette
split layouts, browser panes, named tabs, env vars, and auto-run commands
(local takes precedence), and restart behavior control (
ignore/recreate/confirm)supported languages
Testing
cmux.jsonparsing with simple commands and workspace layout definitionscommands
ignoreswitches to existing workspace,recreatecloses and recreates,confirmshows dialogcmux.jsonupdates commands in real timeDemo Video
Screen.Recording.2026-03-24.at.1.58.09.AM.mov
Checklist
Summary by cubic
Adds support for
cmux.jsonto define custom commands and workspace layouts with live reload and command palette integration. Adds directory trust with an auto-saving Settings editor so confirm-required shell commands from trusted paths run without future prompts.New Features
./cmux.json(per-project) and~/.config/cmux/cmux.json(global); walks up from active tab cwd; local overrides; per-window isolation; if none found, watches current dir.ignore,recreate,confirm).baseCwdfallback, mixed input handling that coalesces CRLF.~/Library/Application Support/cmux/trusted-directories.json; manage in Settings via an editable textarea that auto-saves./docs/custom-commands; tests for parsing, layout, validation, ID rules; redact command text in timeout logs.Migration
cmux.jsonin your project root or~/.config/cmux/to start using custom commands. You can trust a folder once to skip future confirms. No app restart needed.Written for commit c9953d1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests