Skip to content

Fix macOS 27 symbol launch crash - #6728

Merged
azooz2003-bit merged 7 commits into
mainfrom
issue-6703-macos27-symbol-crash
Jun 26, 2026
Merged

azooz2003-bit merged 7 commits into
mainfrom
issue-6703-macos27-symbol-crash

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jun 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Add an AppKit-backed CmuxSystemSymbolImage that resolves SF Symbols through NSImage.SymbolConfiguration, clamps invalid point sizes, and renders a template NSImage from SwiftUI.
  • Move launch and restored main-window chrome symbols in the sidebar, browser toolbar, titlebar, text box, and right sidebar off SwiftUI Image(systemName:) so first layout avoids SwiftUICore renderVectorGlyph and CoreUI CUINamedVectorGlyph.

Testing


View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.


Summary by cubic

Fixes a macOS 27 launch crash by routing SF Symbol rendering through an AppKit-backed path with consistent template rasterization, aspect‑ratio‑preserving fitting, and caching, while preserving global font magnification where expected. Closes #6703.

  • Bug Fixes
    • Added CmuxSystemSymbolImage using NSImage.SymbolConfiguration; clamps sizes, preserves aspect ratio, returns template images, caches AppKit symbols (256 max, FIFO), and bounds the renderability cache for unknown symbols (512 max, FIFO).
    • Preserved global font magnification via a new resolver and appliesGlobalFontMagnification, applied across sidebar, tabs, browser toolbar, titlebar, right sidebar, text box, and headers.
    • Replaced SwiftUI symbol rendering with CmuxSystemSymbolImage across chrome, removed the no‑op flat color helper, and kept call sites short via magnified: and explicit magnification flags.
    • Expanded tests to cover magnification scaling, clamping, template output, aspect‑ratio fitting, cache reuse, and unknown‑symbol rejection with negative‑lookup caching.

Written for commit a3e7c20. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

Refactor

  • Unified SF Symbol rendering across the app using a shared system-symbol image helper with explicit systemName, pointSize, and weight, replacing ad-hoc font/raster sizing approaches.
  • Improved consistency by centralizing AppKit symbol rasterization with clamped sizing, aspect-preserving fits, and cached template images across navigation, toolbars, sidebars, workspace controls, browser panels, and various inputs/popovers.

Tests

  • Added AppKit-backed tests for point-size magnification rules, size clamping, cache reuse, non-square aspect behavior, fitted-symbol scaling, and unknown-symbol handling.

@vercel

vercel Bot commented Jun 24, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Jun 24, 2026 5:33am
cmux-staging Building Building Preview, Comment Jun 24, 2026 5:33am

@coderabbitai

coderabbitai Bot commented Jun 24, 2026 •

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds AppKit-backed SF Symbol rasterization through RenderableSystemSymbol and CmuxSystemSymbolImage, then migrates many icon renderings across browser chrome, sidebar content, titlebar, and text input surfaces to the new API.

Changes

SF Symbol rendering migration

Layer / File(s) Summary
Core symbol image API and tests
Sources/RenderableSystemSymbol.swift, cmuxTests/RenderableSystemSymbolTests.swift
RenderableSystemSymbol adds AppKit image cache state, cached template NSImage creation, fitted sizing, and a SwiftUI wrapper view; the DEBUG cache reset clears both caches, and tests cover magnification, clamping, cache reuse, aspect fitting, and unknown symbol handling.
Browser panel, panel content, and titlebar icons
Sources/Panels/BrowserPanelView.swift, Sources/Panels/PanelContentView.swift, Sources/Update/UpdateTitlebarAccessory.swift
Browser chrome icons, panel headers, and notification/titlebar icons switch from Image(systemName:)-based rendering to CmuxSystemSymbolImage with explicit point sizes and weights; the browser panel also drops its flat-color rendering shim.
Sidebar, workspace, and content icons
Sources/RightSidebarChromeStyle.swift, Sources/SidebarWorkspaceGroupHeaderView.swift, Sources/ContentView.swift
Right sidebar chrome, workspace group headers, and many ContentView icon renderings switch to CmuxSystemSymbolImage, including browser-stack, extension, workspace, branch, metadata, and status icons.
Text input and attachment icons
Sources/TextBoxInput.swift
Attachment, action, and pending-comment icons in the text input area switch to CmuxSystemSymbolImage while keeping the existing magnification behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • manaflow-ai/cmux#5999: Changes Sources/RenderableSystemSymbol.swift and the same symbol-rendering path this PR replaces with CmuxSystemSymbolImage.
  • manaflow-ai/cmux#6396: Also updates RenderableSystemSymbol.swift and multiple ContentView/PanelContentView symbol call sites in the same rendering flow.
  • manaflow-ai/cmux#5670: Overlaps with the sidebar help icon rendering path in Sources/ContentView.swift.

