Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions CLI/cmux.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1945,6 +1945,37 @@ struct CMUXCLI {
let payload = try client.sendV2(method: "workspace.rename", params: params)
printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["workspace"]))

case "set-workspace-color":
let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace")
let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil)
let trailing = Array(rem0.dropFirst(rem0.first == "--" ? 1 : 0))
guard trailing.count == 1 else {
throw CLIError(message: trailing.isEmpty
? "set-workspace-color requires a hex color (e.g. \"#C0392B\")"
: "set-workspace-color: unexpected arguments: \(trailing.dropFirst().joined(separator: " "))")
}
let color = trailing[0].trimmingCharacters(in: .whitespacesAndNewlines)
let body = color.hasPrefix("#") ? String(color.dropFirst()) : color
guard body.count == 6, UInt64(body, radix: 16) != nil else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB)")
Comment on lines +1959 to +1960

@cubic-dev-ai cubic-dev-ai Bot Mar 22, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: set-workspace-color regressed by rejecting previously accepted 8-digit hex colors (#RRGGBBAA).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 1959:

<comment>`set-workspace-color` regressed by rejecting previously accepted 8-digit hex colors (`#RRGGBBAA`).</comment>

<file context>
@@ -1956,8 +1956,8 @@ struct CMUXCLI {
             let body = color.hasPrefix("#") ? String(color.dropFirst()) : color
-            guard body.count == 6 || body.count == 8, UInt64(body.prefix(8), radix: 16) != nil else {
-                throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB or #RRGGBBAA)")
+            guard body.count == 6, UInt64(body, radix: 16) != nil else {
+                throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB)")
             }
</file context>
Suggested change
guard body.count == 6, UInt64(body, radix: 16) != nil else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB)")
guard (body.count == 6 || body.count == 8), UInt64(body, radix: 16) != nil else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB or #RRGGBBAA)")
}
Fix with Cubic

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not a regression — normalizedHex (TabManager.swift:367) only accepts 6-digit hex (body.count == 6). The previous CLI code accepting 8-digit would pass CLI validation but fail server-side. Aligning CLI to 6-digit only is correct.

Comment on lines +1958 to +1960

@coderabbitai coderabbitai Bot Mar 22, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Require explicit # prefix for color input.

set-workspace-color currently accepts bare RRGGBB (Line 1958), but the command contract is documented as #RRGGBB. Tighten validation so only # + 6 hex digits is accepted.

Suggested fix
-            let body = color.hasPrefix("#") ? String(color.dropFirst()) : color
-            guard body.count == 6, UInt64(body, radix: 16) != nil else {
+            guard color.hasPrefix("#") else {
+                throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected `#RRGGBB`)")
+            }
+            let body = String(color.dropFirst())
+            guard body.count == 6, UInt64(body, radix: 16) != nil else {
                 throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected `#RRGGBB`)")
             }
