Repository navigation
Conversation
Add two flags to `cmux markdown open`: - `--pane <id|ref|index>`: Open markdown as a tab in an existing pane instead of creating a new split. Uses the existing `Workspace.newMarkdownSurface(inPane:)` method that was already implemented but not exposed through the CLI/socket API. - `--no-focus`: Open the markdown panel without stealing focus from the current surface. Wires through the existing `v2FocusAllowed(requested:)` pattern used by other commands like break-pane and join-pane. Both flags can be combined: `cmux markdown open plan.md --pane pane:5 --no-focus` Closes manaflow-ai#1457 Addresses manaflow-ai#2770, manaflow-ai#2820
|
@chi-feng is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughExtended Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as CLI Parser
participant Handler as TerminalController
participant Pane as Target Pane
participant Surface as Markdown Surface
User->>CLI: cmux markdown open file.md --pane pane:5 [--no-focus]
CLI->>CLI: Parse --pane and --no-focus flags\nBuild params (pane_id?, focus)
CLI->>Handler: v2MarkdownOpen request with params
alt Pane Mode (pane_id provided)
Handler->>Pane: Validate pane exists
Handler->>Surface: Create markdown surface as tab in Pane
Handler->>Handler: Set source_pane_id, source_surface_id = null
else Split Mode (pane_id absent)
Handler->>Handler: Resolve source surface (surface_id or workspace focused)
Handler->>Surface: Create markdown surface via split (direction)
Handler->>Handler: Set source_surface_id
end
Handler->>Handler: Apply focus based on params.focus
Handler->>User: Return markdown_panel_id, source_pane_id/source_surface_id
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 2994-2999: The code sets params["pane_id"] directly from
normalizePaneHandle result which can be a ref like "pane:<n>" but markdown pane
mode expects a UUID; update the logic around paneOpt handling (where
normalizePaneHandle is used) to detect non-UUID handles and resolve them to
UUIDs by listing panes via the SocketClient (use client.sendV2("pane.list",
params: ["workspace_id": wsHandle]) and find the matching pane by ref or id),
then set params["pane_id"] to the resolved UUID; you can encapsulate this in a
helper like resolvePaneUUIDHandle(handle: String, workspaceHandle: String?,
client: SocketClient) and call it before assigning params["pane_id"], throwing a
CLIError if no match is found.
In `@Sources/TerminalController.swift`:
- Around line 7619-7640: The code currently only uses v2UUID(params, "pane_id")
(requestedPaneUUID) which accepts raw UUIDs and thus treats any present but
non-UUID pane handle (ref/index) as absent; change the lookup to resolve the
pane via the pane-handle path instead of only v2UUID (i.e., call the existing
pane-handle resolver used elsewhere — the same logic that maps id|ref|index to a
Pane id in ws.bonsplitController or the shared handle-resolution helper) so that
a provided pane_id is properly resolved to a pane UUID; if the key is present
but resolution fails, set result = .err(code: "invalid_params", message:
"Invalid pane_id", data: ["pane_id": providedValue]) and return (do not fall
back to split mode), otherwise proceed using the resolved pane UUID where
requestedPaneUUID, paneId, sourcePaneUUID and markdownPanelId are used.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: c3f7a7b1-3cbd-498f-ade5-35e58e314417
📒 Files selected for processing (2)
CLI/cmux.swiftSources/TerminalController.swift
Comment on lines
+2994
to
+2999
| if let paneRaw = paneOpt { | ||
| let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] | ||
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) { | ||
| params["pane_id"] = pane | ||
| } | ||
| } |
There was a problem hiding this comment.
--pane may send a pane ref, but markdown pane mode expects a pane UUID
Line 2996 can resolve to pane:<n>, and Line 2997 forwards it as pane_id. In Sources/TerminalController.swift (Line 7619-7637), markdown pane mode resolves pane_id as UUID and validates UUID ownership, so ref-form handles can fail or misroute.
Suggested fix
- if let paneRaw = paneOpt {
- let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"]
- if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) {
- params["pane_id"] = pane
- }
- }
+ if let paneRaw = paneOpt {
+ let wsHandle = params["workspace_id"] as? String
+ if let paneHandle = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) {
+ params["pane_id"] = try resolvePaneUUIDHandle(
+ paneHandle,
+ workspaceHandle: wsHandle,
+ client: client
+ )
+ }
+ }private func resolvePaneUUIDHandle(_ handle: String, workspaceHandle: String?, client: SocketClient) throws -> String {
if isUUID(handle) { return handle }
var params: [String: Any] = [:]
if let workspaceHandle { params["workspace_id"] = workspaceHandle }
let listed = try client.sendV2(method: "pane.list", params: params)
let panes = listed["panes"] as? [[String: Any]] ?? []
if let match = panes.first(where: { ($0["ref"] as? String) == handle || ($0["id"] as? String) == handle }),
let paneId = match["id"] as? String {
return paneId
}
throw CLIError(message: "Pane handle not found: \(handle)")
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLI/cmux.swift` around lines 2994 - 2999, The code sets params["pane_id"]
directly from normalizePaneHandle result which can be a ref like "pane:<n>" but
markdown pane mode expects a UUID; update the logic around paneOpt handling
(where normalizePaneHandle is used) to detect non-UUID handles and resolve them
to UUIDs by listing panes via the SocketClient (use client.sendV2("pane.list",
params: ["workspace_id": wsHandle]) and find the matching pane by ref or id),
then set params["pane_id"] to the resolved UUID; you can encapsulate this in a
helper like resolvePaneUUIDHandle(handle: String, workspaceHandle: String?,
client: SocketClient) and call it before assigning params["pane_id"], throwing a
CLIError if no match is found.
Author
There was a problem hiding this comment.
v2UUID already handles refs — it calls v2ResolveHandleRef (TerminalController.swift:3124) which looks up the ref-to-UUID map. Tested with --pane pane:10 and it resolves correctly. This is the same pattern used by surface.create, pane.break, and surface.move.
There was a problem hiding this comment.
@chi-feng Thanks — agreed. Since TerminalController’s v2UUID calls v2ResolveHandleRef, passing pane: is valid and gets coerced to the pane UUID on the server side. That matches the existing pattern used by surface.create, pane.break, and surface.move. No change needed here; I’m retracting the earlier suggestion.
✏️ Learnings added
Learnt from: chi-feng
Repo: manaflow-ai/cmux PR: 2821
File: CLI/cmux.swift:2994-2998
Timestamp: 2026-04-11T18:54:42.158Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2 endpoints accept handle refs (e.g., "pane:<n>") for inputs like pane_id and resolve them to UUIDs via v2UUID → v2ResolveHandleRef. CLI commands (including markdown open --pane) can safely forward pane refs without pre-resolving to UUIDs, consistent with surface.create, pane.break, and surface.move.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:0-0
Timestamp: 2026-03-16T08:02:13.558Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, socket v2 methods that accept surface_id/panel_id must handle cross-window routing. For panel background state mutations, v2PanelMarkBackground(params:) and v2PanelMarkForeground(params:) first try v2ResolveTabManager(params:), but only accept it if that manager actually owns the panelId; otherwise they fall back to AppDelegate.shared?.locateSurface(surfaceId:) to find the correct TabManager across windows. This pattern should be used for any panel_id-only routes to avoid active-window bias.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1958-1960
Timestamp: 2026-03-25T00:33:00.675Z
Learning: Repo: manaflow-ai/cmux — CLI/cmux.swift set-workspace-color intentionally accepts both "#RRGGBB" and "RRGGBB" for ergonomics, matching server-side WorkspaceTabColorSettings.normalizedHex/TabManager normalization. Do not suggest enforcing a mandatory leading '#'; at most, suggest clarifying help/usage text.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3607-3645
Timestamp: 2026-03-25T00:32:48.115Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceSetColor(params:) must validate the provided color with WorkspaceTabColorSettings.normalizedHex. If the color parameter is present but invalid, return invalid_params (no fallback). On success, apply the color and return the normalized (uppercase) `#RRGGBB` in the response payload.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:679-709
Timestamp: 2026-04-05T20:54:35.429Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift session restore, SessionTerminalPanelSnapshot now includes isRemoteBacked. createPanel(from:inPane:) uses this per-panel flag (panelWasRemoteBacked) to: (1) pass initialInput only for local panels, and (2) persist panelRestoreCommands only for local panels. Remote-backed terminals also omit restore/detected commands and have listeningPorts [] in the snapshot.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1948-1978
Timestamp: 2026-03-25T00:33:26.452Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift, the set-workspace-color command requires exactly one trailing <hex> argument and enforces a strict 6-digit hex format (`#RRGGBB`, optional leading '#'); clear-workspace-color rejects any unexpected positional args beyond --workspace. This matches server-side normalization and prevents malformed inputs.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2398
File: CLI/cmux.swift:0-0
Timestamp: 2026-04-01T09:50:23.728Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift within CMUXCLI.buildInteractiveRemoteShellScript(...), never export CMUX_TAB_ID from the workspace UUID. CMUX_TAB_ID must be surface-scoped: only set it when a surface ID is available (map CMUX_TAB_ID to CMUX_SURFACE_ID). Rationale: tab-action/rename-tab resolve CMUX_TAB_ID before CMUX_SURFACE_ID; workspace-scoped values misroute or fail.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2398
File: CLI/cmux.swift:4036-4038
Timestamp: 2026-04-01T09:50:41.471Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift (buildInteractiveRemoteShellScript), remote shells intentionally export CMUX_SOCKET_PATH and CMUX_SOCKET as "127.0.0.1:<relayPort>" and prepend PATH with "$HOME/.cmux/bin"; CMUX_BUNDLED_CLI_PATH points to that wrapper. WorkspaceRemoteSessionController.remoteCLIWrapperScript() detects host:port, reads "~/.cmux/relay/<port>.daemon_path", and delegates to cmuxd-remote. Do not flag the non-UNIX value of CMUX_SOCKET_PATH/CMUX_SOCKET as a bug; it is the relay contract for remote sessions.
Learnt from: arieltobiana
Repo: manaflow-ai/cmux PR: 1873
File: Sources/TerminalController.swift:4071-4087
Timestamp: 2026-03-20T17:18:30.333Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceAction(params:) -> case "set_color": palette names are resolved via WorkspaceTabColorSettings.defaultPaletteWithOverrides(), whose entries are always valid hex (validated by the UI). Therefore, additional normalization of entry.hex is unnecessary.
Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:01:33.134Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, sendPickedElementToTerminal(workspaceId:summary:) resolves the target workspace via AppDelegate.shared?.workspaceFor(tabId: workspaceId), which searches across all mainWindowContexts (not self.tabManager which is active-window only). It first injects text into the focused terminal panel if it's a terminal, then falls back to iterating all panels in the workspace.
Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/ContentView.swift:2156-2171
Timestamp: 2026-03-23T06:08:14.740Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId. Rationale: avoid dropping file-open requests when the focused panel is not pane-backed.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:0-0
Timestamp: 2026-04-06T18:23:54.205Z
Learning: Repo: manaflow-ai/cmux — Sources/Workspace.swift: When promoting an existing local terminal to remote-backed in Workspace.seedInitialRemoteTerminalSessionIfNeeded(_), clear both surfaceTTYNames[panelId] and surfaceListeningPorts[panelId] before calling trackRemoteTerminalSurface(_:) to avoid stale local TTY/port metadata in the workspace UI.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:4012-4023
Timestamp: 2026-03-16T08:05:21.899Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2SurfaceSplitSized(params:) must validate "ratio" as follows: if the "ratio" key is present, it must be numeric (NSNumber/Double-coercible) and strictly 0 < ratio < 1, otherwise return invalid_params; when "ratio" is absent, use default 0.6. This mirrors the general pattern that a present-but-invalid param should yield invalid_params rather than falling back.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2398
File: Sources/Workspace.swift:4965-4988
Timestamp: 2026-04-01T09:51:45.737Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, WorkspaceRemoteSessionController.updateRemotePortScanTTYsLocked(_:) must drop remoteScannedPortsByPanel entries when a panel’s TTY changes (compare previousTTYNames vs nextTTYNames). It should also use keepPolledRemotePortsUntilTTYScan to retain the last host-wide polling snapshot only until the first TTY-scoped scan completes, and clear it when no TTYs are tracked.
Learnt from: pstanton237
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-06T12:03:16.921Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift, `claudeTeamsLaunchArguments` must use two independent branches: (1) prepend `--teammate-mode auto` only when the caller has NOT already supplied `--teammate-mode`; (2) append `--settings <claudeHooksJSON>` unconditionally (modulo `CMUX_CLAUDE_HOOKS_DISABLED=1` opt-out). These two branches must never be coupled in a single early-return or conditional block — coupling them was the original `#2229` regression (hook JSON silently bypassed when `--teammate-mode auto|manual` was explicit). Mirrors `Resources/bin/claude:208`: `exec "$REAL_CLAUDE" --settings "$HOOKS_JSON" "$@"` where `$@` content never gates hook injection.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2564
File: Sources/Workspace.swift:1072-1075
Timestamp: 2026-04-04T02:21:20.453Z
Learning: Repo: manaflow-ai/cmux — Foreground-auth deferral: In CLI/cmux.swift, PermitLocalCommand and LocalCommand are injected only into the foreground startup SSH command (e.g., via deferredRemoteReconnectLocalCommand(...)) and are not added to the ssh_options payload sent in workspace.remote.configure. Consequently, WorkspaceRemoteSSHBatchCommandBuilder.batchArguments(...) and WorkspaceRemoteSessionController.backgroundSSHOptions(_:) do not need to filter LocalCommand/PermitLocalCommand for batch operations.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2564
File: CLI/cmux.swift:0-0
Timestamp: 2026-04-04T02:33:06.558Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift, baseSSHArguments(_:, localCommand:) now percent-escapes LocalCommand by replacing "%" with "%%" to prevent OpenSSH percent-token expansion. A CLI regression test asserts that the emitted -o LocalCommand retains doubled percent signs.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2475
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-03T07:19:36.497Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, the command‑palette rename flow uses a single-line SwiftUI TextField via commandPaletteEditorField(style: .singleLine(...)); the multiline NSTextView editor is only used for the workspace description input. Do not flag newline persistence for rename.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.710Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:51.778Z
Learning: For Claude Code session tag extraction in CLI/cmux.swift, in ClaudeHookTagExtractor.extractTags(subtitle:body:), pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-08T03:36:30.160Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.FileBrowserDrawerState threading pattern (PR `#1909`, commit e0e57809): FileBrowserDrawerState must be threaded through AppDelegate.configure() as a weak stored property (matching the sidebarState pattern), passed through both configure() call sites, with registerMainWindow parameter made non-optional. The fallback `?? FileBrowserDrawerState()` must NOT be used as it creates detached instances that are not properly owned by the window context.
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: 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: lawrencecchen
Repo: manaflow-ai/cmux PR: 2514
File: CLI/cmux.swift:9698-9706
Timestamp: 2026-04-01T23:08:15.505Z
Learning: When loading/using a user-provided “custom executable” path in the CLI (e.g., from environment/config/UserDefaults), treat it as an executable file path only if it is NOT a directory and is executable. Concretely, check `isDirectory == false` before checking `isExecutableFile`; this avoids accepting directory-valued paths and should allow safe fallback to PATH or bundled/default executables when the candidate is invalid.
Comment on lines
+7619
to
+7640
| let requestedPaneUUID = v2UUID(params, "pane_id") | ||
|
|
||
| let sourcePaneUUID = ws.paneId(forPanelId: sourceSurfaceId)?.id | ||
| let markdownPanelId: UUID? | ||
| let sourceSurfaceId: UUID? | ||
| let sourcePaneUUID: UUID? | ||
|
|
||
| let directionStr = v2String(params, "direction") ?? "right" | ||
| guard let direction = parseSplitDirection(directionStr) else { | ||
| result = .err(code: "invalid_params", message: "Invalid direction '\(directionStr)' (left|right|up|down)", data: nil) | ||
| return | ||
| } | ||
| let orientation: SplitOrientation = direction.isHorizontal ? .horizontal : .vertical | ||
| let insertFirst = (direction == .left || direction == .up) | ||
| if let requestedPaneUUID { | ||
| // --pane mode: add as tab in existing pane | ||
| guard let paneId = ws.bonsplitController.allPaneIds.first(where: { $0.id == requestedPaneUUID }) else { | ||
| result = .err(code: "not_found", message: "Pane not found", data: ["pane_id": requestedPaneUUID.uuidString]) | ||
| return | ||
| } | ||
| sourceSurfaceId = nil | ||
| sourcePaneUUID = paneId.id | ||
| markdownPanelId = ws.newMarkdownSurface( | ||
| inPane: paneId, | ||
| filePath: filePath, | ||
| focus: focus | ||
| )?.id | ||
| } else { | ||
| // Default: split from source surface | ||
| let resolvedSourceId = v2UUID(params, "surface_id") ?? ws.focusedPanelId |
There was a problem hiding this comment.
pane_id is parsed too narrowly here.
Line 7619 only accepts a raw UUID, but this command now advertises --pane <id|ref|index>. If the client sends a pane ref/index (for example the documented pane:5 form), requestedPaneUUID becomes nil, Line 7625 is skipped, and Line 7640 silently falls back to split mode instead of honoring --pane or rejecting the bad handle. Resolve pane_id through the pane-handle lookup path, and if the key is present but unresolvable, return invalid_params rather than treating it as absent.
Based on learnings: if a parameter key is present but invalid, the API should return invalid_params rather than silently falling back.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 7619 - 7640, The code
currently only uses v2UUID(params, "pane_id") (requestedPaneUUID) which accepts
raw UUIDs and thus treats any present but non-UUID pane handle (ref/index) as
absent; change the lookup to resolve the pane via the pane-handle path instead
of only v2UUID (i.e., call the existing pane-handle resolver used elsewhere —
the same logic that maps id|ref|index to a Pane id in ws.bonsplitController or
the shared handle-resolution helper) so that a provided pane_id is properly
resolved to a pane UUID; if the key is present but resolution fails, set result
= .err(code: "invalid_params", message: "Invalid pane_id", data: ["pane_id":
providedValue]) and return (do not fall back to split mode), otherwise proceed
using the resolved pane UUID where requestedPaneUUID, paneId, sourcePaneUUID and
markdownPanelId are used.
Author
There was a problem hiding this comment.
Same as above — v2UUID (line 3119-3125) tries UUID(uuidString:) first, then falls through to v2ResolveHandleRef for ref-style handles. This is how every other V2 command resolves pane/surface/workspace handles.
There was a problem hiding this comment.
@chi-feng Thanks — confirmed. v2UUID(...) does resolve both raw UUIDs and ref-style handles like "pane:5" via v2ResolveHandleRef, so the --pane pane:5 example is handled correctly.
One small edge case remains: if pane_id is present but unresolvable (blank string, unknown ref), the code currently falls back to split mode. For consistency with other v2 endpoints that treat “present but invalid” as invalid_params, we can add a tiny guard before the if-let:
diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift
@@ func v2MarkdownOpen(params: [String: Any]) -> V2CallResult {
- let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true)
- let requestedPaneUUID = v2UUID(params, "pane_id")
+ let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true)
+ let requestedPaneUUID = v2UUID(params, "pane_id")
+ let hasPaneParam = v2HasNonNullParam(params, "pane_id")
+ let rawPaneParam = v2RawString(params, "pane_id")
+
+ if hasPaneParam && requestedPaneUUID == nil {
+ result = .err(
+ code: "invalid_params",
+ message: "Invalid pane_id",
+ data: ["pane_id": v2OrNull(rawPaneParam)]
+ )
+ return
+ }
let markdownPanelId: UUID?
let sourceSurfaceId: UUID?
let sourcePaneUUID: UUID?
if let requestedPaneUUID {
// --pane mode: add as tab in existing paneEffect:
- "pane:5" and UUIDs keep working (as today).
- If pane_id is present but invalid, we return invalid_params instead of silently falling back to split mode.
If you’d like, I can open a tiny follow-up with this defensive check.
✏️ Learnings added
Learnt from: chi-feng
Repo: manaflow-ai/cmux PR: 2821
File: Sources/TerminalController.swift:7619-7640
Timestamp: 2026-04-11T18:55:28.499Z
Learning: Repo: manaflow-ai/cmux — TerminalController.v2UUID(params:key) resolves either a raw UUID string or a ref-style handle (e.g., "pane:5", "surface:3") via v2ResolveHandleRef. Endpoints accepting window_id/workspace_id/pane_id/surface_id should rely on v2UUID for ref support and, when the key is present but unresolvable, prefer returning invalid_params instead of falling back.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: 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: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:01:33.134Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, sendPickedElementToTerminal(workspaceId:summary:) resolves the target workspace via AppDelegate.shared?.workspaceFor(tabId: workspaceId), which searches across all mainWindowContexts (not self.tabManager which is active-window only). It first injects text into the focused terminal panel if it's a terminal, then falls back to iterating all panels in the workspace.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 1909
File: Sources/ContentView.swift:2156-2171
Timestamp: 2026-03-23T06:08:14.740Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, openFileInTextEditor(_:) must attempt workspace.newTextEditorSplit(from:orientation:filePath:focus:) and, if that returns nil, fall back to workspace.newTextEditorSurface(inPane:filePath:focus:) using bonsplitController.focusedPaneId. Rationale: avoid dropping file-open requests when the focused panel is not pane-backed.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T02:19:07.331Z
Learning: Applies to Sources/**/GhosttyTerminalView.swift : Mount `SurfaceSearchOverlay` from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers such as `Sources/Panels/TerminalPanelView.swift`, to maintain proper layering during split/workspace churn
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/GhosttyTerminalView.swift : `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers. Portal-hosted terminal views can sit above SwiftUI during split/workspace churn.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-04T01:01:43.511Z
Learning: Applies to {**/GhosttyTerminalView.swift,**/TerminalPanelView.swift} : `SurfaceSearchOverlay` must be mounted from `GhosttySurfaceScrollView` in `Sources/GhosttyTerminalView.swift` (AppKit portal layer), not from SwiftUI panel containers such as `Sources/Panels/TerminalPanelView.swift`
Learnt from: homanp
Repo: manaflow-ai/cmux PR: 883
File: CLI/cmux.swift:1569-1586
Timestamp: 2026-03-04T22:05:07.913Z
Learning: In manaflow-ai/cmux CLI (CLI/cmux.swift), for parity with existing browser commands, markdown.open intentionally normalizes --surface before resolving --workspace/--window, relying on server-side resolution to disambiguate final routing. Avoid one-off reordering in markdown; consider any change only as a coordinated, cross-command refactor.
Learnt from: austinywang
Repo: manaflow-ai/cmux PR: 2505
File: Sources/AppDelegate.swift:4910-4918
Timestamp: 2026-04-06T02:02:45.662Z
Learning: Repo: manaflow-ai/cmux — File: Sources/AppDelegate.swift — Keyboard repair pattern: In AppDelegate.repairFocusedTerminalKeyboardRoutingIfNeeded(window:event:), determine whether to repair focus by asking the focused terminal’s hosted view if the current first responder already matches that panel’s preferred keyboard target. Implemented via hostedView.responderMatchesPreferredKeyboardFocus(responder) inside responderNeedsFocusedTerminalKeyRepair(_:in:hostedView:). This covers same-window drift to a different Ghostty surface without comparing workspace/panel IDs. Verified by test cmuxTests/AppDelegateShortcutRoutingTests.swift::testWindowSendEventRepairsVisibleSameWindowResponderDriftForFocusedTerminalTyping.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T02:19:07.331Z
Learning: Applies to Sources/**/TerminalWindowPortal.swift : In `WindowTerminalHostView.hitTest()` in `TerminalWindowPortal.swift`, keep all divider/sidebar/drag routing gated to pointer events only. Do not add work outside the `isPointerEvent` guard to avoid typing latency
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: Sources/TerminalController.swift:3180-3193
Timestamp: 2026-03-09T02:08:14.574Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceClearTags(params:) must only clear all tags when the "source" key is absent. If "source" is present but blank or non-string (v2String(...) returns nil), the API should return invalid_params. Current implementation uses hasSourceKey = params.keys.contains("source") and guards with if hasSourceKey && source == nil { return .err(...)}.
Learnt from: MaTriXy
Repo: manaflow-ai/cmux PR: 1460
File: Sources/TerminalController.swift:4012-4023
Timestamp: 2026-03-16T08:05:16.048Z
Learning: In Sources/TerminalController.swift, implement v2SurfaceSplitSized(params:) to validate the ratio parameter as follows: if ratio is present, it must be numeric (NSNumber/Double-coercible) and strictly 0 < ratio < 1; if not, return invalid_params. If ratio is absent, use the default 0.6. This enforces the rule that a present-but-invalid parameter should yield invalid_params rather than silently falling back to defaults.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2398
File: CLI/cmux.swift:0-0
Timestamp: 2026-04-01T09:50:23.728Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift within CMUXCLI.buildInteractiveRemoteShellScript(...), never export CMUX_TAB_ID from the workspace UUID. CMUX_TAB_ID must be surface-scoped: only set it when a surface ID is available (map CMUX_TAB_ID to CMUX_SURFACE_ID). Rationale: tab-action/rename-tab resolve CMUX_TAB_ID before CMUX_SURFACE_ID; workspace-scoped values misroute or fail.
Learnt from: arieltobiana
Repo: manaflow-ai/cmux PR: 1873
File: Sources/TerminalController.swift:4071-4087
Timestamp: 2026-03-20T17:18:30.333Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceAction(params:) -> case "set_color": palette names are resolved via WorkspaceTabColorSettings.defaultPaletteWithOverrides(), whose entries are always valid hex (validated by the UI). Therefore, additional normalization of entry.hex is unnecessary.
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3607-3645
Timestamp: 2026-03-25T00:32:48.115Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift, v2WorkspaceSetColor(params:) must validate the provided color with WorkspaceTabColorSettings.normalizedHex. If the color parameter is present but invalid, return invalid_params (no fallback). On success, apply the color and return the normalized (uppercase) `#RRGGBB` in the response payload.
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: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1958-1960
Timestamp: 2026-03-25T00:33:00.675Z
Learning: Repo: manaflow-ai/cmux — CLI/cmux.swift set-workspace-color intentionally accepts both "#RRGGBB" and "RRGGBB" for ergonomics, matching server-side WorkspaceTabColorSettings.normalizedHex/TabManager normalization. Do not suggest enforcing a mandatory leading '#'; at most, suggest clarifying help/usage text.
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 2545
File: Sources/Workspace.swift:679-709
Timestamp: 2026-04-05T20:54:35.429Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift session restore, SessionTerminalPanelSnapshot now includes isRemoteBacked. createPanel(from:inPane:) uses this per-panel flag (panelWasRemoteBacked) to: (1) pass initialInput only for local panels, and (2) persist panelRestoreCommands only for local panels. Remote-backed terminals also omit restore/detected commands and have listeningPorts [] in the snapshot.
Learnt from: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/ContentView.swift:3194-3196
Timestamp: 2026-03-04T14:05:48.668Z
Learning: In manaflow-ai/cmux (PR `#819`), Sources/ContentView.swift: The command palette’s external window labels intentionally use the global window index from the full orderedSummaries (index + 1), matching the Window menu in AppDelegate. Do not reindex after filtering out the current window to avoid mismatches (“Window 2” for an external window is expected).
Learnt from: mrosnerr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-05T21:26:10.710Z
Learning: Repo: manaflow-ai/cmux — In Sources/Workspace.swift, applySessionPanelMetadata() must gate listeningPorts restoration on the per-panel snapshot.terminal?.isRemoteBacked flag (not workspace-wide remoteTerminalStartupCommand()), so that local panels are always eligible for port restore regardless of current SSH state. Fixed in commit 4d0fd871 (PR `#2545`).
Learnt from: qkrwpdlr
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-03-23T07:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/TerminalController.swift and Sources/Panels/BrowserPanel.swift, BiDi override (U+202A–202E, U+2066–2069) and zero-width char (U+200B–200F, U+FEFF) filtering is implemented via a shared `dangerousScalars: Set<UInt32>` in each class. v2SanitizeWebText() truncates to 200 chars; v2SanitizeXPath() caps at 2000 chars for selector fidelity. Both use a shared v2SanitizeScalar() predicate. BrowserPickerMessageHandler.sanitize() uses the same dangerousScalars pattern with a 200-char cap.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2475
File: Sources/ContentView.swift:0-0
Timestamp: 2026-04-03T07:19:36.497Z
Learning: Repo: manaflow-ai/cmux — In Sources/ContentView.swift, the command‑palette rename flow uses a single-line SwiftUI TextField via commandPaletteEditorField(style: .singleLine(...)); the multiline NSTextView editor is only used for the workspace description input. Do not flag newline persistence for rename.
Learnt from: lawrencecchen
Repo: manaflow-ai/cmux PR: 2525
File: Sources/GhosttyTerminalView.swift:481-513
Timestamp: 2026-04-02T10:13:39.235Z
Learning: Repo: manaflow-ai/cmux — In Sources/GhosttyTerminalView.swift, terminal file-link resolution trims trailing unmatched closing delimiters “) ] } >” only when they are dangling (more closers than openers), preserving wrapped tokens like “(file:///tmp/a.png)”. Implemented via terminalFileLinkTrailingClosingDelimiters and count comparison inside trimTrailingTerminalFileLinkPunctuation(_:) and exercised by a regression test (PR `#2525`, commit 3f5c5b6d).
Learnt from: pstanton237
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-06T12:03:16.921Z
Learning: Repo: manaflow-ai/cmux — In CLI/cmux.swift, `claudeTeamsLaunchArguments` must use two independent branches: (1) prepend `--teammate-mode auto` only when the caller has NOT already supplied `--teammate-mode`; (2) append `--settings <claudeHooksJSON>` unconditionally (modulo `CMUX_CLAUDE_HOOKS_DISABLED=1` opt-out). These two branches must never be coupled in a single early-return or conditional block — coupling them was the original `#2229` regression (hook JSON silently bypassed when `--teammate-mode auto|manual` was explicit). Mirrors `Resources/bin/claude:208`: `exec "$REAL_CLAUDE" --settings "$HOOKS_JSON" "$@"` where `$@` content never gates hook injection.
Learnt from: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-25T04:48:00.216Z
Learning: Applies to **/*.swift : Socket/CLI commands must not steal macOS app focus (no app activation/window raising side effects). Only explicit focus-intent commands may mutate in-app focus/selection (`window.focus`, `workspace.select/next/previous/last`, `surface.focus`, `pane.focus/last`, browser focus commands, and v1 focus equivalents).
Learnt from: apollow
Repo: manaflow-ai/cmux PR: 1089
File: CLI/cmux.swift:462-499
Timestamp: 2026-03-09T02:08:54.956Z
Learning: Repo: manaflow-ai/cmux
PR: `#1089`
File: CLI/cmux.swift
Component: ClaudeHookTagExtractor.extractTags(subtitle:body:)
Learning: For Claude Code session tag extraction, pre-redact sensitive spans (UUIDs, emails, access tokens, filesystem paths, ENV_VAR=..., long numerics) across the combined body+subtitle using unanchored sensitiveSpanPatterns before tokenization. Then tokenize and still filter each token with anchored sensitivePatterns. Rationale: prevents PII/path fragments from slipping into searchable tags after delimiter splitting.
Learnt from: tayl0r
Repo: manaflow-ai/cmux PR: 0
File: :0-0
Timestamp: 2026-04-08T03:36:30.160Z
Learning: Repo: manaflow-ai/cmux — AppDelegate.FileBrowserDrawerState threading pattern (PR `#1909`, commit e0e57809): FileBrowserDrawerState must be threaded through AppDelegate.configure() as a weak stored property (matching the sidebarState pattern), passed through both configure() call sites, with registerMainWindow parameter made non-optional. The fallback `?? FileBrowserDrawerState()` must NOT be used as it creates detached instances that are not properly owned by the window context.
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: 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: 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.
Contributor
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge; both findings are P2 and neither blocks the primary use-case. Core --pane logic correctly routes through the existing newMarkdownSurface(inPane:) method, nil sourceSurfaceId is handled throughout the response payload, and the --no-focus wiring follows the established pattern of break-pane/join-pane/surface.move. The only notable findings are a wsHandle scoping inconsistency for the edge case of --window without --workspace with a numeric pane index (P2), and a clarifying question about whether --no-focus is intentionally a no-op (P2). CLI/cmux.swift lines 2994–2999 (wsHandle scoping for pane index resolution) Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as cmux CLI
participant Socket as SocketClient
participant TC as TerminalController
participant WS as Workspace
CLI->>CLI: parseOption(--pane), filter(--no-focus)
CLI->>Socket: normalizePaneHandle(paneRaw)
Socket-->>CLI: pane UUID / ref string
CLI->>TC: markdown.open {path, pane_id, focus:false?}
alt --pane mode
TC->>TC: v2UUID(params, pane_id) → requestedPaneUUID
TC->>WS: allPaneIds.first(id == requestedPaneUUID)
WS-->>TC: paneId
TC->>WS: newMarkdownSurface(inPane: paneId, focus: v2FocusAllowed)
WS-->>TC: markdownPanel
else default split mode
TC->>TC: v2UUID(params, surface_id) ?? ws.focusedPanelId
TC->>WS: newMarkdownSplit(from: sourceSurfaceId, orientation, focus: v2FocusAllowed)
WS-->>TC: markdownPanel
end
TC-->>CLI: {surface_id, pane_id, source_surface_id, source_pane_id, ...}
Reviews (1): Last reviewed commit: "feat: add --pane and --no-focus flags to..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+2994
to
+2999
| if let paneRaw = paneOpt { | ||
| let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] | ||
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) { | ||
| params["pane_id"] = pane | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
wsHandle doesn't exclude CMUX_WORKSPACE_ID when --window is used
The workspace param (line 2983) correctly omits CMUX_WORKSPACE_ID when --window is specified (windowOpt == nil guard), but the pane index lookup unconditionally falls back to CMUX_WORKSPACE_ID. When --window is provided without --workspace, a numeric pane index gets resolved against the environment workspace rather than the targeted window's active workspace, causing a "Pane not found" server error for an otherwise valid index.
Suggested change
| if let paneRaw = paneOpt { | |
| let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] | |
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) { | |
| params["pane_id"] = pane | |
| } | |
| } | |
| if let paneRaw = paneOpt { | |
| let wsHandle = workspaceOpt ?? (windowOpt == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) | |
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) { | |
| params["pane_id"] = pane | |
| } | |
| } |
Author
There was a problem hiding this comment.
Good catch — fixed in 18c6b77. Now uses the already-computed workspaceRaw which respects the --window exclusion rule.
| result = .err(code: "not_found", message: "Source surface not found", data: ["surface_id": sourceSurfaceId.uuidString]) | ||
| return | ||
| } | ||
| let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true) |
Contributor
There was a problem hiding this comment.
--no-focus is a no-op: v2FocusAllowed always returns false for markdown.open
socketCommandAllowsInAppFocusMutations() reads socketCommandFocusAllowanceStack.last, which is pushed in withSocketCommandPolicy as focusIntentV2Methods.contains("markdown.open") = false. So v2FocusAllowed(requested: true) and v2FocusAllowed(requested: false) both evaluate to false for this command — identical to the pre-existing v2FocusAllowed() call.
This matches the socket focus policy (non-focus-intent commands don't mutate in-app focus), but it means --no-focus and the default both behave identically. If the intent is for markdown open to actually focus the new panel by default (so --no-focus is a meaningful opt-out), "markdown.open" would need to be added to focusIntentV2Methods. Worth confirming whether focus: false is the intended default here.
Author
There was a problem hiding this comment.
markdown.open is in the focusIntentV2Methods list (TerminalController.swift:2525), so socketCommandAllowsInAppFocusMutations() returns true for it. v2FocusAllowed(requested: false) correctly returns false. Tested manually — terminal retains focus when --no-focus is passed.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/TerminalController.swift">
<violation number="1" location="Sources/TerminalController.swift:7619">
P2: Malformed `pane_id` is silently treated as missing, causing unintended split fallback instead of `invalid_params`.</violation>
</file>
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:2995">
P2: `--pane` resolution ignores the `--window` scoping rule and always falls back to `CMUX_WORKSPACE_ID`, so `--window` + `--pane <index>` can resolve panes in the wrong workspace.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return | ||
| } | ||
| let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true) | ||
| let requestedPaneUUID = v2UUID(params, "pane_id") |
There was a problem hiding this comment.
P2: Malformed pane_id is silently treated as missing, causing unintended split fallback instead of invalid_params.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/TerminalController.swift, line 7619:
<comment>Malformed `pane_id` is silently treated as missing, causing unintended split fallback instead of `invalid_params`.</comment>
<file context>
@@ -7615,35 +7615,58 @@ class TerminalController {
- return
- }
+ let focus = v2FocusAllowed(requested: v2Bool(params, "focus") ?? true)
+ let requestedPaneUUID = v2UUID(params, "pane_id")
- let sourcePaneUUID = ws.paneId(forPanelId: sourceSurfaceId)?.id
</file context>
Author
There was a problem hiding this comment.
The silent fallback is intentional and matches how surface.create handles pane_id (TerminalController.swift:4982-4988). If v2UUID returns nil (malformed input), there is no pane_id param, so the command falls back to the default split behavior. This is consistent with the existing convention where unresolvable optional routing params fall back to defaults rather than erroring.
Author
Manual test resultsTested with Note: |
The workspace handle passed to normalizePaneHandle should follow the same --window exclusion rule as the workspace_id param: when --window is specified, don't fall back to CMUX_WORKSPACE_ID since the window implies a different scope. Use the already-computed workspaceRaw variable which handles this correctly.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
CLI/cmux.swift (1)
2983-2996:⚠️ Potential issue | 🟠 MajorUse normalized workspace handle when resolving pane indexes.
Line 2995 passes
workspaceRaw(can be an index like"2") intonormalizePaneHandle(...). That can mis-scope or fail pane index resolution becausenormalizePaneHandleinternally callspane.listwith that value asworkspace_id.🔧 Proposed fix
- let workspaceRaw = workspaceOpt ?? (windowOpt == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) - if let workspaceRaw { - if let workspace = try normalizeWorkspaceHandle(workspaceRaw, client: client) { - params["workspace_id"] = workspace - } - } + let workspaceRaw = workspaceOpt ?? (windowOpt == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil) + let workspaceHandle = try normalizeWorkspaceHandle(workspaceRaw, client: client) + if let workspaceHandle { + params["workspace_id"] = workspaceHandle + } @@ if let paneRaw = paneOpt { - if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspaceRaw) { + if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspaceHandle) { params["pane_id"] = pane } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 2983 - 2996, The pane normalization call is using workspaceRaw (which may be an index string) causing incorrect pane resolution; after resolving the workspace via normalizeWorkspaceHandle (the let workspace = try normalizeWorkspaceHandle(...) branch that sets params["workspace_id"]), pass that normalized workspace identifier into normalizePaneHandle instead of workspaceRaw. In other words, when handling paneOpt call normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspace) (or the params["workspace_id"] value) so normalizePaneHandle receives the canonical workspace handle returned by normalizeWorkspaceHandle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@CLI/cmux.swift`:
- Around line 2983-2996: The pane normalization call is using workspaceRaw
(which may be an index string) causing incorrect pane resolution; after
resolving the workspace via normalizeWorkspaceHandle (the let workspace = try
normalizeWorkspaceHandle(...) branch that sets params["workspace_id"]), pass
that normalized workspace identifier into normalizePaneHandle instead of
workspaceRaw. In other words, when handling paneOpt call
normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspace) (or the
params["workspace_id"] value) so normalizePaneHandle receives the canonical
workspace handle returned by normalizeWorkspaceHandle.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="CLI/cmux.swift">
<violation number="1" location="CLI/cmux.swift:2995">
P2: Pane index resolution may target the wrong pane when `--window` is used without `--workspace`, because workspace scoping is dropped before `normalizePaneHandle`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| } | ||
| } | ||
| if let paneRaw = paneOpt { | ||
| if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspaceRaw) { |
There was a problem hiding this comment.
P2: Pane index resolution may target the wrong pane when --window is used without --workspace, because workspace scoping is dropped before normalizePaneHandle.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 2995:
<comment>Pane index resolution may target the wrong pane when `--window` is used without `--workspace`, because workspace scoping is dropped before `normalizePaneHandle`.</comment>
<file context>
@@ -2992,8 +2992,7 @@ struct CMUXCLI {
if let paneRaw = paneOpt {
- let wsHandle = workspaceOpt ?? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"]
- if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: wsHandle) {
+ if let pane = try normalizePaneHandle(paneRaw, client: client, workspaceHandle: workspaceRaw) {
params["pane_id"] = pane
}
</file context>
Author
|
Re: Cubic follow-up on |
Author
|
@austinywang would you mind taking a look when you get a chance? Small feature PR — 2 files, all bot review comments resolved. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
+1, would love this feature. currently having to work around it |
Collaborator
|
Thank you for this, @chi-feng! |
teamleaderleo
added
area: cli
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
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--pane <id|ref|index>: Open a markdown file as a new tab in an existing pane instead of always splitting. Calls the existingWorkspace.newMarkdownSurface(inPane:)— the method was already implemented (used by session restore) but not exposed through the socket API or CLI.--no-focus: Open the markdown panel without stealing focus from the current surface. Wires through the existingv2FocusAllowed(requested:)pattern used bybreak-pane,join-pane, andsurface.move.cmux markdown open plan.md --pane pane:5 --no-focusCloses #1457
Addresses #2770, #2820
Motivation
The current
markdown openalways creates a split, which makes it hard to use programmatically (e.g., agent hooks that preview markdown alongside terminals). The common pattern is opening markdown as a tab in a pane that already holds other markdown/browser panels, without disrupting the user's focused terminal. The workaround today requiresmarkdown open→surface move→pane focus, which flickers and steals OS focus.Changes
CLI/cmux.swift(runMarkdownCommand):--paneviaparseOptionand--no-focusas a boolean flagpane_idandfocus: falsein socket paramsnormalizePaneHandle(supports UUID, ref, index)Sources/TerminalController.swift(v2MarkdownOpen):pane_idis present: resolve the pane and callws.newMarkdownSurface(inPane:)focusthroughv2FocusAllowed(requested: v2Bool(params, "focus") ?? true)— same pattern as other commandssource_surface_idbeing null in--panemode (no source surface to split from)Test plan
cmux markdown open plan.md— creates split (existing behavior, unchanged)cmux markdown open plan.md --direction down— creates split downward (unchanged)cmux markdown open plan.md --pane pane:0— adds tab to pane 0cmux markdown open plan.md --pane pane:0 --no-focus— adds tab without stealing focuscmux markdown open plan.md --no-focus— creates split without stealing focuscmux markdown --help— shows new options and examples./scripts/reload.sh --tag markdown-paneSummary by cubic
Add
--paneand--no-focustocmux markdown opento open markdown in an existing pane and avoid stealing focus, while keeping the default split behavior. Also fixes pane resolution to respect--windowscoping.New Features
--pane <id|ref|index>: open markdown as a new tab in an existing pane.--no-focus: open without focusing the new panel; works with split and--pane.Bug Fixes
--paneworkspace scoping now respects--window; no fallback toCMUX_WORKSPACE_IDwhen--windowis set.Written for commit 18c6b77. Summary will update on new commits.
Summary by CodeRabbit
--paneflag to target which pane receives opened markdown content.--no-focusflag to prevent focusing the new markdown panel.