Repository navigation
Workspace layouts: save/delete/customize from the + menu, default layout for new workspaces, inline workspace actions, open agent kinds - #7354
Merged
Conversation
…ce, open agent kinds - actions can now define type "workspace" with an inline workspace definition (name/cwd/color/env/layout) plus optional restart behavior, no separate commands entry needed - workspace definitions support "setup": a bootstrap command sent to the workspace's first terminal ahead of that terminal's own command - agent actions accept any CLI name (claude, codex, opencode, or custom binaries) instead of a hardcoded enum; opencode gets presentation defaults - workspace actions are auto-offered in the plus-button context menu; newWorkspaceMenu true/false overrides per action - "Save Workspace as Action…" in the plus-button menu captures the live split tree, per-panel cwds, detected agent CLIs, browser URLs, and project panels into ~/.config/cmux/cmux.json, preserving JSONC comments via JSONCObjectEditor; "Customize Actions…" opens the config - surface tab bar buttons support inline workspace actions through the same synthetic-command execution and trust path - schema descriptions, custom-commands docs (en+ja), and Localizable strings (en+ja) updated; adds CmuxConfigWorkspaceActionTests (18) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
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:
📝 WalkthroughWalkthroughThis PR adds inline workspace actions, workspace snapshot/save support, execution routing, menu wiring, and related docs/tests/localization so workspaces can be saved into and restored from ChangesWorkspace Action Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant AppDelegate
participant Workspace
participant CmuxConfigActionSaver
participant CmuxConfigExecutor
User->>AppDelegate: Save Workspace as Action…
AppDelegate->>Workspace: captureConfigActionSnapshot()
Workspace-->>AppDelegate: workspace definition snapshot
AppDelegate->>CmuxConfigActionSaver: saveWorkspaceAction(...)
CmuxConfigActionSaver-->>AppDelegate: SaveResult
User->>CmuxConfigExecutor: run inline workspace action
CmuxConfigExecutor->>Workspace: apply custom layout / setup
Workspace-->>CmuxConfigExecutor: workspace ready
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors)
✅ Passed checks (22 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR adds workspace layout save/restore to cmux: users can capture a live workspace (splits, cwds, agent CLIs, browser URLs) into
Confidence Score: 4/5Safe to merge after addressing the TerminalForegroundCommandCapture caseless enum; the JSONC editing, trust path, and localization are well-implemented. The new TerminalForegroundCommandCapture type is a caseless enum with only static functions — a pure namespace — following the same pattern that was already flagged for CmuxConfigActionSaver in the prior round. The repo rules treat this as a blocker for production Swift. Everything else — the JSONC comment-preserving editors, the temp+rename write path, the trust disclosure, the synthetic-command execution route, and the en+ja localization — is correct and well-structured. Sources/TerminalForegroundCommandCapture.swift (caseless enum namespace) and Sources/WorkspaceConfigActionCapture.swift (synchronous process scan on MainActor before dialog) Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant AppDelegate
participant Workspace
participant TerminalForegroundCommandCapture
participant CmuxConfigActionSaver
participant CmuxConfigExecutor
participant CmuxConfigStore
User->>AppDelegate: Right-click + button
AppDelegate->>CmuxConfigStore: newWorkspaceContextMenuItems
AppDelegate->>User: Show context menu
User->>AppDelegate: Save Workspace as Layout
AppDelegate->>Workspace: captureConfigActionSnapshot() [MainActor]
Workspace->>TerminalForegroundCommandCapture: liveCommands(forTTYDevices:) [MainActor]
TerminalForegroundCommandCapture-->>Workspace: [Int64: argv string]
Workspace-->>AppDelegate: WorkspaceConfigActionSnapshot
AppDelegate->>User: NSAlert (name field + make-default checkbox)
User->>AppDelegate: Save (with name)
AppDelegate->>CmuxConfigActionSaver: saveWorkspaceAction() [sync file I/O]
CmuxConfigActionSaver-->>AppDelegate: SaveResult(actionID)
opt makeDefault checked
AppDelegate->>CmuxConfigActionSaver: setNewWorkspaceDefaultAction()
end
AppDelegate->>CmuxConfigStore: loadAll()
User->>AppDelegate: Click saved layout action
AppDelegate->>CmuxConfigExecutor: execute(action:)
CmuxConfigExecutor->>CmuxConfigExecutor: inlineWorkspaceSyntheticCommand
CmuxConfigExecutor->>Workspace: applyCustomLayout(layout:baseCwd:setupCommand:)
Workspace-->>User: New workspace with saved layout
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant AppDelegate
participant Workspace
participant TerminalForegroundCommandCapture
participant CmuxConfigActionSaver
participant CmuxConfigExecutor
participant CmuxConfigStore
User->>AppDelegate: Right-click + button
AppDelegate->>CmuxConfigStore: newWorkspaceContextMenuItems
AppDelegate->>User: Show context menu
User->>AppDelegate: Save Workspace as Layout
AppDelegate->>Workspace: captureConfigActionSnapshot() [MainActor]
Workspace->>TerminalForegroundCommandCapture: liveCommands(forTTYDevices:) [MainActor]
TerminalForegroundCommandCapture-->>Workspace: [Int64: argv string]
Workspace-->>AppDelegate: WorkspaceConfigActionSnapshot
AppDelegate->>User: NSAlert (name field + make-default checkbox)
User->>AppDelegate: Save (with name)
AppDelegate->>CmuxConfigActionSaver: saveWorkspaceAction() [sync file I/O]
CmuxConfigActionSaver-->>AppDelegate: SaveResult(actionID)
opt makeDefault checked
AppDelegate->>CmuxConfigActionSaver: setNewWorkspaceDefaultAction()
end
AppDelegate->>CmuxConfigStore: loadAll()
User->>AppDelegate: Click saved layout action
AppDelegate->>CmuxConfigExecutor: execute(action:)
CmuxConfigExecutor->>CmuxConfigExecutor: inlineWorkspaceSyntheticCommand
CmuxConfigExecutor->>Workspace: applyCustomLayout(layout:baseCwd:setupCommand:)
Workspace-->>User: New workspace with saved layout
Reviews (27): Last reviewed commit: "Clarify the save menu item: Save Workspa..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case let agentSession as AgentSessionPanel: | ||
| var surface = CmuxSurfaceDefinition(type: .terminal) | ||
| surface.name = customName | ||
| surface.command = agentSession.currentProviderID.rawValue |
Contributor
There was a problem hiding this comment.
rawValue used as CLI name instead of executableName
AgentSessionProviderID.rawValue happens to equal executableName today ("codex", "claude", "opencode"), but the type already exposes executableName as an explicit, case-by-case mapping precisely to decouple the two. Using .rawValue ties the saved command to the enum case name by coincidence; if a future provider's rawValue diverges from its CLI binary name the capture will silently persist a non-executable command string, breaking saved actions with no error at capture time. The TerminalPanel branch in the same file deliberately calls agent.kind.rawValue on a type where rawValue is the CLI contract — AgentSessionProviderID has a separate executableName for exactly this reason.
Suggested change
| surface.command = agentSession.currentProviderID.rawValue | |
| surface.command = agentSession.currentProviderID.executableName |
Contributor
Author
There was a problem hiding this comment.
Fixed in c982b3b — capture now uses currentProviderID.executableName.
Contributor
Author
There was a problem hiding this comment.
Fixed in an earlier commit on this branch: the capture path now persists agentSession.currentProviderID.executableName (Sources/WorkspaceConfigActionCapture.swift:205).
— Claude Code
Greptile review on #7354: AgentSessionProviderID.rawValue only coincidentally equals the CLI binary name; executableName is the explicit CLI mapping, so captured workspace actions keep launching if a future provider's case name diverges from its binary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
workflow-guard-tests failed the Swift file length budget after this PR grew four files past their tracked budgets. Move the new (and directly related) code into new files instead of refreshing budgets: - CmuxConfigActionDefinition -> Sources/CmuxConfigActionDefinition.swift - executeWorkspaceCommand -> Sources/CmuxConfigExecutor+WorkspaceLaunch.swift (private -> internal for cross-file access; behavior unchanged) - custom layout builders -> Sources/Workspace+CustomLayout.swift (sendInputWhenReady private -> internal; observer plumbing stays put) - Save Workspace as Action menu items/dialog -> Sources/AppDelegate+WorkspaceActionSave.swift All moves verbatim; new files wired into the app target in project.pbxproj and normalized. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # Resources/Localizable.xcstrings
…no-op Codex review on #7354: type "workspace" buttons render in the surface tab bar but applySurfaceTabBarButtons never registers them in surfaceTabBarCommandButtons, so didRequestCustomAction returns before the inline-workspace execution branch. Test drives the real click entrypoint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Register inline workspace buttons in surfaceTabBarCommandButtons so surface-tab-bar clicks actually execute them (makes the new regression test green). - Treat inline workspace actions as workspace-creating in executeConfiguredNewWorkspaceActionIfAvailable so the throwaway initial workspace is retired, matching named workspaceCommand behavior. - "Save Workspace as Action" now awaits a fresh SharedLiveAgentIndex (new waitForFreshIndex()) before capturing, so running agents aren't silently dropped when the cache is cold or stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/AppDelegate`+WorkspaceActionSave.swift:
- Around line 57-79: The workspace environment is being persisted as part of the
saved action, so secret values can end up in the shared config file. Update the
save flow in AppDelegate+WorkspaceActionSave so the data copied into
CmuxWorkspaceDefinition.env is filtered/redacted before writing, using the
workspaceEnvironment source from the captured snapshot. Add handling for
secret-shaped keys (or a warning/confirmation step) in the save action path
around captureConfigActionSnapshot and the alert/save logic so sensitive env
vars are not stored in plain text.
In `@Sources/CmuxConfigActionDefinition.swift`:
- Around line 84-150: Add ambiguity validation in
CmuxConfigActionDefinition.init(from:) before using inferredType when type is
omitted. Right now the inferredType priority chain (agent, builtin, workspace,
command) can silently ignore other present action keys, so mirror the sibling
decoder’s definedActionForms check from CmuxConfig.swift and throw a decoding
error if more than one action-defining key is set. Keep the existing switch on
inferredType, but only after confirming there is exactly one valid action form,
including the new workspace case.
In `@Sources/Workspace`+CustomLayout.swift:
- Around line 67-69: Replace the direct NSLog diagnostic in
Workspace+CustomLayout’s split-node validation with the shared cmuxDebugLog
helper to match the app/runtime logging convention. Keep the existing DEBUG-only
guard and the same message context, but route the count information through
cmuxDebugLog instead of emitting NSLog from the split.children check.
In `@Sources/WorkspaceConfigActionCapture.swift`:
- Around line 98-102: The captured command is using the agent kind value instead
of the actual executable name, which can diverge from the session launch path.
Update WorkspaceConfigActionCapture to set the saved command from the agent’s
executable name rather than agent.kind.rawValue, aligning it with
AgentSessionPanel’s currentProviderID.executableName and the live-agent snapshot
flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 786fa2b7-eeaa-4887-8a7d-68565afe661f
📒 Files selected for processing (18)
Resources/Localizable.xcstringsSources/AppDelegate+WorkspaceActionSave.swiftSources/AppDelegate.swiftSources/CmuxConfig.swiftSources/CmuxConfigActionDefinition.swiftSources/CmuxConfigActionSaver.swiftSources/CmuxConfigExecutor+WorkspaceLaunch.swiftSources/CmuxConfigExecutor.swiftSources/CmuxWorkspaceDefinition.swiftSources/Workspace+CustomLayout.swiftSources/Workspace.swiftSources/WorkspaceConfigActionCapture.swiftcmux.xcodeproj/project.pbxprojcmuxTests/CmuxConfigWorkspaceActionTests.swiftweb/app/[locale]/docs/custom-commands/page.tsxweb/data/cmux.schema.jsonweb/messages/en.jsonweb/messages/ja.json
Comment on lines
+84
to
+150
| let inferredType: String? | ||
| if let type { | ||
| inferredType = type | ||
| } else if container.contains(.agent) { | ||
| inferredType = "agent" | ||
| } else if container.contains(.builtin) { | ||
| inferredType = "builtin" | ||
| } else if container.contains(.workspace) { | ||
| inferredType = "workspace" | ||
| } else if container.contains(.command) { | ||
| inferredType = "command" | ||
| } else { | ||
| inferredType = nil | ||
| } | ||
|
|
||
| switch inferredType { | ||
| case "builtin": | ||
| let raw = try Self.trimmedString(forKey: .builtin, in: container) ?? "" | ||
| guard let builtIn = CmuxSurfaceTabBarBuiltInAction(configID: raw) else { | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: .builtin, | ||
| in: container, | ||
| debugDescription: "Unknown built-in action '\(raw)'" | ||
| ) | ||
| } | ||
| action = .builtIn(builtIn) | ||
| case "command": | ||
| let command = try Self.requiredTrimmedString(forKey: .command, in: container) | ||
| action = .command(command) | ||
| case "agent": | ||
| let agent = try container.decode(CmuxConfigAgentKind.self, forKey: .agent) | ||
| let args = try Self.trimmedString(forKey: .args, in: container, allowBlankAsNil: true) | ||
| action = .agent(agent, args: args) | ||
| case "workspaceCommand": | ||
| let commandName = try Self.trimmedString(forKey: .commandName, in: container) | ||
| ?? Self.trimmedString(forKey: .name, in: container) | ||
| ?? Self.trimmedString(forKey: .command, in: container) | ||
| guard let commandName else { | ||
| throw DecodingError.dataCorrupted( | ||
| DecodingError.Context( | ||
| codingPath: decoder.codingPath, | ||
| debugDescription: "workspaceCommand actions require commandName" | ||
| ) | ||
| ) | ||
| } | ||
| action = .workspaceCommand(commandName) | ||
| case "workspace": | ||
| guard container.contains(.workspace) else { | ||
| throw DecodingError.dataCorrupted( | ||
| DecodingError.Context( | ||
| codingPath: decoder.codingPath, | ||
| debugDescription: "workspace actions require a 'workspace' object" | ||
| ) | ||
| ) | ||
| } | ||
| let definition = try container.decode(CmuxWorkspaceDefinition.self, forKey: .workspace) | ||
| let restart = try container.decodeIfPresent(CmuxRestartBehavior.self, forKey: .restart) | ||
| action = .workspace(definition, restart: restart) | ||
| case nil: | ||
| action = nil | ||
| default: | ||
| throw DecodingError.dataCorruptedError( | ||
| forKey: .type, | ||
| in: container, | ||
| debugDescription: "Unknown action type '\(inferredType ?? "")'" | ||
| ) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add ambiguity validation when type is omitted.
When type is absent, inferredType picks the first matching key by priority (agent > builtin > workspace > command) without checking whether more than one action-defining key is present. A config accidentally mixing e.g. agent and workspace keys silently resolves to agent, silently discarding the workspace payload with no error surfaced to the user. CmuxSurfaceTabBarButton.init(from:) (Sources/CmuxConfig.swift, definedActionForms check) already guards against this exact ambiguity for the sibling decoder — this file lacks the equivalent check, and the new workspace case widens the collision surface.
♻️ Proposed fix
let inferredType: String?
if let type {
inferredType = type
- } else if container.contains(.agent) {
- inferredType = "agent"
- } else if container.contains(.builtin) {
- inferredType = "builtin"
- } else if container.contains(.workspace) {
- inferredType = "workspace"
- } else if container.contains(.command) {
- inferredType = "command"
- } else {
- inferredType = nil
+ } else {
+ let definedForms = [
+ container.contains(.agent),
+ container.contains(.builtin),
+ container.contains(.workspace),
+ container.contains(.command)
+ ].filter(\.self).count
+ guard definedForms <= 1 else {
+ throw DecodingError.dataCorrupted(
+ DecodingError.Context(
+ codingPath: decoder.codingPath,
+ debugDescription: "action definitions must define only one of 'agent', 'builtin', 'workspace', or 'command' when 'type' is omitted"
+ )
+ )
+ }
+ if container.contains(.agent) {
+ inferredType = "agent"
+ } else if container.contains(.builtin) {
+ inferredType = "builtin"
+ } else if container.contains(.workspace) {
+ inferredType = "workspace"
+ } else if container.contains(.command) {
+ inferredType = "command"
+ } else {
+ inferredType = nil
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let inferredType: String? | |
| if let type { | |
| inferredType = type | |
| } else if container.contains(.agent) { | |
| inferredType = "agent" | |
| } else if container.contains(.builtin) { | |
| inferredType = "builtin" | |
| } else if container.contains(.workspace) { | |
| inferredType = "workspace" | |
| } else if container.contains(.command) { | |
| inferredType = "command" | |
| } else { | |
| inferredType = nil | |
| } | |
| switch inferredType { | |
| case "builtin": | |
| let raw = try Self.trimmedString(forKey: .builtin, in: container) ?? "" | |
| guard let builtIn = CmuxSurfaceTabBarBuiltInAction(configID: raw) else { | |
| throw DecodingError.dataCorruptedError( | |
| forKey: .builtin, | |
| in: container, | |
| debugDescription: "Unknown built-in action '\(raw)'" | |
| ) | |
| } | |
| action = .builtIn(builtIn) | |
| case "command": | |
| let command = try Self.requiredTrimmedString(forKey: .command, in: container) | |
| action = .command(command) | |
| case "agent": | |
| let agent = try container.decode(CmuxConfigAgentKind.self, forKey: .agent) | |
| let args = try Self.trimmedString(forKey: .args, in: container, allowBlankAsNil: true) | |
| action = .agent(agent, args: args) | |
| case "workspaceCommand": | |
| let commandName = try Self.trimmedString(forKey: .commandName, in: container) | |
| ?? Self.trimmedString(forKey: .name, in: container) | |
| ?? Self.trimmedString(forKey: .command, in: container) | |
| guard let commandName else { | |
| throw DecodingError.dataCorrupted( | |
| DecodingError.Context( | |
| codingPath: decoder.codingPath, | |
| debugDescription: "workspaceCommand actions require commandName" | |
| ) | |
| ) | |
| } | |
| action = .workspaceCommand(commandName) | |
| case "workspace": | |
| guard container.contains(.workspace) else { | |
| throw DecodingError.dataCorrupted( | |
| DecodingError.Context( | |
| codingPath: decoder.codingPath, | |
| debugDescription: "workspace actions require a 'workspace' object" | |
| ) | |
| ) | |
| } | |
| let definition = try container.decode(CmuxWorkspaceDefinition.self, forKey: .workspace) | |
| let restart = try container.decodeIfPresent(CmuxRestartBehavior.self, forKey: .restart) | |
| action = .workspace(definition, restart: restart) | |
| case nil: | |
| action = nil | |
| default: | |
| throw DecodingError.dataCorruptedError( | |
| forKey: .type, | |
| in: container, | |
| debugDescription: "Unknown action type '\(inferredType ?? "")'" | |
| ) | |
| } | |
| let inferredType: String? | |
| if let type { | |
| inferredType = type | |
| } else { | |
| let definedForms = [ | |
| container.contains(.agent), | |
| container.contains(.builtin), | |
| container.contains(.workspace), | |
| container.contains(.command) | |
| ].filter(\.self).count | |
| guard definedForms <= 1 else { | |
| throw DecodingError.dataCorrupted( | |
| DecodingError.Context( | |
| codingPath: decoder.codingPath, | |
| debugDescription: "action definitions must define only one of 'agent', 'builtin', 'workspace', or 'command' when 'type' is omitted" | |
| ) | |
| ) | |
| } | |
| if container.contains(.agent) { | |
| inferredType = "agent" | |
| } else if container.contains(.builtin) { | |
| inferredType = "builtin" | |
| } else if container.contains(.workspace) { | |
| inferredType = "workspace" | |
| } else if container.contains(.command) { | |
| inferredType = "command" | |
| } else { | |
| inferredType = nil | |
| } | |
| } | |
| switch inferredType { | |
| case "builtin": | |
| let raw = try Self.trimmedString(forKey: .builtin, in: container) ?? "" | |
| guard let builtIn = CmuxSurfaceTabBarBuiltInAction(configID: raw) else { | |
| throw DecodingError.dataCorruptedError( | |
| forKey: .builtin, | |
| in: container, | |
| debugDescription: "Unknown built-in action '\(raw)'" | |
| ) | |
| } | |
| action = .builtIn(builtIn) | |
| case "command": | |
| let command = try Self.requiredTrimmedString(forKey: .command, in: container) | |
| action = .command(command) | |
| case "agent": | |
| let agent = try container.decode(CmuxConfigAgentKind.self, forKey: .agent) | |
| let args = try Self.trimmedString(forKey: .args, in: container, allowBlankAsNil: true) | |
| action = .agent(agent, args: args) | |
| case "workspaceCommand": | |
| let commandName = try Self.trimmedString(forKey: .commandName, in: container) | |
| ?? Self.trimmedString(forKey: .name, in: container) | |
| ?? Self.trimmedString(forKey: .command, in: container) | |
| guard let commandName else { | |
| throw DecodingError.dataCorrupted( | |
| DecodingError.Context( | |
| codingPath: decoder.codingPath, | |
| debugDescription: "workspaceCommand actions require commandName" | |
| ) | |
| ) | |
| } | |
| action = .workspaceCommand(commandName) | |
| case "workspace": | |
| guard container.contains(.workspace) else { | |
| throw DecodingError.dataCorrupted( | |
| DecodingError.Context( | |
| codingPath: decoder.codingPath, | |
| debugDescription: "workspace actions require a 'workspace' object" | |
| ) | |
| ) | |
| } | |
| let definition = try container.decode(CmuxWorkspaceDefinition.self, forKey: .workspace) | |
| let restart = try container.decodeIfPresent(CmuxRestartBehavior.self, forKey: .restart) | |
| action = .workspace(definition, restart: restart) | |
| case nil: | |
| action = nil | |
| default: | |
| throw DecodingError.dataCorruptedError( | |
| forKey: .type, | |
| in: container, | |
| debugDescription: "Unknown action type '\(inferredType ?? "")'" | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/CmuxConfigActionDefinition.swift` around lines 84 - 150, Add
ambiguity validation in CmuxConfigActionDefinition.init(from:) before using
inferredType when type is omitted. Right now the inferredType priority chain
(agent, builtin, workspace, command) can silently ignore other present action
keys, so mirror the sibling decoder’s definedActionForms check from
CmuxConfig.swift and throw a decoding error if more than one action-defining key
is set. Keep the existing switch on inferredType, but only after confirming
there is exactly one valid action form, including the new workspace case.
Contributor
Author
There was a problem hiding this comment.
Valid hardening suggestion, deferred: mirroring the sibling decoder's definedActionForms ambiguity guard here is out of this closeout loop's write scope. Noted as a follow-up; misconfigured multi-key entries currently resolve by documented priority rather than erroring.
— Claude Code
There was a problem hiding this comment.
@austinywang Understood, thanks for confirming this is a deliberate deferral rather than an oversight. The current priority-based resolution (agent > builtin > workspace > command) will remain as documented behavior for now, and misconfigured entries with multiple action-defining keys will silently resolve via that priority rather than erroring.
Let me know if you'd like me to open a follow-up issue to track adding the definedForms ambiguity guard later, or if you'd rather just leave this noted in the PR thread.
🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: 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`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2964
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-17T21:35:25.493Z
Learning: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry (e.g., resumeCommandWithCwd). The helper should produce a command of the form `cd <shell-quoted cwd> && <resumeCommand>`. Update all call sites that generate “resume” commands (e.g., clipboard actions, drag-drop terminal, and in-app resume) to use this helper so that rc files and newly spawned shells cannot start outside the intended directory. Avoid constructing resume commands directly without the guarded `cd` + shell-quoting + `&&` composition.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:13:36.833Z
Learning: In manaflow-ai/cmux, note that Sources/RestorableAgentSession.swift’s `SessionRestorableAgentSnapshot.resumeCommand` already includes a cwd guard when `workingDirectory` is present (it returns a string like `cd <shell-quoted cwd> && <resumeCommand>`). At call sites (e.g., `Workspace.createPanel(...)`), pass `.resumeCommand` through directly and do not prepend another `cd`/cwd guard or wrap it with an additional `cd <...> &&`—otherwise the working directory may be applied twice or incorrectly.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:14:04.901Z
Learning: In the cmux Swift sources, when restoring a restorable agent session, call sites should pass `resumeCommand` directly (ensuring it has the expected trailing newline if required) to `sendInputWhenReady`. Do not wrap `resumeCommand` with an additional `cd '<cwd>' && ...` guard, because `SessionRestorableAgentSnapshot.resumeCommand` already returns a `cwd`-guarded command; adding another guard can result in `double-cd`. Apply this especially along restore paths (e.g., `Sources/Workspace.swift` restore logic) whenever using `resumeCommand`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3084
File: Sources/AppDelegate.swift:4958-4975
Timestamp: 2026-04-22T11:37:36.238Z
Learning: In Swift code, when re-registering or updating an existing window/session context (e.g., in AppDelegate.registerMainWindow or similar flows), only update an existing *cmuxConfigStore* (or equivalent per-window configuration store) if the incoming configuration/store value is non-nil. Do not overwrite an existing per-window store with nil, so the previous per-window configuration is preserved across re-registration paths.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3128
File: Sources/Panels/BrowserPanelView.swift:4271-4290
Timestamp: 2026-04-23T11:23:49.934Z
Learning: In OmnibarSuggestionsView (and other omnibar-related debug/telemetry logging), never log raw omnibar suggestion content (e.g., URLs, titles, queries). Instead, log only non-sensitive metadata such as suggestion kind/category and the byte length of the text (e.g., "browser.suggestionClick kind=<kind> textBytes=<len>"). Apply this rule consistently to all omnibar-related debug logs to avoid leaking user/search data.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3046
File: Sources/TerminalController.swift:2096-2136
Timestamp: 2026-04-24T22:17:52.550Z
Learning: In Swift request/JSON handlers (e.g., v2 JSON-socket handlers) in Sources, prefer using the v2 helpers for numeric parsing—use `v2Int(params, "<key>")` for normal integer inputs and `v2StrictInt(...)` when strictness is required—rather than casting with `as? Int`. JSONSerialization may yield NSNumber/Double for numeric fields, so v2Int/v2StrictInt ensures correct extraction and type handling. If parsing is used for safety (e.g., timeouts), clamp/validate the parsed value as appropriate (as in `vm.exec` parsing `timeout_ms` and enforcing `>= 1`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3166
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-27T06:58:21.434Z
Learning: In the Swift UI code (e.g., Sources/ContentView.swift), when `preferLiquidGlass`/`materialPolicy.preferLiquidGlass` is enabled for sidebar Liquid Glass, only omit the `Color` tint overlay if `NSGlassEffectView` (native Liquid Glass) is actually available. Compute `usingNativeLiquidGlass = materialPolicy.preferLiquidGlass && SidebarVisualEffectBackground.liquidGlassAvailable`, and when `usingNativeLiquidGlass` is false keep the overlay so the non-native `NSVisualEffectView` fallback still receives the configured tint.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:36.830Z
Learning: When computing NSTextView content height for NSTextView-based editors (e.g., a method like naturalDocumentHeight(...)), account for trailing newline layout. Specifically, include `layoutManager.extraLineFragmentRect.height` in the measured height only when `extraLineFragmentTextContainer == textContainer`. If you don’t, the caret on the final blank line can be clipped. Apply this rule to future NSTextView-based editors in this repo.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3139
File: Sources/Panels/FilePreviewPanel.swift:534-537
Timestamp: 2026-04-28T05:45:32.192Z
Learning: When implementing workspace/panel teardown or close-confirmation logic in Sources (e.g., close/collapse/workspace-close flows), rely on the shared dirty-state gate driven by the panel’s `isDirty` property rather than adding panel-specific teardown special-casing. Ensure each panel (including `FilePreviewPanel`) exposes and keeps its `isDirty` state up to date (e.g., via `Published private(set) var isDirty` and any subscriptions/synchronization logic), so the generic `panel.isDirty` check correctly covers all panel types during teardown.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3218
File: Sources/AppDelegate.swift:4800-4802
Timestamp: 2026-04-28T11:43:53.356Z
Learning: In Swift code that selects or activates the next main-window context (e.g., iterating window-context collections), avoid iterating `mainWindowContexts.values` directly while calling `resolvedWindow(for:)`. Since `resolvedWindow(for:)` may reindex/mutate `mainWindowContexts`, this can cause mutation-during-enumeration issues. Instead, snapshot first with `Array(mainWindowContexts.values)`, then resolve/reindex against that snapshot, and only then call `activateMainWindowContext(_:)` using the resolved result.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3247
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-29T01:08:39.652Z
Learning: Maintain the behavior contract for “Copy Workspace ID(s)”: when triggered from the sidebar context menu (e.g., in Sources/ContentView.swift TabItemView), the command must copy plain UUIDs (IDs-only), not references/refs. For command palette identifier-copy commands where refs are required, ensure the implementation explicitly passes includeRefs: true. This preserves backward compatibility for scripts expecting UUID-only output while allowing the palette to return richer payloads when needed.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3256
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-04-29T01:20:59.683Z
Learning: In this repo’s Swift implementation, keep browser-creation/open behavior consistent when `BrowserAvailabilitySettings` is disabled. For both V1 and V2, any command that includes a URL when creating/opening a browser (e.g., `open_browser` with a URL, `v2 surface.create` with `type=browser` and `url`, `browser.tab.new` with `url`, `v2 browser.open_split` with `url`) must open the URL externally using `NSWorkspace.shared.open(...)` and return appropriate success metadata. Only URL-less/blank browser creations should fail with the `browser_disabled` error.
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3430
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-02T06:07:12.997Z
Learning: In this repo’s SwiftUI views, any view placed under LazyVStack, LazyHStack, List, or ForEach must not capture or hold ObservableObject store instances (e.g., a TabManager). Instead, pass immutable value snapshots (e.g., currentSelectedTabId, sidebarIndexForTabId) plus action closures (e.g., moveToExistingWorkspace, moveToNewWorkspace). Prefer refactoring child view APIs to accept the needed values/closures rather than an ObservableObject reference (e.g., SidebarBonsplitTabWorkspaceDropOverlay should take closures instead of a TabManager).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3480
File: Sources/GhosttyTerminalView.swift:0-0
Timestamp: 2026-05-04T05:31:52.905Z
Learning: In this repo’s Swift sources, keep “surface-scoped” Ghostty config reloads strictly scoped to the target surface. Specifically, GhosttyApp.reloadSurfaceConfiguration(_:soft:source:) should update only the surface via ghostty_surface_update_config and invalidate GhosttyConfig’s load cache, but it must not replace or promote the per-surface config into GhosttyApp’s app-level config/cache (e.g., it must not overwrite GhosttyApp.config or modify app-level cached state). App-level helpers like scrollbarVisibility() and focusFollowsMouseEnabled() must continue to read GhosttyApp.config until a full app reload path is taken.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3502
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-04T11:09:27.707Z
Learning: This repo’s CI enforces a Swift file-length budget for large view files (e.g., Sources/ContentView.swift). When adding helper views/small components, avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources (e.g., Sources/SidebarScrim.swift) and keep it under the CI length threshold. Use access control deliberately: if a extracted view/type must be referenced from other files, do not mark it `private` (use `internal` by omitting `private`); only use `private` for declarations that are truly local to the same file.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:1-54
Timestamp: 2026-05-05T02:19:22.055Z
Learning: In this repo (manafow-ai/cmux), `CmuxTopProcessSnapshot` and `CmuxTopProcessScope` are app-internal types defined under `Sources/`.
When reviewing Swift files under `Sources/`, do not recommend extracting/creating a separate SwiftPM package for extensions over these types (e.g., `CmuxTopSnapshotScopeCache.swift`). Only suggest SwiftPM extraction if the repo introduces a dedicated process-inspection package (e.g., a new `MuxCore`/`process-inspection`-style package) rather than trying to extract app-internal extensions in isolation.
Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3559
File: Sources/GhosttyTerminalView.swift:3824-3846
Timestamp: 2026-05-05T16:41:00.198Z
Learning: In cmux (Sources), the method `AppDelegate.shared?.workspaceContainingPanel(panelId:preferredWorkspaceId:)` returns an optional *named tuple* that includes a `workspace` field (not an optional `Workspace` directly). When using it, access the workspace through `.workspace`, e.g. `AppDelegate.shared?.workspaceContainingPanel(...)?.workspace ?? fallback`, and avoid treating the method’s return value as `Workspace?`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3582
File: Sources/SessionIndexModels.swift:261-276
Timestamp: 2026-05-06T07:12:22.050Z
Learning: In manaflow-ai/cmux, SessionEntry.resumeWorkingDirectory is the single source of truth for the working directory used in registered-agent resume commands. It must be passed as the 'workingDirectory' into AgentResumeCommandBuilder.resumeShellCommand, and resumeCommandWithCwd should consult it before prepending a 'cd <dir>' guard. Registrations with cwd: .ignore set resumeWorkingDirectory to nil, suppressing both the cwd guard in the resume command and the terminal working directory at placement time (ContentView resume, Workspace drag/drop). Apply this guidance to all Swift files under Sources (and related modules) to preserve consistent behavior.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3629
File: Sources/TerminalController.swift:8094-8105
Timestamp: 2026-05-06T10:18:57.314Z
Learning: Redact sensitive fields from all workstream event payloads before any persistence or streaming in the manaflow-ai/cmux project. Specifically redact tool_input, context, and extra in every encoded payload prior to emitting via events.stream or writing to ~/.cmuxterm/events.jsonl. Implement a centralized sanitizer in the encoding/transport stack (e.g., FeedSocketEncoding) and ensure TerminalController.v2FeedPush, CmuxEventBus.publishWorkstreamEvent, and all related encoding paths apply redaction. Add tests that verify sensitive strings do not appear in encoded event payloads. Use an allowlist approach for permitted fields and replace redacted fields with a stable placeholder (e.g., [REDACTED]). Ensure no redacted values are logged or persisted, and that CI checks enforce the rule.
Learnt from: ryouka0731
Repo: manaflow-ai/cmux PR: 3415
File: Sources/Worktree/WorktreeManager.swift:1-10
Timestamp: 2026-05-06T19:24:46.399Z
Learning: Do not recommend extracting Foundation-only Swift files into a new SwiftPM package target in this repo. The established pattern is to keep app-internal helper modules under Sources/ unless the repository introduces a dedicated package (e.g., CMUXAuthCore, CMUXDebugLog, CMUXWorkstream) with a clear architectural rationale. Speculative SwiftPM extractions for individual Foundation-only modules expand PR scope without documented justification. Apply this guideline when reviewing changes to Swift files under Sources/; require a documented package architecture justification before considering packaging changes.
Learnt from: say8425
Repo: manaflow-ai/cmux PR: 3680
File: Sources/cmuxApp.swift:0-0
Timestamp: 2026-05-07T05:01:28.476Z
Learning: In the cmux Swift app, the right sidebar’s visibility should come from `RightSidebarVisibilityTracker.shared` (an `ObservableObject`) and be bound to the active main window’s `FileExplorerState.isVisible`. Update/bind the UI state on window focus changes: `AppDelegate.activateMainWindowContext(...)` must call `bind(to:)` when the window changes so menu/command-palette titles reflect the focused window. Do not mirror these labels via a global `AppStorage("fileExplorer.isVisible")` (or other global storage) used as a label/visibility source; it should reflect the active window state instead.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TabManager.swift:1817-1818
Timestamp: 2026-05-07T08:37:03.967Z
Learning: In this Swift repo (manafow-ai/cmux), when cleaning up stale agent process entries, use `Workspace.clearAgentPID(key:panelId:)` as the single cleanup path. Do not directly mutate `Workspace.statusEntries` or `Workspace.agentPIDs` from outside the dedicated helpers; for example, `TabManager.sweepStaleAgentPIDs` should only call `clearAgentPID` rather than performing its own mutations. This ensures panel-scoped side effects and port/refresh logic run consistently.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TerminalController.swift:15973-15983
Timestamp: 2026-05-07T09:14:20.991Z
Learning: In the manaflow-ai/cmux Swift codebase, ensure option/argument parsing for panel/surface targeting handles explicitly provided but empty values as errors. For example, in Sources/TerminalController.swift, agentTrackingPanelTarget(options:) must treat an explicit empty value for --panel/--surface (e.g., `--panel ""`) as a parse error such as “Missing value for --panel”, rather than falling back to an unscoped/default target. This prevents unintended workspace-wide mutations when a scoped UUID/panel target is required. Apply the same validation rule to any related socket option parsers that support --panel/--surface targeting.
Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3696
File: cmuxTests/ShortcutAndCommandPaletteTests.swift:1716-1771
Timestamp: 2026-05-07T10:56:50.266Z
Learning: In the manaflow-ai/cmux repo, SwiftLint does not enforce a `required_deinit` rule (no project `.swiftlint.yml` in cmux itself, no `required_deinit` in `.github/review-bot-rules/`, and no SwiftLint CI run in `.github/workflows/`). During code reviews, do not raise findings for missing `deinit` on `XCTestCase` subclasses or other Swift classes based on a `required_deinit` rule.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3784
File: README.md:161-161
Timestamp: 2026-05-09T04:48:35.413Z
Learning: In the cmux project, the right-sidebar keyboard shortcut labels were intentionally swapped (per PR `#3784`). Reviewers should NOT flag the ⌘⇧E (Cmd+Shift+E) label as “Open file explorer.” Use these mappings consistently: ⌘⇧E → `focusRightSidebar` with the user-facing label “Toggle right sidebar focus”; ⌘⌥B (Cmd+Option+B) → `toggleFileExplorer` with the user-facing label “Open file explorer.”
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4353
File: Sources/AppIconDockTilePlugin.swift:91-97
Timestamp: 2026-05-19T07:50:03.218Z
Learning: When working with an `NSDockTilePlugIn`/dock tile plugin, remember it runs in the Dock process (`com.apple.dock`), not the main app process. Do not try to read app launch flags via `ProcessInfo.processInfo.arguments` inside the dock plugin—those arguments aren’t visible to the Dock process. For cross-process state, use a shared `UserDefaults` suite identified by the app’s bundle identifier (e.g., `UserDefaults(suiteName: appBundleIdentifier)`), since it’s accessible from both the Dock tile plugin and the main app. If you need deterministic behavior in smoke/UI tests, seed any relevant `UserDefaults` keys (e.g., via `defaults write "$BUNDLE_ID" ...`) before calling `open` so the dock plugin observes the flag before the app starts, and remove the key during cleanup after the smoke test.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4626
File: Sources/Workspace.swift:0-0
Timestamp: 2026-05-23T03:33:51.872Z
Learning: In this repo’s Swift code, the close-tab confirmation trigger must be explicit and wired through all close flows.
- For close operations, ensure the close-tab confirmation policy accepts and uses an explicit `CloseTabConfirmationTrigger` parameter.
- Specifically, `Workspace.markExplicitClose(surfaceId:trigger:)` and `TabManager.closeWorkspaceFromCloseTabGesture(_:trigger:)` must NOT rely on default parameter values for the trigger; each callsite must pass an appropriate trigger.
- At each callsite, pass the correct trigger: `.tabCloseButton` for the X-button, `.shortcut` for Cmd+W, and a purpose-specific value for programmatic/API-driven close paths.
- When touching the related trigger enums, keep them `nonisolated` and `Sendable` as required by the concurrency model.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 4855
File: Sources/cmuxApp.swift:6302-6314
Timestamp: 2026-05-27T08:47:03.390Z
Learning: In the manaflow-ai/cmux settings UI, keep `app.menuBarOnly` / “Menu Bar Only” under the **App** settings bucket because it controls app presence behavior (Dock icon and Cmd+Tab visibility), not notification behavior. Ensure `SettingsNavigation` maps this setting to the `.app` section with the `menu-bar-only` anchor, and that command-palette settings descriptors report it as belonging to the **App** section. When reviewing, do not suggest moving it to **Notifications** solely because it sits near menu bar/Dock-related notification settings.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6841
File: Sources/App/VSCodeServeWebSupport.swift:856-909
Timestamp: 2026-06-26T10:45:17.360Z
Learning: In manaflow-ai/cmux, when reviewing Swift production code for blocking synchronization (per .github/review-bot-rules/swift-blocking-runtime.md), do not flag issues if the PR only relocates existing blocking code verbatim (a move/rename/re-file move) without introducing new blocking synchronization and without worsening the blocking pattern (e.g., no additional call sites, no increased frequency/usage, and the moved code logic remains unchanged). For example, moving ServeWebOutputCollector from one Swift source to another should be treated as allowed under this exception when the diff contains only a verbatim relocation.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6905
File: Sources/OfflineNotesStore.swift:0-0
Timestamp: 2026-06-26T12:31:31.190Z
Learning: When reviewing Swift code in this repo’s `Sources/`, do not automatically flag an app-global “persisted owner” as incorrect just because it uses a single shared instance (e.g., `static let shared`), if—and only if—the feature is backed by exactly one process-wide persistence (one store/file/DB) such that creating multiple per-window instances would cause races on the same persistence.
This singleton pattern is acceptable when the shared store is lazily created, the ownership model is truly app-global, and concurrency is handled appropriately (e.g., `MainActor` for `Observable` state or equivalent thread-safety). Also ensure test seams/injection are available so the persistence boundary can be controlled in tests.
Under these conditions, patterns like the ones used in `Sources/OfflineNotesStore.swift` (e.g., `MainActor Observable` + `static let shared`) are consistent with existing precedents; otherwise, prefer per-window instances or explicit coordination to prevent persistence races.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6905
File: Sources/OfflineNotesStore.swift:0-0
Timestamp: 2026-06-26T12:31:53.114Z
Learning: When reviewing Swift code in this repo, an app-global `Observable` singleton (e.g., `static let shared`) can be acceptable if it truly *owns* a single persisted app-wide backing resource, so creating multiple window-scoped instances would otherwise race on the same store. Treat the pattern as non-problematic (don’t auto-flag) when all/most of these hold: (1) the singleton is the sole owner of one persisted resource (e.g., one JSON file / one shared store), (2) window-scoped instances would contend for the same underlying storage, (3) the singleton is annotated appropriately for concurrency (often `MainActor` when used on the main actor), (4) it supports testability (e.g., is injectable or can be swapped in tests), and (5) any reachability/background work is started lazily when the relevant UI/panel opens. Use the existing app-global precedents (e.g., `PaneMemoryGuardrail`, `DiffCommentStore.shared`) as confirmation of this intended pattern. Example: `Sources/OfflineNotesStore.swift` can remain `MainActor Observable static let shared` when it persists one offline-notes JSON file app-wide and lazily begins its work when the Notes panel opens.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4608
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-07-01T22:19:25.948Z
Learning: When reviewing Swift localization, do not require `String(localized:)` for v1 socket-protocol command reply strings used by the cmux CLI/hooks/tests (i.e., reply literals over the wire that are machine-parsed and must match exact text like `OK` / `ERROR: ...` or command “usage” text). Localizing these reply strings can break scripted clients and tests that match the literal replies. Only flag missing localization for genuine UI-visible copy (e.g., actual notification/display text), which should follow the project’s localization approach.
Comment on lines
+67
to
+69
| #if DEBUG | ||
| NSLog("[CmuxConfig] split node requires exactly 2 children, got %d", split.children.count) | ||
| #endif |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Prefer cmuxDebugLog over NSLog for this diagnostic.
This new production file introduces NSLog for a debug diagnostic; the project convention/logging guideline is to avoid print/debugPrint/dump/NSLog in app/runtime Swift and route through the shared debug log instead.
As per coding guidelines: "Do not introduce or materially expand print, debugPrint, dump, or NSLog in app/runtime Swift code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/Workspace`+CustomLayout.swift around lines 67 - 69, Replace the
direct NSLog diagnostic in Workspace+CustomLayout’s split-node validation with
the shared cmuxDebugLog helper to match the app/runtime logging convention. Keep
the existing DEBUG-only guard and the same message context, but route the count
information through cmuxDebugLog instead of emitting NSLog from the
split.children check.
Source: Coding guidelines
Contributor
Author
There was a problem hiding this comment.
Acknowledged: the NSLog is inside #if DEBUG so it never ships in release builds, and it sits in a defensive branch for a structurally impossible split shape. Swapping it to cmuxDebugLog for consistency is noted as a follow-up nit; leaving it as-is in this pass.
— Claude Code
There was a problem hiding this comment.
@austinywang, understood — that's a fair call given it's a #if DEBUG-only defensive branch. I'll leave it as-is for this pass. Thanks for confirming it's tracked as a follow-up nit.
🧠 Learnings used
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:0-0
Timestamp: 2026-03-04T14:05:42.574Z
Learning: Guideline: In Swift files (cmux project), when handling pluralized strings, prefer using localization keys with the ICU-style plural forms .one and .other. For example, use keys like statusMenu.unreadCount.one for the singular case (1) and statusMenu.unreadCount.other for all other counts, and similarly for statusMenu.tooltip.unread.one/other. Rationale: ensures correct pluralization across locales and makes localization keys explicit. Review code to ensure any unread count strings and related tooltips follow this .one/.other key pattern and verify the correct value is chosen based on the count.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 954
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-05T22:04:34.712Z
Learning: Adopt the convention: for health/telemetry tri-state values in Swift, prefer Optionals (Bool?) over sentinel booleans. In TerminalController.swift, socketConnectable is Bool? and only set when socketProbePerformed is true; downstream logic must treat nil as 'not probed'. Ensure downstream code checks for nil before using a value and uses explicit non-nil checks to determine state, improving clarity and avoiding misinterpretation of default false.
Learnt from: moyashin63
Repo: manaflow-ai/cmux PR: 1074
File: Sources/AppDelegate.swift:7523-7545
Timestamp: 2026-03-09T01:38:24.337Z
Learning: When the command palette is visible (as in manaflow-ai/cmux Sources/AppDelegate.swift), ensure the shortcut handling consumes most Command shortcuts to protect the palette's text input. Specifically, do not allow UI zoom shortcuts (Cmd+Shift+= / Cmd+Shift+− / Cmd+Shift+0) to trigger while the palette is open. Do not reorder shortcut handlers (e.g., uiZoomShortcutAction(...)) to bypass this guard; users must close the palette before performing zoom actions. This guideline should apply to Swift source files handling global shortcuts within the app.
Learnt from: zlatkoc
Repo: manaflow-ai/cmux PR: 1368
File: Sources/Panels/BrowserPanel.swift:69-69
Timestamp: 2026-03-13T13:46:01.733Z
Learning: Do not wrap engine/brand name literals (e.g., displayName values such as Google, DuckDuckGo, Bing, Kagi, Startpage) in String(localized: ...). These are brand/product names that are not translatable UI text. Localization should apply to generic UI strings (labels, buttons, error messages, etc.). Apply this guideline across Swift source files under Sources/ (notably in BrowserPanel.swift and similar UI/engine-related strings) and flag only brand-name strings that are part of user-facing UI text appropriately for translation scope.
Learnt from: kjb0787
Repo: manaflow-ai/cmux PR: 1461
File: Sources/GhosttyTerminalView.swift:5904-5905
Timestamp: 2026-03-15T19:22:32.330Z
Learning: In Swift files under the Sources directory that manage terminal/scroll behavior, ensure the following: when preserving scroll across workspace switches, save savedScrollRow only if the scrollbar offset is greater than 0 (indicating the user has scrolled up). On restore, call scroll_to_row only if savedScrollRow is non-nil; if it is nil, rely on synchronizeScrollView() to keep bottom-pinned sessions following new output. This pattern should be applied wherever GhosttyTerminalView-like views implement setVisibleInUI(_:) to maintain consistent user scroll state across workspace switches.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:06.824Z
Learning: In Swift sources, for any panel_id-only route handling in v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:), first attempt v2ResolveTabManager(params:). Use the manager only if it actually owns the panelId; otherwise fall back to AppDelegate.shared?.locateSurface(surfaceId:) to locate the correct TabManager across windows. Apply this pattern to all panel_id-only routes to avoid active-window bias.
Learnt from: pratikpakhale
Repo: manaflow-ai/cmux PR: 2011
File: Resources/Localizable.xcstrings:15256-15368
Timestamp: 2026-03-23T21:39:50.795Z
Learning: When reviewing this repo’s Swift localization usage, do not flag missing `String.localizedStringWithFormat` for calls that use the modern overload `String(localized: "key", defaultValue: "...\(variable)")` (where `defaultValue` is a `String.LocalizationValue` built with `\(…)`). That overload natively supports interpolation and the xcstrings/runtime substitution handles the resulting placeholders automatically. Only require `String.localizedStringWithFormat` when using the older `String(localized:)` overload that takes a plain `String` (i.e., where format arguments must be passed separately), such as for keys like `clipboard.sshError.single`.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: 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`). Ensure validation logic matches the existing behavior (e.g., WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) as well as CLI/cmux.swift). Update any error/help text for workspace color to reference only `#RRGGBB` (not `#RRGGBBAA`).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/GhosttyTerminalView.swift:3891-3903
Timestamp: 2026-04-02T21:37:21.463Z
Learning: In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags (e.g., hasStartupCommand/hasInitialInput), (2) byte counts, and (3) the relevant surface id (so issues can be correlated without exposing the underlying strings).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2528
File: Sources/cmuxApp.swift:6439-6444
Timestamp: 2026-04-03T03:35:54.082Z
Learning: In this repo’s keyboard shortcut implementation, ensure `KeyboardShortcutSettings.setShortcut(...)` does nothing (no-op) when `KeyboardShortcutSettings.isManagedBySettingsFile(action)` returns `true` (i.e., the shortcut is managed via `settings.json`). This prevents writing back overrides into `UserDefaults` and keeps `settings.json` as the source of truth.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2964
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-17T21:35:25.493Z
Learning: In this repo’s shell session resume flow, always build resume commands using the cwd guard helper exposed by SessionEntry (e.g., resumeCommandWithCwd). The helper should produce a command of the form `cd <shell-quoted cwd> && <resumeCommand>`. Update all call sites that generate “resume” commands (e.g., clipboard actions, drag-drop terminal, and in-app resume) to use this helper so that rc files and newly spawned shells cannot start outside the intended directory. Avoid constructing resume commands directly without the guarded `cd` + shell-quoting + `&&` composition.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:13:36.833Z
Learning: In manaflow-ai/cmux, note that Sources/RestorableAgentSession.swift’s `SessionRestorableAgentSnapshot.resumeCommand` already includes a cwd guard when `workingDirectory` is present (it returns a string like `cd <shell-quoted cwd> && <resumeCommand>`). At call sites (e.g., `Workspace.createPanel(...)`), pass `.resumeCommand` through directly and do not prepend another `cd`/cwd guard or wrap it with an additional `cd <...> &&`—otherwise the working directory may be applied twice or incorrectly.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2978
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-22T08:14:04.901Z
Learning: In the cmux Swift sources, when restoring a restorable agent session, call sites should pass `resumeCommand` directly (ensuring it has the expected trailing newline if required) to `sendInputWhenReady`. Do not wrap `resumeCommand` with an additional `cd '<cwd>' && ...` guard, because `SessionRestorableAgentSnapshot.resumeCommand` already returns a `cwd`-guarded command; adding another guard can result in `double-cd`. Apply this especially along restore paths (e.g., `Sources/Workspace.swift` restore logic) whenever using `resumeCommand`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3084
File: Sources/AppDelegate.swift:4958-4975
Timestamp: 2026-04-22T11:37:36.238Z
Learning: In Swift code, when re-registering or updating an existing window/session context (e.g., in AppDelegate.registerMainWindow or similar flows), only update an existing *cmuxConfigStore* (or equivalent per-window configuration store) if the incoming configuration/store value is non-nil. Do not overwrite an existing per-window store with nil, so the previous per-window configuration is preserved across re-registration paths.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3128
File: Sources/Panels/BrowserPanelView.swift:4271-4290
Timestamp: 2026-04-23T11:23:49.934Z
Learning: In OmnibarSuggestionsView (and other omnibar-related debug/telemetry logging), never log raw omnibar suggestion content (e.g., URLs, titles, queries). Instead, log only non-sensitive metadata such as suggestion kind/category and the byte length of the text (e.g., "browser.suggestionClick kind=<kind> textBytes=<len>"). Apply this rule consistently to all omnibar-related debug logs to avoid leaking user/search data.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3046
File: Sources/TerminalController.swift:2096-2136
Timestamp: 2026-04-24T22:17:52.550Z
Learning: In Swift request/JSON handlers (e.g., v2 JSON-socket handlers) in Sources, prefer using the v2 helpers for numeric parsing—use `v2Int(params, "<key>")` for normal integer inputs and `v2StrictInt(...)` when strictness is required—rather than casting with `as? Int`. JSONSerialization may yield NSNumber/Double for numeric fields, so v2Int/v2StrictInt ensures correct extraction and type handling. If parsing is used for safety (e.g., timeouts), clamp/validate the parsed value as appropriate (as in `vm.exec` parsing `timeout_ms` and enforcing `>= 1`).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3166
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-27T06:58:21.434Z
Learning: In the Swift UI code (e.g., Sources/ContentView.swift), when `preferLiquidGlass`/`materialPolicy.preferLiquidGlass` is enabled for sidebar Liquid Glass, only omit the `Color` tint overlay if `NSGlassEffectView` (native Liquid Glass) is actually available. Compute `usingNativeLiquidGlass = materialPolicy.preferLiquidGlass && SidebarVisualEffectBackground.liquidGlassAvailable`, and when `usingNativeLiquidGlass` is false keep the overlay so the non-native `NSVisualEffectView` fallback still receives the configured tint.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3182
File: Sources/ContentView.swift:10850-10875
Timestamp: 2026-04-27T10:11:36.830Z
Learning: When computing NSTextView content height for NSTextView-based editors (e.g., a method like naturalDocumentHeight(...)), account for trailing newline layout. Specifically, include `layoutManager.extraLineFragmentRect.height` in the measured height only when `extraLineFragmentTextContainer == textContainer`. If you don’t, the caret on the final blank line can be clipped. Apply this rule to future NSTextView-based editors in this repo.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3139
File: Sources/Panels/FilePreviewPanel.swift:534-537
Timestamp: 2026-04-28T05:45:32.192Z
Learning: When implementing workspace/panel teardown or close-confirmation logic in Sources (e.g., close/collapse/workspace-close flows), rely on the shared dirty-state gate driven by the panel’s `isDirty` property rather than adding panel-specific teardown special-casing. Ensure each panel (including `FilePreviewPanel`) exposes and keeps its `isDirty` state up to date (e.g., via `Published private(set) var isDirty` and any subscriptions/synchronization logic), so the generic `panel.isDirty` check correctly covers all panel types during teardown.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3218
File: Sources/AppDelegate.swift:4800-4802
Timestamp: 2026-04-28T11:43:53.356Z
Learning: In Swift code that selects or activates the next main-window context (e.g., iterating window-context collections), avoid iterating `mainWindowContexts.values` directly while calling `resolvedWindow(for:)`. Since `resolvedWindow(for:)` may reindex/mutate `mainWindowContexts`, this can cause mutation-during-enumeration issues. Instead, snapshot first with `Array(mainWindowContexts.values)`, then resolve/reindex against that snapshot, and only then call `activateMainWindowContext(_:)` using the resolved result.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3247
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-29T01:08:39.652Z
Learning: Maintain the behavior contract for “Copy Workspace ID(s)”: when triggered from the sidebar context menu (e.g., in Sources/ContentView.swift TabItemView), the command must copy plain UUIDs (IDs-only), not references/refs. For command palette identifier-copy commands where refs are required, ensure the implementation explicitly passes includeRefs: true. This preserves backward compatibility for scripts expecting UUID-only output while allowing the palette to return richer payloads when needed.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3256
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-04-29T01:20:59.683Z
Learning: In this repo’s Swift implementation, keep browser-creation/open behavior consistent when `BrowserAvailabilitySettings` is disabled. For both V1 and V2, any command that includes a URL when creating/opening a browser (e.g., `open_browser` with a URL, `v2 surface.create` with `type=browser` and `url`, `browser.tab.new` with `url`, `v2 browser.open_split` with `url`) must open the URL externally using `NSWorkspace.shared.open(...)` and return appropriate success metadata. Only URL-less/blank browser creations should fail with the `browser_disabled` error.
Learnt from: pgbezerra
Repo: manaflow-ai/cmux PR: 3307
File: Sources/cmuxApp.swift:6413-6417
Timestamp: 2026-04-30T11:55:31.575Z
Learning: In this repo (manaflow-ai/cmux), when adding a new Settings section in SwiftUI (e.g., in Sources/cmuxApp.swift or related Views), don’t wire navigation/search with a raw anchor string alone. Instead: (1) create a corresponding SettingsNavigationTarget enum case (e.g., .workspaces); (2) provide the localized title, symbol, search text, and aliases for that case; (3) add/update the matching entry in SettingsSearchIndex so the sidebar/search can navigate to it; and (4) apply .settingsSearchAnchor(SettingsSearchIndex.sectionID(for: <target>)) to the section header. This prevents broken jump-to behavior by ensuring the navigation anchor and the search index stay consistent.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3430
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-02T06:07:12.997Z
Learning: In this repo’s SwiftUI views, any view placed under LazyVStack, LazyHStack, List, or ForEach must not capture or hold ObservableObject store instances (e.g., a TabManager). Instead, pass immutable value snapshots (e.g., currentSelectedTabId, sidebarIndexForTabId) plus action closures (e.g., moveToExistingWorkspace, moveToNewWorkspace). Prefer refactoring child view APIs to accept the needed values/closures rather than an ObservableObject reference (e.g., SidebarBonsplitTabWorkspaceDropOverlay should take closures instead of a TabManager).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3480
File: Sources/GhosttyTerminalView.swift:0-0
Timestamp: 2026-05-04T05:31:52.905Z
Learning: In this repo’s Swift sources, keep “surface-scoped” Ghostty config reloads strictly scoped to the target surface. Specifically, GhosttyApp.reloadSurfaceConfiguration(_:soft:source:) should update only the surface via ghostty_surface_update_config and invalidate GhosttyConfig’s load cache, but it must not replace or promote the per-surface config into GhosttyApp’s app-level config/cache (e.g., it must not overwrite GhosttyApp.config or modify app-level cached state). App-level helpers like scrollbarVisibility() and focusFollowsMouseEnabled() must continue to read GhosttyApp.config until a full app reload path is taken.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3502
File: Sources/ContentView.swift:0-0
Timestamp: 2026-05-04T11:09:27.707Z
Learning: This repo’s CI enforces a Swift file-length budget for large view files (e.g., Sources/ContentView.swift). When adding helper views/small components, avoid bloating the existing file: extract the subview into a dedicated Swift file under Sources (e.g., Sources/SidebarScrim.swift) and keep it under the CI length threshold. Use access control deliberately: if a extracted view/type must be referenced from other files, do not mark it `private` (use `internal` by omitting `private`); only use `private` for declarations that are truly local to the same file.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3471
File: Sources/CmuxTopSnapshotScopeCache.swift:1-54
Timestamp: 2026-05-05T02:19:22.055Z
Learning: In this repo (manafow-ai/cmux), `CmuxTopProcessSnapshot` and `CmuxTopProcessScope` are app-internal types defined under `Sources/`.
When reviewing Swift files under `Sources/`, do not recommend extracting/creating a separate SwiftPM package for extensions over these types (e.g., `CmuxTopSnapshotScopeCache.swift`). Only suggest SwiftPM extraction if the repo introduces a dedicated process-inspection package (e.g., a new `MuxCore`/`process-inspection`-style package) rather than trying to extract app-internal extensions in isolation.
Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3559
File: Sources/GhosttyTerminalView.swift:3824-3846
Timestamp: 2026-05-05T16:41:00.198Z
Learning: In cmux (Sources), the method `AppDelegate.shared?.workspaceContainingPanel(panelId:preferredWorkspaceId:)` returns an optional *named tuple* that includes a `workspace` field (not an optional `Workspace` directly). When using it, access the workspace through `.workspace`, e.g. `AppDelegate.shared?.workspaceContainingPanel(...)?.workspace ?? fallback`, and avoid treating the method’s return value as `Workspace?`.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3582
File: Sources/SessionIndexModels.swift:261-276
Timestamp: 2026-05-06T07:12:22.050Z
Learning: In manaflow-ai/cmux, SessionEntry.resumeWorkingDirectory is the single source of truth for the working directory used in registered-agent resume commands. It must be passed as the 'workingDirectory' into AgentResumeCommandBuilder.resumeShellCommand, and resumeCommandWithCwd should consult it before prepending a 'cd <dir>' guard. Registrations with cwd: .ignore set resumeWorkingDirectory to nil, suppressing both the cwd guard in the resume command and the terminal working directory at placement time (ContentView resume, Workspace drag/drop). Apply this guidance to all Swift files under Sources (and related modules) to preserve consistent behavior.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3629
File: Sources/TerminalController.swift:8094-8105
Timestamp: 2026-05-06T10:18:57.314Z
Learning: Redact sensitive fields from all workstream event payloads before any persistence or streaming in the manaflow-ai/cmux project. Specifically redact tool_input, context, and extra in every encoded payload prior to emitting via events.stream or writing to ~/.cmuxterm/events.jsonl. Implement a centralized sanitizer in the encoding/transport stack (e.g., FeedSocketEncoding) and ensure TerminalController.v2FeedPush, CmuxEventBus.publishWorkstreamEvent, and all related encoding paths apply redaction. Add tests that verify sensitive strings do not appear in encoded event payloads. Use an allowlist approach for permitted fields and replace redacted fields with a stable placeholder (e.g., [REDACTED]). Ensure no redacted values are logged or persisted, and that CI checks enforce the rule.
Learnt from: ryouka0731
Repo: manaflow-ai/cmux PR: 3415
File: Sources/Worktree/WorktreeManager.swift:1-10
Timestamp: 2026-05-06T19:24:46.399Z
Learning: Do not recommend extracting Foundation-only Swift files into a new SwiftPM package target in this repo. The established pattern is to keep app-internal helper modules under Sources/ unless the repository introduces a dedicated package (e.g., CMUXAuthCore, CMUXDebugLog, CMUXWorkstream) with a clear architectural rationale. Speculative SwiftPM extractions for individual Foundation-only modules expand PR scope without documented justification. Apply this guideline when reviewing changes to Swift files under Sources/; require a documented package architecture justification before considering packaging changes.
Learnt from: say8425
Repo: manaflow-ai/cmux PR: 3680
File: Sources/cmuxApp.swift:0-0
Timestamp: 2026-05-07T05:01:28.476Z
Learning: In the cmux Swift app, the right sidebar’s visibility should come from `RightSidebarVisibilityTracker.shared` (an `ObservableObject`) and be bound to the active main window’s `FileExplorerState.isVisible`. Update/bind the UI state on window focus changes: `AppDelegate.activateMainWindowContext(...)` must call `bind(to:)` when the window changes so menu/command-palette titles reflect the focused window. Do not mirror these labels via a global `AppStorage("fileExplorer.isVisible")` (or other global storage) used as a label/visibility source; it should reflect the active window state instead.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TabManager.swift:1817-1818
Timestamp: 2026-05-07T08:37:03.967Z
Learning: In this Swift repo (manafow-ai/cmux), when cleaning up stale agent process entries, use `Workspace.clearAgentPID(key:panelId:)` as the single cleanup path. Do not directly mutate `Workspace.statusEntries` or `Workspace.agentPIDs` from outside the dedicated helpers; for example, `TabManager.sweepStaleAgentPIDs` should only call `clearAgentPID` rather than performing its own mutations. This ensures panel-scoped side effects and port/refresh logic run consistently.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 3626
File: Sources/TerminalController.swift:15973-15983
Timestamp: 2026-05-07T09:14:20.991Z
Learning: In the manaflow-ai/cmux Swift codebase, ensure option/argument parsing for panel/surface targeting handles explicitly provided but empty values as errors. For example, in Sources/TerminalController.swift, agentTrackingPanelTarget(options:) must treat an explicit empty value for --panel/--surface (e.g., `--panel ""`) as a parse error such as “Missing value for --panel”, rather than falling back to an unscoped/default target. This prevents unintended workspace-wide mutations when a scoped UUID/panel target is required. Apply the same validation rule to any related socket option parsers that support --panel/--surface targeting.
Learnt from: psh4607
Repo: manaflow-ai/cmux PR: 3696
File: cmuxTests/ShortcutAndCommandPaletteTests.swift:1716-1771
Timestamp: 2026-05-07T10:56:50.266Z
Learning: In the manaflow-ai/cmux repo, SwiftLint does not enforce a `required_deinit` rule (no project `.swiftlint.yml` in cmux itself, no `required_deinit` in `.github/review-bot-rules/`, and no SwiftLint CI run in `.github/workflows/`). During code reviews, do not raise findings for missing `deinit` on `XCTestCase` subclasses or other Swift classes based on a `required_deinit` rule.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 3784
File: README.md:161-161
Timestamp: 2026-05-09T04:48:35.413Z
Learning: In the cmux project, the right-sidebar keyboard shortcut labels were intentionally swapped (per PR `#3784`). Reviewers should NOT flag the ⌘⇧E (Cmd+Shift+E) label as “Open file explorer.” Use these mappings consistently: ⌘⇧E → `focusRightSidebar` with the user-facing label “Toggle right sidebar focus”; ⌘⌥B (Cmd+Option+B) → `toggleFileExplorer` with the user-facing label “Open file explorer.”
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4353
File: Sources/AppIconDockTilePlugin.swift:91-97
Timestamp: 2026-05-19T07:50:03.218Z
Learning: When working with an `NSDockTilePlugIn`/dock tile plugin, remember it runs in the Dock process (`com.apple.dock`), not the main app process. Do not try to read app launch flags via `ProcessInfo.processInfo.arguments` inside the dock plugin—those arguments aren’t visible to the Dock process. For cross-process state, use a shared `UserDefaults` suite identified by the app’s bundle identifier (e.g., `UserDefaults(suiteName: appBundleIdentifier)`), since it’s accessible from both the Dock tile plugin and the main app. If you need deterministic behavior in smoke/UI tests, seed any relevant `UserDefaults` keys (e.g., via `defaults write "$BUNDLE_ID" ...`) before calling `open` so the dock plugin observes the flag before the app starts, and remove the key during cleanup after the smoke test.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4626
File: Sources/Workspace.swift:0-0
Timestamp: 2026-05-23T03:33:51.872Z
Learning: In this repo’s Swift code, the close-tab confirmation trigger must be explicit and wired through all close flows.
- For close operations, ensure the close-tab confirmation policy accepts and uses an explicit `CloseTabConfirmationTrigger` parameter.
- Specifically, `Workspace.markExplicitClose(surfaceId:trigger:)` and `TabManager.closeWorkspaceFromCloseTabGesture(_:trigger:)` must NOT rely on default parameter values for the trigger; each callsite must pass an appropriate trigger.
- At each callsite, pass the correct trigger: `.tabCloseButton` for the X-button, `.shortcut` for Cmd+W, and a purpose-specific value for programmatic/API-driven close paths.
- When touching the related trigger enums, keep them `nonisolated` and `Sendable` as required by the concurrency model.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 4855
File: Sources/cmuxApp.swift:6302-6314
Timestamp: 2026-05-27T08:47:03.390Z
Learning: In the manaflow-ai/cmux settings UI, keep `app.menuBarOnly` / “Menu Bar Only” under the **App** settings bucket because it controls app presence behavior (Dock icon and Cmd+Tab visibility), not notification behavior. Ensure `SettingsNavigation` maps this setting to the `.app` section with the `menu-bar-only` anchor, and that command-palette settings descriptors report it as belonging to the **App** section. When reviewing, do not suggest moving it to **Notifications** solely because it sits near menu bar/Dock-related notification settings.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6841
File: Sources/App/VSCodeServeWebSupport.swift:856-909
Timestamp: 2026-06-26T10:45:17.360Z
Learning: In manaflow-ai/cmux, when reviewing Swift production code for blocking synchronization (per .github/review-bot-rules/swift-blocking-runtime.md), do not flag issues if the PR only relocates existing blocking code verbatim (a move/rename/re-file move) without introducing new blocking synchronization and without worsening the blocking pattern (e.g., no additional call sites, no increased frequency/usage, and the moved code logic remains unchanged). For example, moving ServeWebOutputCollector from one Swift source to another should be treated as allowed under this exception when the diff contains only a verbatim relocation.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6905
File: Sources/OfflineNotesStore.swift:0-0
Timestamp: 2026-06-26T12:31:31.190Z
Learning: When reviewing Swift code in this repo’s `Sources/`, do not automatically flag an app-global “persisted owner” as incorrect just because it uses a single shared instance (e.g., `static let shared`), if—and only if—the feature is backed by exactly one process-wide persistence (one store/file/DB) such that creating multiple per-window instances would cause races on the same persistence.
This singleton pattern is acceptable when the shared store is lazily created, the ownership model is truly app-global, and concurrency is handled appropriately (e.g., `MainActor` for `Observable` state or equivalent thread-safety). Also ensure test seams/injection are available so the persistence boundary can be controlled in tests.
Under these conditions, patterns like the ones used in `Sources/OfflineNotesStore.swift` (e.g., `MainActor Observable` + `static let shared`) are consistent with existing precedents; otherwise, prefer per-window instances or explicit coordination to prevent persistence races.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 6905
File: Sources/OfflineNotesStore.swift:0-0
Timestamp: 2026-06-26T12:31:53.114Z
Learning: When reviewing Swift code in this repo, an app-global `Observable` singleton (e.g., `static let shared`) can be acceptable if it truly *owns* a single persisted app-wide backing resource, so creating multiple window-scoped instances would otherwise race on the same store. Treat the pattern as non-problematic (don’t auto-flag) when all/most of these hold: (1) the singleton is the sole owner of one persisted resource (e.g., one JSON file / one shared store), (2) window-scoped instances would contend for the same underlying storage, (3) the singleton is annotated appropriately for concurrency (often `MainActor` when used on the main actor), (4) it supports testability (e.g., is injectable or can be swapped in tests), and (5) any reachability/background work is started lazily when the relevant UI/panel opens. Use the existing app-global precedents (e.g., `PaneMemoryGuardrail`, `DiffCommentStore.shared`) as confirmation of this intended pattern. Example: `Sources/OfflineNotesStore.swift` can remain `MainActor Observable static let shared` when it persists one offline-notes JSON file app-wide and lazily begins its work when the Notes panel opens.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 4608
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-07-01T22:19:25.948Z
Learning: When reviewing Swift localization, do not require `String(localized:)` for v1 socket-protocol command reply strings used by the cmux CLI/hooks/tests (i.e., reply literals over the wire that are machine-parsed and must match exact text like `OK` / `ERROR: ...` or command “usage” text). Localizing these reply strings can break scripted clients and tests that match the literal replies. Only flag missing localization for genuine UI-visible copy (e.g., actual notification/display text), which should follow the project’s localization approach.
…dget workflow-guard-tests failed: the review fixes put AppDelegate.swift 4 lines over its exact-at-budget length. Move the plus-button context-menu cluster (showNewWorkspaceContextMenu, performNewWorkspaceContextMenuItem, NewWorkspaceContextMenuActionBox) verbatim into Sources/AppDelegate+NewWorkspaceContextMenu.swift, wired into the app target. preferredMainWindowContextForWorkspaceCreation and executeConfiguredCmuxAction widen private -> internal for cross-file access; behavior unchanged. AppDelegate.swift: 17958 -> 17881 (budget 17954). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The plus-button menu's "Customize Actions…" reused the sidebar's external-editor opener, whose OS-default fallback for .json can be Xcode (triggering its components-install dialog). Open cmux.json in an in-app file editor tab in the current workspace instead, reusing the shared openFileSurfaces path; the external opener remains the fallback when no workspace context exists. Config materialization is factored into SidebarWorkspaceGroupConfigOpener.materializedCmuxConfigURL so both paths share it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/AppDelegate+WorkspaceActionSave.swift (2)
102-116: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
String.localizedStringWithFormatinstead ofString(format:)for these plain-string localized templates.
messageFormatandskippedFormatare obtained via the plain-Stringoverload ofString(localized:)and then formatted separately withString(format:). Based on learnings,String.localizedStringWithFormatshould be used for this exact pattern (olderString(localized:)overload + separately-passed format args), rather thanString(format:).♻️ Suggested fix
- var message = String( - format: messageFormat, - (globalConfigPath as NSString).abbreviatingWithTildeInPath - ) + var message = String.localizedStringWithFormat( + messageFormat, + (globalConfigPath as NSString).abbreviatingWithTildeInPath + ) if snapshot.skippedPanelCount > 0 { let skippedFormat = String( localized: "dialog.saveWorkspaceAction.skippedNote", defaultValue: "%lld panels have no layout representation (previews, viewers, …) and will be left out." ) - message += "\n\n" + String(format: skippedFormat, Int64(snapshot.skippedPanelCount)) + message += "\n\n" + String.localizedStringWithFormat(skippedFormat, Int64(snapshot.skippedPanelCount)) }Based on learnings, "Only require
String.localizedStringWithFormatwhen using the olderString(localized:)overload that takes a plainString... where format arguments must be passed separately."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/AppDelegate`+WorkspaceActionSave.swift around lines 102 - 116, In AppDelegate+WorkspaceActionSave, the localized plain-string templates for the workspace save message are still being formatted with String(format:); switch both the messageFormat and skippedFormat usage to String.localizedStringWithFormat when applying the path and panel-count arguments. Keep the existing String(localized:) lookups, but replace the separate formatting calls in the save-workspace action flow so the formatting matches the intended localization pattern.Source: Learnings
68-88: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd a timeout around
waitForFreshIndex()The save dialog is gated on an unboundedawait; if the refresh task wedges, the user gets no dialog or progress feedback. A short timeout/fallback would keep the action responsive.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/AppDelegate`+WorkspaceActionSave.swift around lines 68 - 88, The save dialog flow in presentSaveWorkspaceActionDialog(context:) can hang on the unbounded waitForFreshIndex() call, blocking the UI with no feedback. Add a short timeout or fallback around SharedLiveAgentIndex.shared.waitForFreshIndex() inside the Task so the dialog still opens if the refresh does not complete promptly, and keep the existing workspace/window guards in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/AppDelegate`+WorkspaceActionSave.swift:
- Around line 47-66: The guard in customizeCmuxConfigActionsMenuItem mixes a
side effect into its boolean checks by calling workspace.openFileSurfaces(...)
as the final condition. Move that call out of the guard in
AppDelegate+WorkspaceActionSave so the guard only validates
sender.representedObject, mainWindowContexts, selectedWorkspace, and
focusedPaneId; then invoke openFileSurfaces(...) in a separate step before
deciding whether to fall back to
SidebarWorkspaceGroupConfigOpener.openCmuxConfigInEditor(). Keep the existing
fallback behavior unchanged.
---
Outside diff comments:
In `@Sources/AppDelegate`+WorkspaceActionSave.swift:
- Around line 102-116: In AppDelegate+WorkspaceActionSave, the localized
plain-string templates for the workspace save message are still being formatted
with String(format:); switch both the messageFormat and skippedFormat usage to
String.localizedStringWithFormat when applying the path and panel-count
arguments. Keep the existing String(localized:) lookups, but replace the
separate formatting calls in the save-workspace action flow so the formatting
matches the intended localization pattern.
- Around line 68-88: The save dialog flow in
presentSaveWorkspaceActionDialog(context:) can hang on the unbounded
waitForFreshIndex() call, blocking the UI with no feedback. Add a short timeout
or fallback around SharedLiveAgentIndex.shared.waitForFreshIndex() inside the
Task so the dialog still opens if the refresh does not complete promptly, and
keep the existing workspace/window guards in place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3af3b4c3-6a4d-4f52-92cf-3ed6265583a5
📒 Files selected for processing (2)
Sources/AppDelegate+WorkspaceActionSave.swiftSources/SidebarWorkspaceGroupConfigOpener.swift
…post-save reload - The workspace-action trust dialog now discloses every shell string the action will run (setup + each surface command) instead of only the benign action name; covered by a disclosure test. - waitForFreshIndex treats a pending coalesced hook-store change (changePending / deferredReloadTask) as dirty and forces the reload, so saving within the 2s coalescing window can't capture a stale agent index. - Save Workspace as Action calls cmuxConfigStore.loadAll() after a successful write: the app's store runs without file watchers, so the saved action now appears in the plus-button menu and palette immediately, as the dialog promises. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dogfood: saved actions only kept the layout — claude/codex/etc running in panes were dropped because capture depended solely on the hook-driven agent index. Capture now reads each terminal's foreground process (CmuxTopProcessSnapshot surface attribution + KERN_PROCARGS2 argv, shared parser widened from TerminalSSHSessionDetector): argv[0] is basenamed, known agent resume flags are stripped so relaunches are fresh, arguments are shell-quoted, and any foreground command (htop, npm run dev, …) is saved — with the agent index as fallback. Also fixes two codex-review findings: - inline workspace actions/buttons now carry `confirm` into the synthetic command (shared inlineWorkspaceSyntheticCommand on the resolved action, mirrored on buttons) - saved action ids are uniquified against the active store's resolved ids so project-local actions can't shadow the new global action Saver + foreground-capture tests split into CmuxConfigActionSaverTests (wired in pbxproj) to satisfy the file length budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…alog Two codex-review findings on the foreground-command capture: - the executable token is now shell-quoted like every argument, so a spaced or metacharacter-bearing basename can't be replayed as shell syntax - the Save Workspace as Action dialog lists every command the action will persist and re-run (new localized header, en+ja), so secret-bearing foreground argv is never written to cmux.json without the user seeing it verbatim first Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 5: - Save Workspace as Action now also discloses browser/project URLs (OAuth codes, presigned params travel in URLs) and the names of environment variables whose values will be persisted, alongside the command list (headers localized en+ja). - The project-action trust dialog title is now passed through sanitizeForDisplay like the command body, so a project-local action title carrying bidi/zero-width controls can't spoof the header. The review's claim that disclosure lines render unsanitized was refuted: makeConfirmDialog already sanitizes the joined command string; the unsanitized surface was the title. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 6: basenaming argv[0] broke path-qualified invocations — ./gradlew test saved as "gradlew test" and replayed a different (or missing) executable. argv[0] is the form the user invoked (shells pass the typed word), so keep it verbatim and shell-quoted; panes replay from their saved cwd, which makes relative forms work. Agent resume-stripping and the shell filter now key off the basename only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 7: - cmux.json is chmod 0600 after every save (atomic rewrites reset to the umask default) and the config directory is created 0700; the materialized empty config starts owner-only too. Saved actions can carry env values, URLs, and command lines. Permission asserted in the saver test. - The project-local trust prompt now discloses workspace-level and per-surface env assignments alongside setup/surface commands — ZDOTDIR/BASH_ENV/PATH-style keys change what executes, so they are part of what the user approves. Disclosure helpers move to CmuxConfigExecutor+WorkspaceLaunch.swift for the length budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… env Codex-review round 8: - Captured agent argv now goes through AgentLaunchSanitizer (the same provider-aware policies agent restore uses) instead of a duplicate strip list, so Amp thread continuations, --continue=/session forms, and other stale-session artifacts never replay from saved actions; non-restorable launch forms fall back to the bare CLI. - Save Workspace as Action no longer copies workspaceEnvironment into the saved action: values can be secrets and the dialog can only disclose keys. Users who want env persisted add it via Customize Actions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 9:
- knownAgentKind maps alias basenames (agy, cursor-agent, hermes, omp,
acli) and grok-* arch builds to their sanitizer kinds so foreground
agents launched under different binary names still get stale-session
stripping; covered with an agy --continue test against the shared
sanitizer.
- The trust prompt now discloses workspace-level cwd and per-surface
cwd ("cwd /tmp/target: rm -rf ./scratch") since cwd controls where
disclosed commands run.
- The Save dialog's typed name now becomes the saved workspace's name,
so launching the action creates/matches the workspace shown in the
menu instead of the captured customTitle.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 10: RestorableAgentKind.allCases intentionally omits the registry-owned kinds, so exact pi/grok/antigravity foreground commands skipped the sanitizer and could persist resume/session flags. Add them to knownAgentExecutables explicitly, with regression coverage for the exact names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 11: - Foreground command capture no longer trusts the child process's CMUX_SURFACE_ID/CMUX_PANEL_ID environment (spoofable/stale). Identity now flows from the workspace's own panel->tty registry (surfaceTTYNames) joined to processes by tty device id, matching the repo's no-heuristic-identity policy; the env-scoped scan is gone. - Auto-appended plus-menu actions with type workspaceCommand now run the same named-command validation as explicit contextMenu entries: dead references are skipped and surface as configuration issues instead of no-op menu items. Store-level test added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Comment on lines
+1318
to
+1327
| /// survive the wrap. | ||
| var inlineWorkspaceSyntheticCommand: CmuxCommandDefinition? { | ||
| guard let inline = action.inlineWorkspace else { return nil } | ||
| return CmuxCommandDefinition( | ||
| name: title, | ||
| restart: inline.restart, | ||
| workspace: inline.definition, | ||
| confirm: confirm | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
name in inline workspace synthetic command breaks restart matching
CmuxResolvedConfigAction.inlineWorkspaceSyntheticCommand passes name: title directly, but title is String?. When both the action's title and the embedded definition.name are absent, executeWorkspaceCommand resolves workspaceName = wsDef.name ?? command.name to nil. The tab-scan then becomes tabs.first(where: { $0.customTitle == nil }), which matches ANY unnamed workspace — so restart: "recreate" would silently close an unrelated workspace, and restart: "ignore" would focus a random one instead of creating a new workspace.
The CmuxSurfaceTabBarButton counterpart already guards against this with title ?? tooltip ?? inline.definition.name ?? id. The action version needs the same fallback.
Suggested change
| /// survive the wrap. | |
| var inlineWorkspaceSyntheticCommand: CmuxCommandDefinition? { | |
| guard let inline = action.inlineWorkspace else { return nil } | |
| return CmuxCommandDefinition( | |
| name: title, | |
| restart: inline.restart, | |
| workspace: inline.definition, | |
| confirm: confirm | |
| ) | |
| } | |
| var inlineWorkspaceSyntheticCommand: CmuxCommandDefinition? { | |
| guard let inline = action.inlineWorkspace else { return nil } | |
| return CmuxCommandDefinition( | |
| name: title ?? inline.definition.name ?? id, | |
| restart: inline.restart, | |
| workspace: inline.definition, | |
| confirm: confirm | |
| ) | |
| } |
Contributor
Author
There was a problem hiding this comment.
Not applicable to this type: CmuxResolvedConfigAction.title is a non-optional String populated by fromDefinition via definition.title ?? definition.tooltip ?? defaultTitle(for:action:) (Sources/CmuxConfig.swift:1358-1366), so the synthetic command name can never be nil here. The title ?? tooltip ?? inline.definition.name ?? id chain exists on the button variant because CmuxSurfaceTabBarButton.title is optional; the resolved-action variant already has a stronger guarantee upstream.
— Claude Code
Contributor
There was a problem hiding this comment.
You're correct — I was wrong about this. The let title: String declaration confirmed by fromDefinition's definition.title ?? definition.tooltip ?? defaultTitle(for:action:) chain means title is always a non-nil String on CmuxResolvedConfigAction, so name: title in the synthetic command can never produce a nil name. The longer fallback chain on CmuxSurfaceTabBarButton exists precisely because that type's title is optional. My original concern doesn't apply here.
…lock Codex-review round 15: - The trust prompt labels setup with the cwd it actually executes in (the first terminal surface's cwd overrides the workspace cwd), and cwd-only terminal surfaces are disclosed too. - saveWorkspaceAction refuses when an existing config has a non-object "actions" value instead of letting the JSONC upsert replace (and lose) the user's content; regression test asserts the file survives byte-identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nfig Dogfood: saved actions had no UI removal path. The plus-button menu now has a Delete Action submenu listing global-config actions, and holding Option turns any deletable saved action row into its delete affordance (native alternate items). Deleting confirms with a destructive-styled dialog, removes the entry via a new comment-preserving JSONCObjectEditor.removeNestedObjectProperty (refuses ambiguous shapes like block-comment separators instead of risking user content), writes through the shared owner-only config writer, and reloads the store so menus update immediately. Strings localized en+ja. Also from codex-review round 16: - save/delete fail closed when the existing config does not fully parse or its "actions" value is not an object — structural edits can never replace broken user-authored content (regression tests keep the file byte-identical). - the new test files move from XCTest to Swift Testing per the repo test policy, with the plus-menu store tests split into CmuxConfigNewWorkspaceMenuTests to stay under the file length budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 17: deleting the last action stored the preceding comma's indices from the original string and applied them to the already-edited copy — String.Index values are not valid across string instances and can trap or mis-splice. Both removals now happen in a single reconstruction from original-string slices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…refactor Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex-review round 18: the cwd/url prefixes in the workspace trust dialog are user-visible text and must route through String(localized:). Format keys added with en+ja entries; env KEY=value lines stay literal config syntax. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # Resources/Localizable.xcstrings # Sources/AppDelegate.swift # Sources/Workspace.swift
… new workspaces User-facing rename of the plus-menu save/delete/customize feature from "action" wording to "workspace layout" wording (the pre-existing `actions` config key and internal identifiers are unchanged; en+ja localization keys moved), retitle the sibling saved-template submenu to "New Workspace from Template" for disambiguation, and add a "Default for New Workspace" affordance: a plus-menu submenu and a save-dialog checkbox that set/unset `ui.newWorkspace.action` in cmux.json through a new comment-preserving JSONC string setter (JSONCObjectEditor+Set), wired into both targets that compile the JSONC editor family. Schema documents ui.newWorkspace.action; docs gain a "Default for new workspaces" subsection (en+ja). Coded by GPT 5.5 (codex exec) against the fable plan; fable-judge verdict APPROVE. Orchestrator touch-ups: split a tuple-array == assertion the compiler rejects and refreshed a stale doc comment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
austinywang
changed the title
Dogfood feedback on the Default for New Workspace submenu: it offered every loaded action (built-ins, agent/command actions), and rows had no icons. The menu model now filters to workspace-creating actions using the same predicate as the executor (workspace command reference or inline workspace definition), still surfacing a hand-edited non-layout default as a checked row so a set default is never displayed as unchecked. Rows get their icons through the same actionLookup + contextMenuImage path the delete submenu uses. fable-judge verdict APPROVE; codex coder unavailable this round (revoked auth), fixes applied by the orchestrator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
CI workflow-guard-tests failed on the Swift file length budget: cmuxTests/CmuxConfigWorkspaceActionTests.swift grew past the 500-line threshold (596 before the branch's submenu-filtering test, 640 after) while untracked in the budget. Move the default-workspace-layout persistence and menu-model tests -- including the filtering test added on the branch meanwhile -- plus the two helpers only they use into a new CmuxConfigNewWorkspaceDefaultLayoutTests suite wired into the cmuxTests target (376 + 349 lines afterwards). Also adds two deleteAction default-clearing tests that are red at this commit; the next commit makes them green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review finding (P3): CmuxConfigActionSaver.deleteAction removed only actions.<id>, so deleting the layout configured as ui.newWorkspace.action left a dangling default: new-workspace creation fell back to a blank terminal and a config issue warned until the user hand-edited cmux.json. Detect the match on the validated source and clear the key through the comment-preserving JSONC remove editor inside the same atomic write; unrelated defaults are untouched. Turns the two suite tests from the previous commit green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Dogfood feedback: "Save as Workspace Layout…" didn't say what gets saved. The menu item (en+ja), schema description, and docs copy now lead with the workspace as the subject. Dialog strings already say "Saves this workspace…" and are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 7, 2026
This was referenced Jul 14, 2026
This branch was successfully deployed
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
1 participant
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.
What
Workspace layouts: save the live workspace as a reusable layout, manage it from the
+menu, and optionally make one the default for plain New Workspace.actionsentries can use"type": "workspace"with an embedded workspace definition (name/cwd/color/env/layout + optionalrestart), no separatecommandsentry required.setupbootstrap command — workspace definitions support"setup", sent to the workspace's first terminal ahead of that terminal's owncommand."type": "agent"accepts any CLI name (claude,codex,opencode, or your own binary).opencodegets a default icon/title.+button right-click menu automatically; opt in/out withnewWorkspaceMenu: true|false. Explicitui.newWorkspace.contextMenuentries are never duplicated.+-menu item captures the live workspace (split tree with divider ratios, per-panel cwds, detected running agent CLIs, browser URLs, project panels, focused panel) and writes it into~/.config/cmux/cmux.json, preserving JSONC comments/formatting. Customize Workspace Layouts… opens the config in cmux's own editor.+-menu submenu and ⌥-alternate rows remove saved layouts via comment-preserving JSONC removal, with a confirmation dialog.+-menu submenu (checkmark on the current default, "None (Blank Terminal)" to clear) and a save-dialog checkbox ("Use as default for new workspaces") set/unsetui.newWorkspace.actionin the global cmux.json through one shared, comment-preserving write path (JSONCObjectEditor+Set). The runtime hook already existed: plain New Workspace runs the configured action viaexecuteConfiguredNewWorkspaceActionIfAvailable, with project-local config taking precedence over global. Schema now documentsui.newWorkspace.action.How it executes
Inline workspace actions wrap into a synthetic
CmuxCommandDefinitionand reuse the existingCmuxConfigExecutor.execute(command:)path, so trust prompts, restart matching, and layout application behave identically to named workspace commands. The default-layout write path is fail-closed: config is validated before edit, writes go through the owner-only temp-file+rename path, and malformed configs are never modified.Tests
CmuxConfigWorkspaceActionTests(21 tests): everything from the previous rounds plus the default-layout coverage — JSONC setter creates/replacesui.newWorkspace.actionwhile preserving comments, partial-path creation, removal incl. absent-key no-op, fail-closed on unparseable config and non-object path segments, save-then-set-default flow, and theNewWorkspaceDefaultLayoutMenuModelbuilder (sorted entries, checkmark state, dangling-id case). Full focused run green: 42 Swift Testing tests across the config action/saver/menu suites plus 12 XCTest context-menu tests, 0 failures. Test file wired into pbxproj;lint-pbxproj-test-wiring.shpasses.Docs / localization
web/data/cmux.schema.json:ui.newWorkspace.actiondocumented; feature descriptions use "workspace layout" wording.docs/custom-commandspage: section renamed to Workspace layouts (with a note that layouts are stored as entries in theactionsblock) + new "Default for new workspaces" subsection — keys added toen.json+ja.json.Localizable.xcstringswith en+ja pairs (26 keys audited, old keys removed); sibling submenu retitle localized in both languages.Process
Implemented via the fable loop: planned in
/tmp/fable-workspace-layout/PLAN.md, coded by GPT 5.5 (codex, high effort), reviewed by the fable-judge subagent (verdict: APPROVE). Orchestrator applied two disclosed touch-ups: fixed a tuple-array==compile error in the new tests and refreshed one stale doc comment.🤖 Generated with Claude Code