feat: add set-color/clear-color to workspace-action CLI - #1833
Conversation
- CLI: parse --color option in runWorkspaceAction - CLI: validate --color required for set-color action - CLI: pass color param to v2 payload - CLI: update help text and usage summary - TerminalController: add set_color/clear_color to supportedActions - TerminalController: handle set_color/clear_color cases via setTabColor
|
@bagelcode-jhkim is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughExtended CLI argument parsing for workspace actions with Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as CLI/cmux
participant TC as TerminalController
participant Settings as WorkspaceTabColorSettings
participant TM as tabManager
User->>CLI: workspace-action set-color --color "FF5733"
CLI->>CLI: Parse --color option
CLI->>CLI: Validate color parameter present
CLI->>TC: sendV2(method: "workspace.action",<br/>params: {action, color})
TC->>TC: Validate color not empty
alt Color is hex
TC->>Settings: normalizedHex(color)
Settings-->>TC: Validated hex "#RRGGBB"
else Color is name
TC->>Settings: defaultPalette lookup
Settings-->>TC: Hex from palette
else Invalid
TC-->>User: invalid_params error
end
TC->>TM: setTabColor(tabId: color: "#RRGGBB")
TM-->>TC: ✓ Updated
TC-->>User: {success, color: "#RRGGBB"}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can disable the changed files summary in the walkthrough.Disable the |
Greptile SummaryThis PR exposes two new workspace tab color operations — Key changes:
One logic bug found: the named-color lookup in Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User as User (CLI)
participant CLI as cmux.swift
participant Socket as SocketClient
participant TC as TerminalController
participant WCS as WorkspaceTabColorSettings
participant TM as TabManager
User->>CLI: workspace-action --action set-color --color Red
CLI->>CLI: parse --color → colorOpt
CLI->>CLI: guard colorOpt not empty
CLI->>Socket: sendV2("workspace.action", {action:"set_color", color:"Red"})
Socket->>TC: v2WorkspaceAction(params)
TC->>TC: resolve workspace by ID
TC->>WCS: defaultPalette.first(where name == "Red")
Note over WCS: ⚠️ Should use palette() to<br/>respect user overrides
WCS-->>TC: entry.hex = "#C0392B"
TC->>TM: setTabColor(tabId:, color:"#C0392B")
TM-->>TC: (tab color updated)
TC-->>Socket: .ok({action, workspace_id, color:"#C0392B"})
Socket-->>CLI: payload
CLI->>User: OK action=set_color workspace=...
User->>CLI: workspace-action --action clear-color
CLI->>Socket: sendV2("workspace.action", {action:"clear_color"})
Socket->>TC: v2WorkspaceAction(params)
TC->>TM: setTabColor(tabId:, color:nil)
TC-->>Socket: .ok({action, workspace_id, color:null})
Socket-->>CLI: payload
CLI->>User: OK action=clear_color workspace=...
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c9f5903836
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | ||
| $0.name.lowercased() == colorRaw.lowercased() |
There was a problem hiding this comment.
Resolve named colors using configured palette
Named color resolution currently searches WorkspaceTabColorSettings.defaultPalette, which always uses the hardcoded base hex values and ignores user overrides from Settings. In environments where a user has remapped a default color (for example, changed Red to another hex), workspace-action --action set-color --color Red will apply the wrong color compared with the UI palette. This should resolve through the override-aware API (for example defaultColorHex(named:) or defaultPaletteWithOverrides) so CLI behavior matches configured workspace colors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | ||
| $0.name.lowercased() == colorRaw.lowercased() | ||
| }) { | ||
| resolved = entry.hex | ||
| } else { | ||
| let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ") | ||
| result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use #RRGGBB or: \(names)", data: nil) |
There was a problem hiding this comment.
Named color lookup ignores user overrides and custom colors
WorkspaceTabColorSettings.defaultPalette returns the raw, hardcoded originalPRPalette — it does not apply user-configured default overrides (stored under workspaceTabColor.defaultOverrides) nor does it include user-added custom colors (stored under workspaceTabColor.customColors).
This creates two problems:
- If a user has overridden, say, "Red" to a custom hex via the UI,
set-color --color Redwill silently apply the original#C0392Binstead of their override. - Any custom named colors the user has added cannot be referenced by name from the CLI.
WorkspaceTabColorSettings.palette() (which returns defaultPaletteWithOverrides + customColorEntries) is the correct source of truth here, and is what the UI color picker uses. The error message listing valid names at line 4081 also needs to use the same source so it accurately reflects the user's actual palette.
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | |
| $0.name.lowercased() == colorRaw.lowercased() | |
| }) { | |
| resolved = entry.hex | |
| } else { | |
| let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ") | |
| result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use #RRGGBB or: \(names)", data: nil) | |
| } else if let entry = WorkspaceTabColorSettings.palette().first(where: { | |
| $0.name.lowercased() == colorRaw.lowercased() | |
| }) { | |
| resolved = entry.hex | |
| } else { | |
| let names = WorkspaceTabColorSettings.palette().map(\.name).joined(separator: ", ") |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CLI/cmux.swift (1)
3307-3309: Normalize--colorbefore validation and payload construction.Line 3307 and Line 3318 currently treat whitespace-only input as valid. Trimming once avoids passing effectively-empty values downstream.
Proposed diff
- if action == "set_color", (colorOpt?.isEmpty ?? true) { + let normalizedColor = colorOpt?.trimmingCharacters(in: .whitespacesAndNewlines) + if action == "set_color", (normalizedColor?.isEmpty ?? true) { throw CLIError(message: "workspace-action set-color requires --color <#hex|name>") } @@ - if let color = colorOpt, !color.isEmpty { + if let color = normalizedColor, !color.isEmpty { params["color"] = color }Also applies to: 3318-3320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3307 - 3309, For the "set_color" branch, normalize the incoming color option by trimming whitespace before validating and before using it in the payload: replace uses of colorOpt with a trimmedColor (e.g., let trimmedColor = colorOpt?.trimmingCharacters(in: .whitespacesAndNewlines)) and use trimmedColor for the isEmpty check that throws the CLIError and later when constructing the payload; ensure you still preserve optional semantics (nil vs empty) so downstream code sees a proper nil or non-empty hex/name string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/TerminalController.swift`:
- Around line 4063-4084: The handler for "set_color" currently only treats
strings starting with "#" as hex; update the branching so that after checking
colorRaw.hasPrefix("#") and normalizing that, you also attempt to normalize bare
6-digit hex strings by calling WorkspaceTabColorSettings.normalizedHex(colorRaw)
before falling back to WorkspaceTabColorSettings.defaultPalette lookup; if
normalization succeeds assign to resolved, otherwise continue to palette lookup
and finally return the same invalid_params error message using resolved/colorRaw
as appropriate.
---
Nitpick comments:
In `@CLI/cmux.swift`:
- Around line 3307-3309: For the "set_color" branch, normalize the incoming
color option by trimming whitespace before validating and before using it in the
payload: replace uses of colorOpt with a trimmedColor (e.g., let trimmedColor =
colorOpt?.trimmingCharacters(in: .whitespacesAndNewlines)) and use trimmedColor
for the isEmpty check that throws the CLIError and later when constructing the
payload; ensure you still preserve optional semantics (nil vs empty) so
downstream code sees a proper nil or non-empty hex/name string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d8f5396-87c0-483e-90ad-6ebeef65d860
📒 Files selected for processing (2)
CLI/cmux.swiftSources/TerminalController.swift
| case "set_color": | ||
| guard let colorRaw = v2String(params, "color"), !colorRaw.isEmpty else { | ||
| result = .err(code: "invalid_params", message: "set-color requires --color", data: nil) | ||
| return | ||
| } | ||
| // Resolve named color to hex via palette lookup | ||
| let resolved: String | ||
| if colorRaw.hasPrefix("#") { | ||
| guard let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw) else { | ||
| result = .err(code: "invalid_params", message: "Invalid hex color '\(colorRaw)'. Expected #RRGGBB", data: nil) | ||
| return | ||
| } | ||
| resolved = normalized | ||
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | ||
| $0.name.lowercased() == colorRaw.lowercased() | ||
| }) { | ||
| resolved = entry.hex | ||
| } else { | ||
| let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ") | ||
| result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use #RRGGBB or: \(names)", data: nil) | ||
| return | ||
| } |
There was a problem hiding this comment.
Accept bare 6-digit hex values before falling back to palette names.
Line 4070 only treats #... input as hex, so C0392B is rejected here even though WorkspaceTabColorSettings.normalizedHex(...) already accepts that format.
♻️ Proposed fix
case "set_color":
- guard let colorRaw = v2String(params, "color"), !colorRaw.isEmpty else {
- result = .err(code: "invalid_params", message: "set-color requires --color", data: nil)
+ guard let colorRaw = v2String(params, "color") else {
+ result = .err(code: "invalid_params", message: "Missing or invalid color", data: nil)
return
}
- // Resolve named color to hex via palette lookup
let resolved: String
- if colorRaw.hasPrefix("#") {
- guard let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw) else {
- result = .err(code: "invalid_params", message: "Invalid hex color '\(colorRaw)'. Expected `#RRGGBB`", data: nil)
- return
- }
+ if let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw) {
resolved = normalized
} else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: {
$0.name.lowercased() == colorRaw.lowercased()
}) {
resolved = entry.hex
} else {
let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ")
- result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use `#RRGGBB` or: \(names)", data: nil)
+ result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use a 6-digit hex color or: \(names)", data: nil)
return
}📝 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.
| case "set_color": | |
| guard let colorRaw = v2String(params, "color"), !colorRaw.isEmpty else { | |
| result = .err(code: "invalid_params", message: "set-color requires --color", data: nil) | |
| return | |
| } | |
| // Resolve named color to hex via palette lookup | |
| let resolved: String | |
| if colorRaw.hasPrefix("#") { | |
| guard let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw) else { | |
| result = .err(code: "invalid_params", message: "Invalid hex color '\(colorRaw)'. Expected #RRGGBB", data: nil) | |
| return | |
| } | |
| resolved = normalized | |
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | |
| $0.name.lowercased() == colorRaw.lowercased() | |
| }) { | |
| resolved = entry.hex | |
| } else { | |
| let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ") | |
| result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use #RRGGBB or: \(names)", data: nil) | |
| return | |
| } | |
| case "set_color": | |
| guard let colorRaw = v2String(params, "color") else { | |
| result = .err(code: "invalid_params", message: "Missing or invalid color", data: nil) | |
| return | |
| } | |
| let resolved: String | |
| if let normalized = WorkspaceTabColorSettings.normalizedHex(colorRaw) { | |
| resolved = normalized | |
| } else if let entry = WorkspaceTabColorSettings.defaultPalette.first(where: { | |
| $0.name.lowercased() == colorRaw.lowercased() | |
| }) { | |
| resolved = entry.hex | |
| } else { | |
| let names = WorkspaceTabColorSettings.defaultPalette.map(\.name).joined(separator: ", ") | |
| result = .err(code: "invalid_params", message: "Unknown color '\(colorRaw)'. Use a 6-digit hex color or: \(names)", data: nil) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/TerminalController.swift` around lines 4063 - 4084, The handler for
"set_color" currently only treats strings starting with "#" as hex; update the
branching so that after checking colorRaw.hasPrefix("#") and normalizing that,
you also attempt to normalize bare 6-digit hex strings by calling
WorkspaceTabColorSettings.normalizedHex(colorRaw) before falling back to
WorkspaceTabColorSettings.defaultPalette lookup; if normalization succeeds
assign to resolved, otherwise continue to palette lookup and finally return the
same invalid_params error message using resolved/colorRaw as appropriate.
|
Thanks for the contribution! |
…1833) * [auto] cmux session changes (1 files) [03/19 17:38] * feat: add set-color/clear-color to workspace-action - CLI: parse --color option in runWorkspaceAction - CLI: validate --color required for set-color action - CLI: pass color param to v2 payload - CLI: update help text and usage summary - TerminalController: add set_color/clear_color to supportedActions - TerminalController: handle set_color/clear_color cases via setTabColor * fix: resolve named colors via palette, improve clear_color response * fix: validate hex input via normalizedHex, fix synopsis typo
…1833) * [auto] cmux session changes (1 files) [03/19 17:38] * feat: add set-color/clear-color to workspace-action - CLI: parse --color option in runWorkspaceAction - CLI: validate --color required for set-color action - CLI: pass color param to v2 payload - CLI: update help text and usage summary - TerminalController: add set_color/clear_color to supportedActions - TerminalController: handle set_color/clear_color cases via setTabColor * fix: resolve named colors via palette, improve clear_color response * fix: validate hex input via normalizedHex, fix synopsis typo
Summary
Add CLI support for setting workspace tab colors via
workspace-action.Usage
Changes
--coloroption parsing, validation forset_coloraction, help text and synopsis updatesset_color/clear_colortosupportedActions, implement switch cases with:WorkspaceTabColorSettings.defaultPalettenormalizedHex()Motivation
Currently workspace colors can only be set via the UI context menu. This enables scripting use cases like:
No model changes needed —
tabManager.setTabColor(tabId:color:)andWorkspaceTabColorSettingsalready exist.Summary by cubic
Add
set-colorandclear-colortocmux workspace-actionso you can script workspace tab colors. Supports named colors and#RRGGBBwith validation and clear errors.--colorin the CLI, require it forset-color, and pass it in the v2 payload.WorkspaceTabColorSettings.defaultPaletteand validate hex vianormalizedHex().set_color/clear_colorinTerminalController; update help text and synopsis.Written for commit c9f5903. Summary will update on new commits.
Summary by CodeRabbit
New Features
--coloroption