📝 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 body = color.hasPrefix("#") ? String(color.dropFirst()) : color
guard body.count == 6, UInt64(body, radix: 16) != nil else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected #RRGGBB)")
let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace")
let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil)
let colorArgs = rem0.dropFirst(rem0.first == "--" ? 1 : 0)
let color = colorArgs.joined(separator: " ").trimmingCharacters(in: .whitespacesAndNewlines)
guard color.hasPrefix("#") else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected `#RRGGBB`)")
}
let body = String(color.dropFirst())
guard body.count == 6, UInt64(body, radix: 16) != nil else {
throw CLIError(message: "set-workspace-color: invalid hex color \"\(color)\" (expected `#RRGGBB`)")
}
let wsId = try resolveWorkspaceId(workspaceArg, client: client)
let params: [String: Any] = ["workspace_id": wsId, "color": color]
let payload = try client.sendV2(method: "workspace.set_color", params: params)
printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["workspace"]))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 1958 - 1960, The current validation accepts bare
RRGGBB; change it to require an explicit leading '#' by first checking
color.hasPrefix("#") and throwing the existing CLIError if missing, then
validate that String(color.dropFirst()) has exactly 6 characters and parses as
hex (e.g., via UInt64(..., radix:16)); keep using the same CLIError message for
invalid format and reference the same "color" variable and the guard/throw code
path so callers of set-workspace-color only accept inputs matching "#RRGGBB".

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Intentional — CLI is lenient (accepts both #RRGGBB and bare RRGGBB) for ergonomics, matching normalizedHex which strips the # prefix internally.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@thunter009 Thanks for the clarification — acknowledged. I’ll withdraw my earlier suggestion. Accepting both “#RRGGBB” and “RRGGBB” in the CLI is consistent with the server-side normalization flow we’ve noted before (normalizedHex/TabManager normalization), so the current implementation looks good.

Optional follow-ups for clarity only:

  • Subcommand help/usage and PR summary could mention “#RRGGBB or RRGGBB” so users aren’t surprised that the bare form also works.
  • If you prefer, the invalid input message could say “expected 6 hex digits (RRGGBB, optional ‘#’)” — purely ergonomic.

Want me to open a small follow-up to tweak the help text/message, or leave as-is?


✏️ Learnings added
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: CLI/cmux.swift:1958-1960
Timestamp: 2026-03-25T00:32:58.190Z
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.

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: 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:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/BrowserPanel.swift, BrowserPanel.updateWorkspaceId(_:) is a dedicated method (var workspaceId) that updates both BrowserPanel.workspaceId and pickerMessageHandler?.updateWorkspaceId(_:) atomically. It is called from BrowserPanel.reattachToWorkspace(_:) when a panel moves between workspaces. This ensures BrowserPickerMessageHandler always posts notifications with the current workspaceId, not a stale one from panel initialization.

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 — TabManager.setTabColor(tabId:color:) normalizes hex values on write, ensuring the reported "color" and persisted value remain consistent; extra normalization in the caller is redundant.

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: 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: 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: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: outoftime
Repo: manaflow-ai/cmux PR: 1528
File: Resources/shell-integration/fish/vendor_conf.d/cmux-fish-integration.fish:541-546
Timestamp: 2026-03-17T13:59:10.665Z
Learning: Repo: manaflow-ai/cmux — The socket command `report_git_branch` (parsed in Sources/TerminalController.swift) expects the branch name as an **unquoted, bare token**. Wrapping the branch name in double quotes causes it to be silently discarded by the parser. This matches the bash/zsh shell integration convention. Do not suggest quoting the branch argument for this command; the fix was attempted in commit 829437c7 and immediately reverted because it broke branch reporting.

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: CR
Repo: manaflow-ai/cmux PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-17T08:16:21.950Z
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: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: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:16.241Z
Learning: For manaflow-ai/cmux PR `#819` (Japanese i18n), keep scope limited to localization changes; UX enhancements like preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels should be handled in a separate follow-up issue.

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

}
let wsId = try resolveWorkspaceId(workspaceArg, client: client)
let params: [String: Any] = ["workspace_id": wsId, "color": color]
let payload = try client.sendV2(method: "workspace.set_color", params: params)
printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["workspace"]))

case "clear-workspace-color":
let (wsArg, rem0) = parseOption(commandArgs, name: "--workspace")
let workspaceArg = wsArg ?? (windowId == nil ? ProcessInfo.processInfo.environment["CMUX_WORKSPACE_ID"] : nil)
let trailing = rem0.filter { $0 != "--" }
guard trailing.isEmpty else {
throw CLIError(message: "clear-workspace-color: unexpected arguments: \(trailing.joined(separator: " "))")
}
let wsId = try resolveWorkspaceId(workspaceArg, client: client)
let params: [String: Any] = ["workspace_id": wsId]
let payload = try client.sendV2(method: "workspace.clear_color", params: params)
printV2Payload(payload, jsonOutput: jsonOutput, idFormat: idFormat, fallbackText: v2OKSummary(payload, idFormat: idFormat, kinds: ["workspace"]))

