Fix cmux.json named workspace colors - #3149
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds named-color support for workspace color fields by resolving palette names from UserDefaults during config decode; introduces a dedicated workspace definition type and palette-fingerprint-based cache invalidation so config parses refresh when the palette changes. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ConfigLoader as Config Loader
participant Decoder as CmuxWorkspaceDefinition
participant Resolver as WorkspaceTabColorSettings
participant Palette as UserDefaults (Palette)
participant Cache as ParsedConfigCache
User->>ConfigLoader: request load cmux.json
ConfigLoader->>Decoder: decode workspace entries
Decoder->>Resolver: resolve raw color string
Resolver->>Palette: lookup named color
Palette-->>Resolver: return hex or nil
alt Resolved
Resolver-->>Decoder: return resolved hex
else Unrecognized
Resolver-->>Decoder: throw DecodingError
end
Decoder-->>ConfigLoader: decoded workspace definitions
ConfigLoader->>Resolver: compute paletteCacheFingerprint
Resolver->>Palette: read palette entries
Palette-->>Resolver: palette data
Resolver-->>Cache: provide fingerprint
Cache->>Cache: compare with stored fingerprint
alt fingerprint differs
Cache-->>ConfigLoader: invalidate and store new parsed result + fingerprint
else match
Cache-->>ConfigLoader: reuse cached parse result
end
ConfigLoader-->>User: return parsed config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 3/8 reviews remaining, refill in 30 minutes and 8 seconds.Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c55836ee4
ℹ️ 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".
Contributor
Greptile SummaryThis PR fixes #3075 by allowing Confidence Score: 4/5Safe to merge; only P2 findings around test isolation and the UserDefaults coupling that is intentional by design. No P0 or P1 issues found. The logic in resolvedColorHex is correct, the error message update is accurate, and the happy-path test passes. Two P2 items: the test assertion is implicitly tied to UserDefaults.standard (risk of cross-test pollution), and the decoder-side call has no way to inject a custom UserDefaults instance (known architectural constraint). No files require special attention; minor test isolation concern in cmuxTests/CmuxConfigTests.swift. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["cmux.json decode: color field"] --> B["container.decodeIfPresent(String)"]
B -->|"rawColor present"| C["resolvedColorHex(rawColor)"]
B -->|"absent"| D["color = nil"]
C --> E["normalizedHex(raw)"]
E -->|"valid 6-digit hex"| F["return '#RRGGBB'"]
E -->|"nil (not hex)"| G["trim whitespace"]
G -->|"empty"| H["return nil → DecodingError"]
G -->|"non-empty"| I["effectivePaletteMap(defaults: .standard)"]
I --> J["case-insensitive name lookup"]
J -->|"found"| K["return hex value"]
J -->|"not found"| H
F --> L["color = normalized hex"]
K --> L
Reviews (1): Last reviewed commit: "Accept named colors in cmux.json" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
Sources/CmuxConfig.swift (1)
1608-1619:⚠️ Potential issue | 🔴 CriticalPalette-override changes bypass the config cache—named colors in cmux.json won't update until the file changes.
WorkspaceTabColorSettings.resolvedColorHex(_:)readsUserDefaults.standardviaeffectivePaletteMap, so named color hex values are resolved at decode time according to the current palette overrides. However,parsedConfigCacheis keyed only by file size + modification date. When a user edits a palette override (e.g., remaps "Indigo" to a new hex), the cached decodedCmuxConfigFileretains the previously resolved hex. The palette-change handlers (setColorat cmuxApp.swift:6777 andaddCustomColorat ContentView.swift:13326) only update the UI display and do not callloadAll()or invalidate the cache. As a result, workspaces with named colors drift out of sync with the current palette overrides until the cmux.json file itself is modified.To fix: clear
parsedConfigCachewhen palette overrides change, either by callingloadAll()from the palette-change handlers or by introducing a UserDefaults observer that invalidates the cache on palette mutations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/CmuxConfig.swift` around lines 1608 - 1619, The parsed CmuxConfigFile cache (parsedConfigCache) is populated with resolved hexes by WorkspaceTabColorSettings.resolvedColorHex during decoding, so palette overrides can leave cached CmuxConfigFile entries stale; update the palette-change handlers (setColor and addCustomColor) to invalidate parsedConfigCache or call loadAll() after applying overrides, or add a UserDefaults observer tied to the effectivePaletteMap/palette override keys to clear parsedConfigCache when palette values change so named colors are re-resolved on next load.
🧹 Nitpick comments (1)
cmuxTests/CmuxConfigTests.swift (1)
1047-1061: Optional: derive the expected hex from the resolver instead of hardcoding#283593.Asserting against the literal
"#283593"ties this regression test to the current default palette mapping for "Indigo" and to the assumption that the test runner'sUserDefaults.standardhas no palette override for that name. Computing the expectation viaWorkspaceTabColorSettings.resolvedColorHex("Indigo")would keep the test focused on the decoder's behavior (it uses the resolver) rather than on a specific palette value, and would survive future palette retunes without false failures. Still a valid regression test as-is — just a resilience nit.♻️ Proposed tweak
- let config = try decode(json) - XCTAssertEqual(config.commands[0].workspace?.color, "#283593") + let config = try decode(json) + let expected = try XCTUnwrap(WorkspaceTabColorSettings.resolvedColorHex("Indigo")) + XCTAssertEqual(config.commands[0].workspace?.color, expected)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/CmuxConfigTests.swift` around lines 1047 - 1061, Update the assertion in testDecodeWorkspaceCommandAcceptsNamedColor to derive the expected hex dynamically from the resolver instead of hardcoding "#283593": call WorkspaceTabColorSettings.resolvedColorHex("Indigo") and compare that result to config.commands[0].workspace?.color (the value returned by decode(json)), so the test verifies that the decoder uses the resolver without depending on a specific palette value.
🤖 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 `@Sources/CmuxConfig.swift`:
- Around line 1608-1619: The parsed CmuxConfigFile cache (parsedConfigCache) is
populated with resolved hexes by WorkspaceTabColorSettings.resolvedColorHex
during decoding, so palette overrides can leave cached CmuxConfigFile entries
stale; update the palette-change handlers (setColor and addCustomColor) to
invalidate parsedConfigCache or call loadAll() after applying overrides, or add
a UserDefaults observer tied to the effectivePaletteMap/palette override keys to
clear parsedConfigCache when palette values change so named colors are
re-resolved on next load.
---
Nitpick comments:
In `@cmuxTests/CmuxConfigTests.swift`:
- Around line 1047-1061: Update the assertion in
testDecodeWorkspaceCommandAcceptsNamedColor to derive the expected hex
dynamically from the resolver instead of hardcoding "#283593": call
WorkspaceTabColorSettings.resolvedColorHex("Indigo") and compare that result to
config.commands[0].workspace?.color (the value returned by decode(json)), so the
test verifies that the decoder uses the resolver without depending on a specific
palette value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40b33870-8946-4a77-8ad0-58f9da21b3f1
📒 Files selected for processing (3)
Sources/CmuxConfig.swiftSources/TabManager.swiftcmuxTests/CmuxConfigTests.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 3 files
You’re at about 95% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
BenevolentFutures
added a commit
to Stage-11-Agentics/c11
that referenced
this pull request
Apr 27, 2026
Config and CLI paths could supply a color as a human-readable name like "Red" or "Blue" rather than a hex string. The executor passed the raw string directly to setCustomColor, which normalizes only valid 6-digit hex values and silently no-ops everything else, causing the workspace tab to render with no color. Added resolveColorToHex, a nonisolated static helper that first tries normalizedHex for valid hex input and then does a case-insensitive lookup in WorkspaceTabColorSettings.defaultPaletteWithOverrides. Strings that are neither valid hex nor known palette names produce an ApplyFailure with code "unknown_color_name" rather than a silent no-op. Reported-by: @zacharygutt <upstream issue manaflow-ai/cmux#3075> Reference: manaflow-ai/cmux#3095 by @austinywang Reference: manaflow-ai/cmux#3149 by @lawrencecchen Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
BenevolentFutures
added a commit
to Stage-11-Agentics/c11
that referenced
this pull request
Apr 27, 2026
…cks) (#87) * fix: propagate SSH_AUTH_SOCK through SSH subprocess environment macOS GUI apps launched from Finder or Dock do not inherit SSH_AUTH_SOCK or SSH_AGENT_PID because those are set by ssh-agent in the user's shell, not in the launch environment. Previously, WorkspaceRemoteDaemonRPCClient.start() and startReverseRelayLocked() created Process objects without setting process.environment, leaving it nil and relying on implicit inheritance. Explicit assignment of ProcessInfo.processInfo.environment to both SSH Process objects ensures agent socket variables pass through and SSH key authentication works from the GUI app. Reported-by: @knight42 <upstream issue manaflow-ai/cmux#3162> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: parse K/M/G byte-count suffixes in scrollback-limit config The Ghostty config format documents that scrollback-limit accepts K, M, and G suffixes (kibibytes, mebibytes, gibibytes). The previous parser used plain Int(value) and silently discarded values like "1G", "512M", or "100K", leaving the scrollback limit at the 10000-line default. Added a private parseByteCount helper that strips a trailing case-insensitive K/M/G suffix, parses the numeric prefix, and multiplies by the appropriate power-of-1024 factor. Plain integers continue to work unchanged. The Ghostty Zig layer expects a raw usize byte count, so expansion is handled entirely in the Swift config reader. Reported-by: @shaun0927 <upstream issue manaflow-ai/cmux#2950> Reference: manaflow-ai/cmux#2952 by @shaun0927 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: resolve named palette colors in WorkspaceApplyPlan customColor Config and CLI paths could supply a color as a human-readable name like "Red" or "Blue" rather than a hex string. The executor passed the raw string directly to setCustomColor, which normalizes only valid 6-digit hex values and silently no-ops everything else, causing the workspace tab to render with no color. Added resolveColorToHex, a nonisolated static helper that first tries normalizedHex for valid hex input and then does a case-insensitive lookup in WorkspaceTabColorSettings.defaultPaletteWithOverrides. Strings that are neither valid hex nor known palette names produce an ApplyFailure with code "unknown_color_name" rather than a silent no-op. Reported-by: @zacharygutt <upstream issue manaflow-ai/cmux#3075> Reference: manaflow-ai/cmux#3095 by @austinywang Reference: manaflow-ai/cmux#3149 by @lawrencecchen Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: invalidate GhosttyConfig cache on system appearance change light:X,dark:Y paired themes resolve to the correct variant via GhosttyConfig.currentColorSchemePreference(), but the load cache keyed on ColorSchemePreference was never cleared when macOS switched between light and dark mode. The stale cached config would be returned on the first load after an appearance change, keeping the wrong theme active until the next explicit config reload. Added a NSKeyValueObservation on NSApp.effectiveAppearance in AppDelegate.applicationDidFinishLaunching that calls GhosttyConfig.invalidateLoadCache() and then triggers GhosttyApp.shared.reloadConfiguration so terminals pick up the correct light or dark theme variant immediately. Reported-by: @Corey-T1000 <upstream issue manaflow-ai/cmux#2922> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: include .badge in notification authorization request requestAuthorizationIfNeeded called UNUserNotificationCenter.requestAuthorization with options [.alert, .sound] but omitted .badge. On macOS, setting NSApp.dockTile.badgeLabel requires .badge authorization — without it the assignment silently no-ops and the badge counter never appears in the Dock. The rest of the badge pipeline (isDockBadgeEnabled check, dockBadgeLabel computation, dockTile.badgeLabel assignment) was already correct. Adding .badge to the authorization options closes both #1764 and #2718 which report the same symptom. Reported-by: @gilsiun <upstream issue manaflow-ai/cmux#1764> Also-closes: manaflow-ai/cmux#2718 (reported by @tuzisang) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: remove .safeHelp from high-churn titlebar buttons to prevent UAF NSToolTipManager installs per-view tooltip handlers via SwiftUI's .help() modifier. When a hosting view is removed during split churn or workspace teardown, the NSView backing the tooltip may be freed while NSToolTipManager still holds a reference, causing a use-after-free crash on the next mouse-enter event. Removed .safeHelp() from the three high-churn titlebar control buttons (toggle sidebar, notifications, new workspace) which remount frequently during tab management. The .accessibilityLabel() calls already present on all three buttons preserve VoiceOver discoverability without touching NSToolTipManager. The safeHelp() extension remains available for use on stable views that do not remount during normal workspace interactions. Reported-by: @austinywang <upstream issue manaflow-ai/cmux#1036> Reference: manaflow-ai/cmux#1038 by @lawrencecchen Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: subtract legacy scrollbar gutter from terminal width in .legacy mode When a mouse is connected, macOS switches to legacy (always-visible) scrollers. The scroll view was initialized with scrollerStyle = .overlay but the system can override this, causing the vertical scrollbar to occupy physical column space. The previous synchronizeCoreSurface() used scrollView.contentSize.width directly, which does not account for the scrollbar gutter in legacy mode, so the rightmost terminal columns render under the scrollbar. The fix checks scrollView.scrollerStyle and subtracts NSScroller.scrollerWidth(for:.regular, scrollerStyle:.legacy) only when the effective style is .legacy. Overlay mode behavior is unchanged — that path was a prior deliberate decision to prevent column-flap during split churn (comment at line 8544 is preserved). Reported-by: @Thinkscape <upstream issue manaflow-ai/cmux#2997> Reference: manaflow-ai/cmux#3001 by @rdsciv Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: add coalescing guard to palette overlay update to reduce idle spin The focus-reassert path (reassertTerminalSurfaceFocus, 0.05s throttle) and first-responder scheduling (scheduleAutomaticFirstResponderApply, pendingAutomaticFirstResponderApply flag) were already guarded in c11 against redundant work. The third idle runloop offender was the WindowCommandPaletteOverlayController.update() path: SwiftUI calls it on every parent render cycle, and when the palette is hidden each call re-assigned hostingView.rootView = AnyView(EmptyView()), which drives a redundant SwiftUI layout pass. Added an early-exit guard: when both the current and new visibility state are false, skip the update entirely. The transition from visible to hidden is preserved — the guard only elides the no-op hidden-to-hidden calls. Reported-by: @lawrencecchen <upstream issue manaflow-ai/cmux#2996> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: prevent white-on-white text in light mode and honor config palette entries Two-part fix for the light-mode text regression introduced in 0.63.2: 1. Contrast fallback: added applyContrastFallbackIfNeeded(), a mutating helper called at the end of loadFromDisk. When both backgroundColor and foregroundColor have luminance > 0.5 (both near-white), the foreground is replaced with #1A1A1A. This closes the case where a light theme loads the background correctly but the foreground defaults to the near-white Monokai value (#fdfff1) because the theme file does not explicitly set foreground. This fix layers on Pick 5 (cache invalidation on appearance change) which ensures the correct light-vs-dark config is loaded after a system switch. 2. Palette override: applyPalette(to:config:) now checks config.palette[i] before falling back to the Monokai hardcoded default. When a theme file or config key sets palette entries, those colors are used for the ANSI palette instead of the defaults. Reported-by: @exlaw <upstream issue manaflow-ai/cmux#2708> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * ghostty: bump submodule to Stage-11-Agentics fork, fix PageList SIGSEGV race (#2738) Point the ghostty submodule at the Stage-11-Agentics/ghostty fork (all other c11 submodules already live under Stage-11-Agentics). The fork is based on manaflow-ai/ghostty main and carries one additional fix: terminal: snapshot page rows in SlidingWindow.Meta to fix SIGSEGV race SlidingWindow.highlight() was reading meta.node.data.size.rows directly from the live page node without holding the terminal lock. The IO thread calls resizeCols() (with the terminal lock) and frees old page nodes, so a concurrent search thread calling next() could crash with a use-after-free SIGSEGV. The fix snapshots the row count into Meta.rows during append() (which IS called under the terminal lock) and uses that snapshot throughout highlight(). Reported-by: @tmad4000 <upstream issue manaflow-ai/cmux#2738> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * localize: add workspace.apply.unknownColorName translations for 6 locales Covers C11-22 pick 4 (#3075) error string: unknown named color in config. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: guard parseByteCount against overflow and negative values scrollback-limit = 9000000000G caused an Int multiplication trap at config load. Add overflow check via multipliedReportingOverflow and reject negative prefixes before the multiply. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: update ghostty-fork.md for Stage-11-Agentics fork and PageList race fix Document the submodule URL change (manaflow-ai -> Stage-11-Agentics), the PageList SIGSEGV fix in sliding_window.zig, and conflict notes for future upstream syncs. Required by CLAUDE.md: "Keep docs/ghostty-fork.md up to date with any fork changes and conflict notes." Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: log when applyContrastFallbackIfNeeded overrides foreground color Silent override produced confusing 'my foreground setting is not applying' bugs. Log the old/new values so users can find this via Console.app or debug output. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: document named palette color support for customColor in schema WorkspaceLayoutExecutor now accepts named palette colors (e.g. "Red") in addition to hex strings. Unknown names produce an ApplyFailure with code unknown_color_name. Schema previously said hex-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor: make parseByteCount a static func (reads no instance state) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * ghostty: rebase PageList race fix onto c11 theme-picker fork tip (bc9be90a) The previous submodule bump (df2a237) was based on a fresh manaflow-ai/ghostty main, accidentally dropping 7 c11 theme-picker commits that c11 requires. This commit rebases the PageList SIGSEGV race fix onto bc9be90a (the existing c11 fork tip) so both the theme-picker hooks and the PageList fix are present. The 7 theme-picker commits modify src/cli/list_themes.zig to read CMUX_THEME_PICKER_COLOR_SCHEME, CMUX_THEME_PICKER_INITIAL_LIGHT, and CMUX_THEME_PICKER_INITIAL_DARK -- env vars set by c11 before calling ghostty +list-themes. Without them the theme picker ignores c11's setup. Reported-by: @tmad4000 <upstream issue manaflow-ai/cmux#2738> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: correct ghostty-fork.md ancestry and rename cmux to c11 in theme picker sections The fork base is bc9be90a (c11 theme-picker tip), not manaflow-ai main. Rename 'cmux theme picker' to 'c11 theme picker' per naming policy. Update PageList fix SHA to c64952975 (cherry-picked commit). Add upstream-sync note for list_themes.zig conflict guidance. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: reject plain negative values in parseByteCount fallback path scrollback-limit = -1 was accepted as -1 (no suffix branch, guard did not apply). Guard the Int(s) fallback the same way. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: substitute color name into ApplyFailure message for unknown_color_name String(localized:defaultValue:"\(color)") returns the xcstrings value verbatim in non-development builds; %@ was not substituted. Use String(format:) to fill the placeholder. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: fix customColor example to use a confirmed palette color name "Ocean Blue" does not exist in the palette; use a name that does. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: validate palette index is in 0...15 before assignment Out-of-bounds indices were silently accepted and could write past the 16-entry palette array. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs: update customColor doc comment to mention named palette colors Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * ci: fix build-ghosttykit and download to use Stage-11-Agentics/ghostty The workflow was publishing xcframework releases to manaflow-ai/ghostty using a token that only has write access to Stage-11-Agentics repos, causing exit code 4 on every PR. Similarly the download script was fetching from manaflow-ai/ghostty where no release exists for the Stage-11-Agentics fork's ghostty SHA. Fixes: - build-ghosttykit.yml: check-release and upload steps now target Stage-11-Agentics/ghostty - build-ghosttykit.yml: add "Pin checksum" step that computes SHA256 of the tarball post-upload and commits it to ghosttykit-checksums.txt, eliminating the manual step and the chicken-and-egg where the checksum guard fires before the release exists - download-prebuilt-ghosttykit.sh: default DOWNLOAD_URL now points to Stage-11-Agentics/ghostty Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: publish GhosttyKit releases to c11 repo using GITHUB_TOKEN GHOSTTY_RELEASE_TOKEN is not configured on this fork. Switch the build-ghosttykit workflow to publish xcframework releases to Stage-11-Agentics/c11 instead, using GITHUB_TOKEN with an explicit contents:write permission grant. Update download-prebuilt-ghosttykit.sh to fetch from the same location. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: checkout branch ref so git push works in Pin checksum step Actions checks out a detached HEAD for PRs by default, causing git push to fail with exit 128. Checking out the actual branch ref (head_ref for PRs, ref_name for push events) puts us on a real branch so the push succeeds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: always run Pin checksum, download tarball when release pre-exists If the release was created in a previous CI run (exists=true), the old conditional skipped Pin checksum entirely, leaving ghosttykit-checksums.txt unpopulated forever. Remove the condition and download the tarball from the existing release when the local file isn't present, so the checksum is always committed regardless of which run built the release. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: pin GhosttyKit checksum for ghostty c649529750b12e7fde7a33b74d5310a1b988cb67 --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CmuxConfigNamedColorTests.swift`:
- Around line 40-53: The test is flaky because decode(json) falls back to
UserDefaults.standard for palette lookup; make the test hermetic by passing an
isolated UserDefaults instance into decode (or supplying a fresh/empty
colorDefaults) so it can't read real palette names, e.g. create a new
UserDefaults(suiteName: UUID().uuidString) or a cleared temporary UserDefaults
and call decode(json, colorDefaults: thatInstance) inside
testDecodeWorkspaceCommandRejectsUnknownNamedColor to ensure the unknown name
cannot collide with system state.
🪄 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: 3f0782e5-7e4a-4926-bf47-7a0828e59db5
📒 Files selected for processing (5)
GhosttyTabs.xcodeproj/project.pbxprojSources/CmuxConfig.swiftSources/CmuxWorkspaceDefinition.swiftSources/WorkspaceTabColorResolution.swiftcmuxTests/CmuxConfigNamedColorTests.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- Sources/CmuxConfig.swift
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3075
Summary:
Verification:
xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -configuration Debug -destination 'platform=macOS' -derivedDataPath /tmp/cmux-3075-before -only-testing:cmuxTests/CmuxConfigDecodingTests/testDecodeWorkspaceCommandAcceptsNamedColor testfailed on commit4b444812withInvalid color "Indigo". Expected 6-digit hex format: #RRGGBB.\n- After fix: the same command passed on commit4c55836e.\n-git diff --checkpassed.\n-./scripts/reload.sh --tag c3075apassed.Summary by cubic
Allow named workspace colors in
cmux.json, resolving them from the configured workspace tab color palette and normalizing to hex. Also improves the error message to mention hex or a palette name.WorkspaceTabColorSettings.resolvedColorHex, falls back to hex, and supportsdecoder.userInfo[.cmuxWorkspaceColorDefaults]for deterministic decoding.cmux.json.Written for commit 8969041. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Improvements
Tests