Suggested reviewers

  • lawrencecchen

Poem

🐇 The icons hopped to a steadier beat,
With point sizes set and weights so neat.
Template images glimmer, cached just right,
And beta-bug shadows fade out of sight.
A carrot for symbols, crisp and clear,
So the app can launch without fear.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (2 errors, 1 warning)

Check name Status Explanation Resolution
Cmux No Test Or Debug Seam In Production Source ❌ Error Sources/RenderableSystemSymbol.swift adds #if DEBUG static func resetRenderabilityCacheForTesting(), a test-only seam in shipping source with no production caller. Move the cache-reset into cmuxTests via @testable import; if tests need access, widen state to internal instead of adding a production ForTesting helper.
Cmux No Ambient Global State ❌ Error Sources/RenderableSystemSymbol.swift:5 adds a caseless enum namespace with mutable @MainActor caches (lines 11-15), which the rule forbids. Move the caches/rendering logic onto an instance-owned renderer/service, inject it at the SwiftUI/AppKit seam, and keep only private file-scope helpers if needed.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (22 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely states the main fix: a macOS 27 symbol launch crash.
Linked Issues check ✅ Passed The PR implements the macOS 27 main-window symbol rasterization guard and migrates launch-time symbol rendering as required by #6703.
Out of Scope Changes check ✅ Passed The changed files all support the symbol-rendering crash fix; no unrelated feature work is evident.
Cmux Swift Actor Isolation ✅ Passed MainActor-isolated AppKit symbol caches are only used from SwiftUI/UI or @MainActor callers; no new Sendable/shared-mutable or background-store isolation debt.
Cmux Swift Blocking Runtime ✅ Passed No new semaphores, sleeps, sync waits, polling, or manual locks were added; the new symbol cache is @MainActor-isolated and the tests are nonblocking.
Cmux Browser Automation Off-Main ✅ Passed browser.wait/screenshot/cookies stay in socketWorkerMethods and v2BrowserAutomationCommandOnSocketWorker, with v2MainSync hops; policy tests cover them.
Cmux Expensive Synchronous Load ✅ Passed Diff only adds AppKit-backed SF Symbol rendering/caching; no new synchronous agent-history/JSON load was moved onto MainActor or an interactive path.
Cmux Cache Substitution Correctness ✅ Passed The cache only memoizes transient SF Symbol rasterization for UI rendering; no persistence/history/undo/snapshot authoritative read was replaced, and cold misses still resolve fresh.
Cmux No Hacky Sleeps ✅ Passed No new fixed sleeps, polling, or timer-based race workarounds were added; the only setTimeout is existing spinner timing, and the Slack notifier was removed.
Cmux Algorithmic Complexity ✅ Passed No new scalable scans or hot-path rescans were introduced; the only new loops are fixed-size symbol candidates and a bounded 256-entry cache eviction.
Cmux Swift Concurrency ✅ Passed PASS: The PR adds a synchronous @MainActor symbol cache/helper and UI/test icon swaps; it doesn’t introduce new DispatchQueue, Combine state, completion handlers, or unmanaged Tasks.
Cmux Swift @Concurrent ✅ Passed No edited hunk adds nonisolated async or @concurrent code; the new symbol helper is synchronous @MainActor UI-bound rendering.
Cmux Swift File And Package Boundaries ✅ Passed PASS: The PR adds one focused 219-line AppKit/SwiftUI symbol bridge and makes small incidental icon swaps in already-large UI files.
Cmux Swiftpm Lockfiles ✅ Passed No Package.resolved, .gitignore, or package-reference changes appeared in the PR diff; vendored bonsplit keeps its upstream Package.resolved ignore policy.
Cmux Swift Logging ✅ Passed No added print/debugPrint/dump/NSLog/Logger/os_log in the touched runtime Swift files; the only logging found is existing cmuxDebugLog under #if DEBUG.
Cmux User-Facing Error Privacy ✅ Passed The PR only migrates SF Symbol rendering/caching and adds tests; it introduces no new user-facing error, alert, command-output, or recovery copy exposing sensitive details.
Cmux Full Internationalization ✅ Passed The PR only swaps SF Symbol rendering/caching/tests; no new user-facing text, locale catalogs, or web i18n files are added or changed.
Cmux Swiftui State Layout ✅ Passed PR only swaps SF Symbol renderers for an AppKit-backed bridge view; no new ObservableObject/@published, no new layout GeometryReader, and no render-time SwiftUI state writes added.
Cmux Architecture Rethink ✅ Passed The change is a focused AppKit symbol-rendering bridge with a single MainActor cache, not a timing/lifecycle workaround or split ownership fix.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR only updates icon-rendering views/helpers; it չի adds or changes NSWindow/NSPanel/WindowGroup code or cmuxAuxiliaryWindowIdentifiers ownership.
Cmux Source Artifacts ✅ Passed Only existing submodule gitlinks changed (ghostty, vendor/bonsplit); no artifact, cache, build, or scratch paths were added.
Description check ✅ Passed Summary and Testing are present, but the template’s Demo Video, Review Trigger, and Checklist sections are missing.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-6703-macos27-symbol-crash

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jun 24, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR routes SF Symbol rendering off SwiftUI's Image(systemName:) and through an AppKit-backed CmuxSystemSymbolImage view backed by NSImage.SymbolConfiguration, fixing a macOS 27 launch crash caused by renderVectorGlyph/CUINamedVectorGlyph racing at first layout. A @MainActor-isolated, bounded LRU-style cache (256 images, 512 renderability entries) is added to RenderableSystemSymbol to avoid per-render AppKit allocations.

  • New CmuxSystemSymbolImage struct wraps AppKit symbol lookup with size clamping, aspect-ratio-preserving fitting, template-mode rendering, and a two-tier @MainActor cache; a magnified: init label selectively opts into global font magnification.
  • Call-site migration across ContentView, BrowserPanelView, PanelContentView, RightSidebarChromeStyle, SidebarWorkspaceGroupHeaderView, TextBoxInput, and UpdateTitlebarAccessory replaces cmuxFont/cmuxSymbolRasterSize with the new component; cmuxFlatSymbolColorRendering() no-op is removed.
  • Expanded test coverage verifies magnification scaling, size clamping, template output, aspect-ratio fitting, cache identity reuse, and negative-lookup caching for unknown symbols.

Confidence Score: 5/5

Safe to merge — the change is a mechanical, well-tested migration of SF Symbol rendering to an AppKit-backed path with proper @mainactor isolation and bounded caching.

The new CmuxSystemSymbolImage implementation is correct: the @MainActor-isolated cache avoids per-render allocations, size clamping is applied consistently, aspect-ratio fitting is verified by tests, and the magnified: vs systemName: label distinction cleanly separates magnification-aware from fixed-size call sites. No concurrency violations, no data races, no disk I/O on interactive paths, and no user-facing text changes. The open concern from prior review threads (the #if DEBUG resetRenderabilityCacheForTesting() seam extended to cover the two new caches) is the only residual issue and was already known before this PR.

Sources/RenderableSystemSymbol.swift warrants a second look for the pre-existing #if DEBUG production seam, which this PR extended; all other files are straightforward call-site migrations.

Important Files Changed

Filename Overview
Sources/RenderableSystemSymbol.swift Core change: adds CmuxSystemSymbolImage view and configuredAppKitImage/resolvedRasterPointSize/fittedSymbolSize static helpers; bounded @mainactor caches for AppKit images and renderability. The existing #if DEBUG resetRenderabilityCacheForTesting() seam is extended to also reset the two new caches (noted in previous review thread).
Sources/ContentView.swift Systematic replacement of Image(systemName:).cmuxFont/.cmuxSymbolRasterSize with CmuxSystemSymbolImage across sidebar, tab row, and footer views. Magnification intentionally extended to media-activity indicators (speaker/mic/camera) and close button that previously used fixed system font sizes.
Sources/Panels/BrowserPanelView.swift Replaces all chrome toolbar symbols (nav arrows, reload, screenshot, keyboard, devtools, profile, theme, import) with CmuxSystemSymbolImage; removes cmuxFlatSymbolColorRendering() no-op extension. All browser toolbar icons remain non-magnified (systemName: init), consistent with the original cmuxSymbolRasterSize behavior.
Sources/Panels/PanelContentView.swift PanelFilePathHeader header icon now uses CmuxSystemSymbolImage with explicit pointSize: 16 (was unsized Image); PanelHeaderIconGlyph migrated from cmuxSymbolRasterSize(13) to CmuxSystemSymbolImage. Straightforward migration with no behavioral regressions.
Sources/RightSidebarChromeStyle.swift HeaderChromeIconStyle.symbol() and ModeBarButton migrated to CmuxSystemSymbolImage; ModeBarButton explicitly opts in to appliesGlobalFontMagnification: true to match the original cmuxFont behavior.
Sources/SidebarWorkspaceGroupHeaderView.swift Chevron, icon, and plus-button symbols migrated to CmuxSystemSymbolImage with appliesGlobalFontMagnification: true, matching original cmuxFont behavior. Clean, mechanical replacement.
Sources/TextBoxInput.swift Attachment preview, chip, add-files, send, and pending-comments icons migrated to CmuxSystemSymbolImage with the magnified: init (matching original cmuxFont behavior). No issues.
Sources/Update/UpdateTitlebarAccessory.swift Titlebar chrome and notifications popover icons migrated to CmuxSystemSymbolImage; uses non-magnified systemName: init consistently with original cmuxSymbolRasterSize. Clean migration.
cmuxTests/RenderableSystemSymbolTests.swift New tests cover magnification scaling, size clamping, template output, non-square aspect ratio, cache identity reuse, and negative-lookup caching for unknown symbols. Tests are @MainActor-annotated where required and rely on the #if DEBUG resetRenderabilityCacheForTesting() fixture.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["CmuxSystemSymbolImage.body"] --> B["resolvedRasterPointSize(pointSize, globalFontPercent, appliesGlobalFontMagnification)"]
    B --> C{"appliesGlobalFontMagnification?"}
    C -- "No" --> D["clampedRasterPointSize(pointSize) → rasterSize ≥ 1"]
    C -- "Yes" --> E["clampedRasterPointSize then GlobalFontMagnification.scaledSize → rasterSize"]
    D --> F["configuredAppKitImage(systemName, rasterSize, weight) @MainActor"]
    E --> F
    F --> G{"appKitImageCache hit?"}
    G -- "Yes" --> H["Return cached NSImage"]
    G -- "No" --> I{"renderabilityCache == false?"}
    I -- "Yes" --> J["Return nil → Color.clear placeholder"]
    I -- "No" --> K["NSImage(systemSymbolName:)"]
    K -- "nil" --> L["cacheRenderability(false) → Return nil"]
    K -- "image" --> M["withSymbolConfiguration(pointSize, weight)"]
    M --> N["copy(), isTemplate = true, size = fittedSymbolSize(natural, max: rasterSize)"]
    N --> O["Store in appKitImageCache (FIFO evict at 256)"]
    O --> H
    H --> P["Image(nsImage:).renderingMode(.template).frame(rasterSize × rasterSize)"]
    J --> Q["Color.clear.frame(rasterSize × rasterSize).accessibilityHidden(true)"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["CmuxSystemSymbolImage.body"] --> B["resolvedRasterPointSize(pointSize, globalFontPercent, appliesGlobalFontMagnification)"]
    B --> C{"appliesGlobalFontMagnification?"}
    C -- "No" --> D["clampedRasterPointSize(pointSize) → rasterSize ≥ 1"]
    C -- "Yes" --> E["clampedRasterPointSize then GlobalFontMagnification.scaledSize → rasterSize"]
    D --> F["configuredAppKitImage(systemName, rasterSize, weight) @MainActor"]
    E --> F
    F --> G{"appKitImageCache hit?"}
    G -- "Yes" --> H["Return cached NSImage"]
    G -- "No" --> I{"renderabilityCache == false?"}
    I -- "Yes" --> J["Return nil → Color.clear placeholder"]
    I -- "No" --> K["NSImage(systemSymbolName:)"]
    K -- "nil" --> L["cacheRenderability(false) → Return nil"]
    K -- "image" --> M["withSymbolConfiguration(pointSize, weight)"]
    M --> N["copy(), isTemplate = true, size = fittedSymbolSize(natural, max: rasterSize)"]
    N --> O["Store in appKitImageCache (FIFO evict at 256)"]
    O --> H
    H --> P["Image(nsImage:).renderingMode(.template).frame(rasterSize × rasterSize)"]
    J --> Q["Color.clear.frame(rasterSize × rasterSize).accessibilityHidden(true)"]
Loading

Reviews (5): Last reviewed commit: "Bound renderability symbol cache" | Re-trigger Greptile

Comment on lines +109 to 124
var body: some View {
let rasterSize = RenderableSystemSymbol.clampedRasterPointSize(pointSize)
let systemFont: Font = weight.map { .system(size: rasterSize, weight: $0) } ?? .system(size: rasterSize)
return font(systemFont)
.frame(width: rasterSize, height: rasterSize, alignment: alignment)
if let image = RenderableSystemSymbol.configuredAppKitImage(
systemName: systemName,
pointSize: rasterSize,
weight: weight
) {
Image(nsImage: image)
.renderingMode(.template)
.frame(width: rasterSize, height: rasterSize, alignment: alignment)
} else {
Color.clear
.frame(width: rasterSize, height: rasterSize, alignment: alignment)
.accessibilityHidden(true)
}
}

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 Per-render NSImage allocations on hot sidebar-row path

CmuxSystemSymbolImage.body calls configuredAppKitImage unconditionally on every render, allocating at least three NSImage objects per symbol (NSImage(systemSymbolName:), withSymbolConfiguration, copy()). TabItemView alone hosts up to 8 CmuxSystemSymbolImage instances (pin, audio, mic, camera, xmark, two branch icons, circle.fill), meaning ≥24 AppKit allocations per visible tab per render cycle. The previous cmuxSymbolRasterSize modifier was a zero-allocation font/frame attribute change — this replacement adds unbounded AppKit churn on every view invalidation for every visible row. Add a @MainActor static var imageCache: [CacheKey: NSImage] = [:] on RenderableSystemSymbol (keyed by (systemName, rasterSize, weight)) matching the pattern already used for renderabilityCache, so repeat renders of the same symbol at the same size reuse the existing NSImage instance.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 72665b1 by adding a bounded @mainactor cache keyed by symbol name, clamped raster size, and AppKit font weight. Added a test that repeated configuredAppKitImage calls reuse the same NSImage instance.

— Claude Code

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/RenderableSystemSymbol.swift`:
- Around line 95-98: The image sizing logic at line 98 forces NSImage to a
square size regardless of the symbol's natural aspect ratio, causing non-square
SF Symbols to be distorted. Instead of setting image.size to NSSize(width:
rasterSize, height: rasterSize), calculate the proportional dimensions by
determining the natural aspect ratio of the symbol after applying the
configuration and scaling it so the longer dimension equals rasterSize while
maintaining the correct aspect ratio. Additionally, update the test that
currently uses the symmetric questionmark.circle symbol to use a non-square
symbol like chevron.right or arrow.left to verify that non-square symbols render
without distortion.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 46173ec4-369f-4eaf-86d0-4c9da8584a5d

📥 Commits

Reviewing files that changed from the base of the PR and between 94ed3b2 and 72665b1.

📒 Files selected for processing (2)
  • Sources/RenderableSystemSymbol.swift
  • cmuxTests/RenderableSystemSymbolTests.swift

Comment thread Sources/RenderableSystemSymbol.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/RightSidebarChromeStyle.swift (1)

28-32: 🎯 Functional Correctness | 🟡 Minor

Verify appliesGlobalFontMagnification consistency for header chrome icons.

HeaderChromeIconStyle.symbol does not set appliesGlobalFontMagnification, defaulting to false. This differs from ModeBarButton (line 357) and all three SidebarWorkspaceGroupHeaderView icons (lines 128, 151, 188), which explicitly set appliesGlobalFontMagnification: true. Confirm that header chrome icons should remain fixed size when users adjust global font magnification settings, or add the parameter if they should scale consistently with other sidebar UI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/RightSidebarChromeStyle.swift` around lines 28 - 32, The
CmuxSystemSymbolImage initialization in HeaderChromeIconStyle.symbol is missing
the appliesGlobalFontMagnification parameter, which defaults to false and
differs from the consistency pattern used in ModeBarButton and
SidebarWorkspaceGroupHeaderView icons that explicitly set this to true. Add the
appliesGlobalFontMagnification parameter to the CmuxSystemSymbolImage call and
set it to true to ensure header chrome icons scale consistently with other
sidebar UI elements when users adjust global font magnification settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@Sources/RightSidebarChromeStyle.swift`:
- Around line 28-32: The CmuxSystemSymbolImage initialization in
HeaderChromeIconStyle.symbol is missing the appliesGlobalFontMagnification
parameter, which defaults to false and differs from the consistency pattern used
in ModeBarButton and SidebarWorkspaceGroupHeaderView icons that explicitly set
this to true. Add the appliesGlobalFontMagnification parameter to the
CmuxSystemSymbolImage call and set it to true to ensure header chrome icons
scale consistently with other sidebar UI elements when users adjust global font
magnification settings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 840e68a8-656f-4c52-8500-9bb338018322

📥 Commits

Reviewing files that changed from the base of the PR and between 72665b1 and 0228823.

📒 Files selected for processing (6)
  • Sources/ContentView.swift
  • Sources/RenderableSystemSymbol.swift
  • Sources/RightSidebarChromeStyle.swift
  • Sources/SidebarWorkspaceGroupHeaderView.swift
  • Sources/TextBoxInput.swift
  • cmuxTests/RenderableSystemSymbolTests.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cmuxTests/RenderableSystemSymbolTests.swift`:
- Around line 62-71: The test fittedSymbolSizeScalesLongerDimensionToRasterSize
only tests landscape orientation (width larger than height) and an invalid-size
fallback, but does not cover portrait orientation (height larger than width).
Add another `#expect` assertion to the
fittedSymbolSizeScalesLongerDimensionToRasterSize function that tests the
portrait case by calling RenderableSystemSymbol.fittedSymbolSize with
NSSize(width: 10, height: 20) and maximumDimension: 16, expecting it to return
NSSize(width: 8, height: 16), which verifies the function correctly scales the
longer dimension regardless of orientation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 10e89a79-b84a-4192-8b51-b97910b2f8fe

📥 Commits

Reviewing files that changed from the base of the PR and between 0228823 and c3f35cb.

📒 Files selected for processing (2)
  • Sources/RenderableSystemSymbol.swift
  • cmuxTests/RenderableSystemSymbolTests.swift

Comment on lines +62 to +71
@Test func fittedSymbolSizeScalesLongerDimensionToRasterSize() {
#expect(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 20, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 8))
#expect(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 0, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 16))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a portrait aspect-ratio case.

This covers landscape scaling and invalid-size fallback, but not the opposite orientation. A regression that always scales from width could still pass; add 10×20 → 8×16 to lock in portrait symbols too.

🧪 Proposed test addition
     `@Test` func fittedSymbolSizeScalesLongerDimensionToRasterSize() {
         `#expect`(RenderableSystemSymbol.fittedSymbolSize(
             NSSize(width: 20, height: 10),
             maximumDimension: 16
         ) == NSSize(width: 16, height: 8))
+        `#expect`(RenderableSystemSymbol.fittedSymbolSize(
+            NSSize(width: 10, height: 20),
+            maximumDimension: 16
+        ) == NSSize(width: 8, height: 16))
         `#expect`(RenderableSystemSymbol.fittedSymbolSize(
             NSSize(width: 0, height: 10),
             maximumDimension: 16
         ) == NSSize(width: 16, height: 16))
     }
📝 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
@Test func fittedSymbolSizeScalesLongerDimensionToRasterSize() {
#expect(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 20, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 8))
#expect(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 0, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 16))
}
`@Test` func fittedSymbolSizeScalesLongerDimensionToRasterSize() {
`#expect`(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 20, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 8))
`#expect`(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 10, height: 20),
maximumDimension: 16
) == NSSize(width: 8, height: 16))
`#expect`(RenderableSystemSymbol.fittedSymbolSize(
NSSize(width: 0, height: 10),
maximumDimension: 16
) == NSSize(width: 16, height: 16))
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmuxTests/RenderableSystemSymbolTests.swift` around lines 62 - 71, The test
fittedSymbolSizeScalesLongerDimensionToRasterSize only tests landscape
orientation (width larger than height) and an invalid-size fallback, but does
not cover portrait orientation (height larger than width). Add another `#expect`
assertion to the fittedSymbolSizeScalesLongerDimensionToRasterSize function that
tests the portrait case by calling RenderableSystemSymbol.fittedSymbolSize with
NSSize(width: 10, height: 20) and maximumDimension: 16, expecting it to return
NSSize(width: 8, height: 16), which verifies the function correctly scales the
longer dimension regardless of orientation.

Comment on lines 151 to 158
#if DEBUG
@MainActor
static func resetRenderabilityCacheForTesting() {
renderabilityCache.removeAll()
appKitImageCache.removeAll()
appKitImageCacheInsertionOrder.removeAll()
}
#endif

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 #if DEBUG test seam in production source worsened

resetRenderabilityCacheForTesting() already existed as a #if DEBUG-guarded function named with ForTesting, which matches the pattern the team's production-seam rule explicitly fails. This PR worsens the seam by extending it to also reset appKitImageCache and appKitImageCacheInsertionOrder — two new private caches whose only reset path in tests is now this compile-guarded function. The canonical fix is to widen appKitImageCache and appKitImageCacheInsertionOrder from private to internal, then let the test target reach them directly via @testable import, removing the need for the #if DEBUG function to cover the new state (or removing the function entirely).

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/TextBoxInput.swift (1)

2879-2890: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add an explicit accessibility label for the icon-only dismiss button.

This button has .help(...) but no .accessibilityLabel(...), which can make VoiceOver output ambiguous.

Suggested patch
             Button {
                 dismissPendingComments()
             } label: {
                 CmuxSystemSymbolImage(magnified: "xmark", pointSize: 9, weight: .bold)
                     .frame(width: 16, height: 16)
                     .background(Circle().fill(foreground.opacity(0.12)))
             }
             .buttonStyle(.plain)
             .help(String(
                 localized: "textbox.diffComments.dismiss",
                 defaultValue: "Dismiss comments without sending"
             ))
+            .accessibilityLabel(String(
+                localized: "textbox.diffComments.dismiss",
+                defaultValue: "Dismiss comments without sending"
+            ))
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/TextBoxInput.swift` around lines 2879 - 2890, The icon-only dismiss
Button in TextBoxInput should have an explicit accessibility label in addition
to the existing help text. Update the Button that calls dismissPendingComments()
and uses CmuxSystemSymbolImage so it includes a clear .accessibilityLabel(...)
describing the dismiss action, while keeping the current .help(...) for tooltip
behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@Sources/TextBoxInput.swift`:
- Around line 2879-2890: The icon-only dismiss Button in TextBoxInput should
have an explicit accessibility label in addition to the existing help text.
Update the Button that calls dismissPendingComments() and uses
CmuxSystemSymbolImage so it includes a clear .accessibilityLabel(...) describing
the dismiss action, while keeping the current .help(...) for tooltip behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a360f8c2-f526-483e-81bb-10f122159c9b

📥 Commits

Reviewing files that changed from the base of the PR and between c3f35cb and b19dc3c.

📒 Files selected for processing (6)
  • Sources/ContentView.swift
  • Sources/Panels/BrowserPanelView.swift
  • Sources/RenderableSystemSymbol.swift
  • Sources/TextBoxInput.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • cmuxTests/RenderableSystemSymbolTests.swift

@azooz2003-bit
azooz2003-bit merged commit d4342fb into main Jun 26, 2026
31 checks passed
@azooz2003-bit
azooz2003-bit deleted the issue-6703-macos27-symbol-crash branch June 26, 2026 05:01
austinywang added a commit that referenced this pull request Jun 29, 2026
* Add failing guard for macOS 27 launch-path SF Symbols (#6745)

Asserts the content views laid out during launch / session restore never
use SwiftUI Image(systemName:) / Label(systemImage:), which crash on
macOS 27 via CoreUI CUINamedVectorGlyph _rasterizeImageUsingScaleFactor
during the first window layout. This commit adds only the test; it fails
because NotificationsPage, WorkspaceContentView, TerminalPanelView, and
RemoteTmuxPaneHeader still use the crash-prone APIs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Fix macOS 27 launch crash in restore-path content views (#6745)

#6728 moved launch *chrome* (sidebar, titlebar, toolbars) off SwiftUI
Image(systemName:) to dodge the macOS 27 CoreUI CUINamedVectorGlyph
_rasterizeImageUsingScaleFactor crash, but missed the content-area views
that are laid out on the first frame of a launched or session-restored
window.

The decisive one is NotificationsPage: it is mounted unconditionally in
the main content ZStack (ContentView.terminalContent) and only toggled via
.opacity, so its body — and its empty-state `bell.slash` glyph — is measured
during the first NSWindow.makeKeyAndOrderFront: layout on every launch, even
when the sidebar shows the tab list. That raw Image(systemName:) still
crashes the app before any window appears on macOS 27 beta 2, exactly as
reported.

Route these views' SF Symbols through the AppKit-backed CmuxSystemSymbolImage
instead:
- NotificationsPage: bell.slash, bell.badge, xmark.circle.fill
- WorkspaceContentView (EmptyPanelView): terminal.fill + the Terminal/Browser
  action-button labels (Label(systemImage:) rasterizes the same way)
- TerminalPanelView (AgentHibernationPlaceholderView): pause.circle
- RemoteTmuxPaneHeader: pane control glyphs

The last three render the moment a saved layout containing an empty panel, a
hibernated agent, or a remote tmux pane is restored — no user interaction
required — so they are part of the same launch-crash surface.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Address review: behavioral symbol coverage + dedupe empty-pane button

- Replace the source-scanning launch-path guard test (flagged in review as a
  brittle "fake regression test") with behavioral coverage: assert every
  launch/restore-path SF Symbol resolves through the non-crashing AppKit path
  (configuredAppKitImage) the fix routes them onto. The macOS 27 crash itself
  only reproduces on a macOS 27 runner, which is documented on the test.
- Dedupe emptyPaneActionButton's two near-identical branches into one shared
  button with a conditional .keyboardShortcut, keeping behavior identical and
  bringing WorkspaceContentView.swift back under the Swift file-length budget.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Address review: label clear button; drop non-guarding symbol test

- Add an explicit .accessibilityLabel to NotificationRow's icon-only clear
  button (reusing the existing notifications.row.clear string). Migrating to
  CmuxSystemSymbolImage drops the implicit VoiceOver name the SwiftUI
  system-symbol path supplied, so VoiceOver would otherwise announce an
  unlabeled button. Matches RemoteTmuxPaneHeader's existing pattern.
- Remove the launch-path symbol test. Both the source-scan and the
  configuredAppKitImage variants were flagged in review as not real regression
  guards: the macOS 27 CoreUI crash only reproduces on macOS 27, so no test on
  CI's older macOS can distinguish the safe AppKit route from the crashing
  SwiftUI route. Per repo policy ("if the bug isn't cleanly testable, say so
  rather than faking a test"), the protection is the production migration plus
  manual macOS 27 validation, documented on the PR. The existing
  RenderableSystemSymbolTests already cover the AppKit rendering path itself.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: cmux <cmux@cmuxs-Mac-mini.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
Preview – cmux — a3e7c20b Deployed Jun 24, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Launch crash on macOS 27.0 beta 2 (26A5368g): CoreUI SF Symbol rasterization (CUINamedVectorGlyph) in createMainWindow

1 participant