Comment thread
coderabbitai[bot] marked this conversation as resolved.
case "current-workspace":
let response = try sendV1Command("current_workspace", client: client)
if jsonOutput {
Expand Down Expand Up @@ -6545,6 +6576,32 @@ struct CMUXCLI {
cmux rename-workspace "backend logs"
cmux rename-window --workspace workspace:2 "agent run"
"""
case "set-workspace-color":
return """
Usage: cmux set-workspace-color [--workspace <id|ref|index>] <hex>

Set the tab color for a workspace. Defaults to the current workspace.

Flags:
--workspace <id|ref|index> Workspace to color (default: current/$CMUX_WORKSPACE_ID)

Example:
cmux set-workspace-color "#C0392B"
cmux set-workspace-color --workspace workspace:2 "#2D1B69"
"""
case "clear-workspace-color":
return """
Usage: cmux clear-workspace-color [--workspace <id|ref|index>]

Remove the custom tab color from a workspace.

Flags:
--workspace <id|ref|index> Workspace to clear color from (default: current/$CMUX_WORKSPACE_ID)

Example:
cmux clear-workspace-color
cmux clear-workspace-color --workspace workspace:2
"""
case "current-workspace":
return """
Usage: cmux current-workspace
Expand Down Expand Up @@ -11159,6 +11216,8 @@ struct CMUXCLI {
select-workspace --workspace <id|ref>
rename-workspace [--workspace <id|ref>] <title>
rename-window [--workspace <id|ref>] <title>
set-workspace-color [--workspace <id|ref|index>] <hex>
clear-workspace-color [--workspace <id|ref|index>]
current-workspace
read-screen [--workspace <id|ref>] [--surface <id|ref>] [--scrollback] [--lines <n>]
send [--workspace <id|ref>] [--surface <id|ref>] <text>
Expand Down
77 changes: 77 additions & 0 deletions Sources/TerminalController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2065,6 +2065,10 @@ class TerminalController {
return v2Result(id: id, self.v2WorkspaceReorder(params: params))
case "workspace.rename":
return v2Result(id: id, self.v2WorkspaceRename(params: params))
case "workspace.set_color":
return v2Result(id: id, self.v2WorkspaceSetColor(params: params))
case "workspace.clear_color":
return v2Result(id: id, self.v2WorkspaceClearColor(params: params))
case "workspace.action":
return v2Result(id: id, self.v2WorkspaceAction(params: params))
case "workspace.next":
Expand Down Expand Up @@ -2441,6 +2445,8 @@ class TerminalController {
"workspace.move_to_window",
"workspace.reorder",
"workspace.rename",
"workspace.set_color",
"workspace.clear_color",
"workspace.action",
"workspace.next",
"workspace.previous",
Expand Down Expand Up @@ -3598,6 +3604,77 @@ class TerminalController {
"title": title
])
}
private func v2WorkspaceSetColor(params: [String: Any]) -> V2CallResult {
guard let tabManager = v2ResolveTabManager(params: params) else {
return .err(code: "unavailable", message: "TabManager not available", data: nil)
}
guard let workspaceId = v2UUID(params, "workspace_id") else {
return .err(code: "invalid_params", message: "Missing or invalid workspace_id", data: nil)
}
guard let colorRaw = v2String(params, "color"),
!colorRaw.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
return .err(code: "invalid_params", message: "Missing or invalid color", data: nil)
}

let trimmed = colorRaw.trimmingCharacters(in: .whitespacesAndNewlines)
guard let color = WorkspaceTabColorSettings.normalizedHex(trimmed) else {
return .err(code: "invalid_params", message: "Invalid hex color \"\(trimmed)\" (expected #RRGGBB)", data: nil)
}
Comment on lines +3620 to +3622

@coderabbitai coderabbitai Bot Mar 19, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Update the invalid-color error message to include alpha hex format.

The message currently says only #RRGGBB, but this command contract allows #RRGGBBAA as well; clients get incorrect remediation guidance.

💡 Proposed fix
-        guard let color = WorkspaceTabColorSettings.normalizedHex(trimmed) else {
-            return .err(code: "invalid_params", message: "Invalid hex color \"\(trimmed)\" (expected `#RRGGBB`)", data: nil)
+        guard let color = WorkspaceTabColorSettings.normalizedHex(trimmed) else {
+            return .err(
+                code: "invalid_params",
+                message: "Invalid hex color \"\(trimmed)\" (expected `#RRGGBB` or `#RRGGBBAA`)",
+                data: nil
+            )
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalController.swift` around lines 3597 - 3599, The error message
returned when color parsing fails should mention that both 6- and 8-digit hex
are accepted; update the message in the guard that calls
WorkspaceTabColorSettings.normalizedHex (the block that returns .err(code:
"invalid_params", message: ... , data: nil)) to include acceptance of either
"#RRGGBB" or "#RRGGBBAA" (e.g. change "expected `#RRGGBB`" to "expected `#RRGGBB` or
`#RRGGBBAA`"), keeping the rest of the returned payload identical and still
referencing the trimmed input.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not applicable — normalizedHex only accepts 6-digit hex (body.count == 6 in TabManager.swift:367). The error message correctly says #RRGGBB only.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@thunter009 Agreed—no change needed. WorkspaceTabColorSettings.normalizedHex(...) only accepts 6‑digit hex (#RRGGBB), and the CLI validation is aligned to that, so the current error text is correct. I’ll avoid suggesting #RRGGBBAA for this path going forward.


✏️ Learnings added
Learnt from: thunter009
Repo: manaflow-ai/cmux PR: 1825
File: Sources/TerminalController.swift:3620-3622
Timestamp: 2026-03-25T00:32:54.735Z
Learning: Repo: manaflow-ai/cmux — Color validation: WorkspaceTabColorSettings.normalizedHex(...) and TabManager.setTabColor(tabId:color:) accept only 6‑digit `#RRGGBB` (no alpha). CLI/cmux.swift also enforces `#RRGGBB`. Error/help text for workspace color should reference only `#RRGGBB`, not `#RRGGBBAA`.

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: 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: 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 — TabManager.setTabColor(tabId:color:) normalizes hex values on write, ensuring the reported "color" and persisted value remain consistent; extra normalization in the caller is redundant.

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: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:12:42.553Z
Learning: Repo: manaflow-ai/cmux — In Sources/Panels/BrowserPanel.swift, BrowserPanel.updateWorkspaceId(_:) is a dedicated method (var workspaceId) that updates both BrowserPanel.workspaceId and pickerMessageHandler?.updateWorkspaceId(_:) atomically. It is called from BrowserPanel.reattachToWorkspace(_:) when a panel moves between workspaces. This ensures BrowserPickerMessageHandler always posts notifications with the current workspaceId, not a stale one from panel initialization.

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: atani
Repo: manaflow-ai/cmux PR: 819
File: Sources/AppDelegate.swift:3491-3493
Timestamp: 2026-03-04T14:06:16.241Z
Learning: For manaflow-ai/cmux PR `#819` (Japanese i18n), keep scope limited to localization changes; UX enhancements like preferring workspace.customTitle in workspaceDisplayName() or altering move-target labels should be handled in a separate follow-up issue.

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: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: 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`.

var applied = false
v2MainSync {
guard tabManager.tabs.contains(where: { $0.id == workspaceId }) else { return }
tabManager.setTabColor(tabId: workspaceId, color: color)
applied = true
}

guard applied else {
return .err(code: "not_found", message: "Workspace not found", data: [
"workspace_id": workspaceId.uuidString,
"workspace_ref": v2Ref(kind: .workspace, uuid: workspaceId)
])
}

let windowId = v2ResolveWindowId(tabManager: tabManager)
return .ok([
"workspace_id": workspaceId.uuidString,
"workspace_ref": v2Ref(kind: .workspace, uuid: workspaceId),
"window_id": v2OrNull(windowId?.uuidString),
"window_ref": v2Ref(kind: .window, uuid: windowId),
"color": color
])
Comment on lines +3614 to +3644

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Invalid hex silently clears color and returns false success

v2WorkspaceSetColor only checks that the color string is non-empty, but setCustomColor internally calls WorkspaceTabColorSettings.normalizedHex, which returns nil for any malformed input (wrong length, non-hex characters, etc.). When normalizedHex returns nil, customColor is set to nil — effectively the same as clearing the color — yet the response returns "color": color with the original invalid string, giving the caller a false success signal.

Additionally, even for a valid hex input, the response echoes back the raw (non-normalized) string (e.g. "#c0392b") while the persisted value is the uppercased form "#C0392B" produced by normalizedHex.

The fix is to validate and normalize via normalizedHex before calling setTabColor, returning an error on invalid input and the normalized value on success:

guard let colorRaw = v2String(params, "color"),
      !colorRaw.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty else {
    return .err(code: "invalid_params", message: "Missing or invalid color", data: nil)
}

let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw)
guard let color = normalized else {
    return .err(code: "invalid_params",
                message: "Invalid hex color '\(colorRaw)'. Expected a 6-digit hex string e.g. \"#C0392B\"",
                data: nil)
}

// ... (use `color` — already normalized — when calling setTabColor and in the response)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 563d9d8 (hex validation + normalizedHex guard) and 2bdb009 (return normalized hex in response). Invalid hex now returns an error, and the response echoes the normalized value.

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

private func v2WorkspaceClearColor(params: [String: Any]) -> V2CallResult {
guard let tabManager = v2ResolveTabManager(params: params) else {
return .err(code: "unavailable", message: "TabManager not available", data: nil)
}
guard let workspaceId = v2UUID(params, "workspace_id") else {
return .err(code: "invalid_params", message: "Missing or invalid workspace_id", data: nil)
}

var cleared = false
v2MainSync {
guard tabManager.tabs.contains(where: { $0.id == workspaceId }) else { return }
tabManager.setTabColor(tabId: workspaceId, color: nil)
cleared = true
}

guard cleared else {
return .err(code: "not_found", message: "Workspace not found", data: [
"workspace_id": workspaceId.uuidString,
"workspace_ref": v2Ref(kind: .workspace, uuid: workspaceId)
])
}

let windowId = v2ResolveWindowId(tabManager: tabManager)
return .ok([
"workspace_id": workspaceId.uuidString,
"workspace_ref": v2Ref(kind: .workspace, uuid: workspaceId),
"window_id": v2OrNull(windowId?.uuidString),
"window_ref": v2Ref(kind: .window, uuid: windowId),
])
}

private func v2WorkspaceNext(params: [String: Any]) -> V2CallResult {
guard let tabManager = v2ResolveTabManager(params: params) else {
return .err(code: "unavailable", message: "TabManager not available", data: nil)
Expand Down