Skip to content

Fix explicit-self capture breaking clean macOS builds of main - #8331

Merged
lawrencecchen merged 3 commits into
mainfrom
fix-sidebar-slot-self-capture
Jul 17, 2026
Merged

lawrencecchen merged 3 commits into
mainfrom
fix-sidebar-slot-self-capture

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Jul 17, 2026 •

Copy link
Copy Markdown
Collaborator

Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift:101 references color inside the NSImage(size:flipped:) drawing closure without explicit self, which Swift rejects (escaping closure capture semantics). Clean checkouts of main fail to build the macOS app with reference to property 'color' in closure requires explicit use of 'self'; unnoticed because PR CI is off during the advisory experiment (second clean-build breakage this week after the unpushed bonsplit pointer, manaflow-ai/bonsplit#188).

One line: self.color.set(). Verified by compiling the tagged dev app from a checkout that previously failed.

🤖 Generated with Claude Code


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


Summary by cubic

Fixes the macOS build by capturing the tint [color] in the escaping NSImage(size:flipped:) drawing closure in SidebarWorkspaceRowSlotViews. This meets Swift capture rules, restores clean builds of main, and ensures pull request icons render with the correct tint.

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

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Fixed pull request icons in the sidebar so their closed-state rendering consistently uses the correct tint and stroke color.

cmux reload-cloud and others added 2 commits July 17, 2026 00:57
The NSImage drawing handler is escaping, so referencing the enclosing
view's color without an explicit capture fails to compile; capture the
color by value. Unbreaks clean Debug builds of current main carried into
this branch.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift does
not compile on a clean checkout: the NSImage drawing closure references
the view's color property without explicit self, which Swift rejects in
an escaping closure. macOS builds of main have been broken since the
change landed (PR CI is disabled this week, so nothing caught it).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The closed-state pull request icon rendering closure now explicitly captures the color value when constructing the tinted NSImage.

Changes

Sidebar icon rendering

Layer / File(s) Summary
Closed-state tint update
Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift
The .closed rendering closure explicitly captures color for image tinting.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The diff only changes an NSImage closure capture inside an @MainActor UI view; it adds no new actor-isolation debt or background access.
Cmux Swift Blocking Runtime ✅ Passed The diff only adds explicit capture in an NSImage drawing closure; no semaphores, waits, sleeps, syncs, polls, or locks were introduced.
Cmux Browser Automation Off-Main ✅ Passed Only SidebarWorkspaceRowSlotViews.swift changed; no browser automation routing or policy code was touched, so the off-main browser rule doesn't apply.
Cmux Expensive Synchronous Load ✅ Passed Diff only changes self.color capture inside an NSImage drawing closure; no expensive agent-history load or new main-actor sync parsing was added.
Cmux Cache Substitution Correctness ✅ Passed Patch only fixes explicit self capture in a transient sidebar tinting closure; it doesn't replace any fresh read with a cached value in a persistence/history/snapshot path.
Cmux No Hacky Sleeps ✅ Passed Diff only changes self.color.set() to color.set() in a Swift drawing closure; no sleeps, timers, polling, or waits were added.
Cmux Algorithmic Complexity ✅ Passed The diff is a one-line explicit-self capture fix in a single NSImage drawing closure; it adds no collection scans, sorting, batching, or hot-path algorithmic work.
Cmux Swift Concurrency ✅ Passed Diff is a one-line AppKit closure-capture fix; it adds no DispatchQueue, Combine, completion-handler, or fire-and-forget Task patterns.
Cmux Swift @Concurrent ✅ Passed The diff only changes an NSImage drawing closure inside a @MainActor view; it adds no async work or @concurrent annotations, so the rule is not violated.
Cmux Swift Package Boundaries ✅ Passed Changed file is a small AppKit view in Sources/Sidebar/AppKitList/Cells, and the rule explicitly allows UI-only/AppKit glue; no reusable domain logic was added.
Cmux Swiftpm Lockfiles ✅ Passed Diff only touches a Swift source file; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes to trigger the rule.
Cmux Swift Logging ✅ Passed Diff only changes color.set() to self.color.set() in a drawing closure; no print/debugPrint/dump/NSLog/Logger changes were added.
Cmux User-Facing Error Privacy ✅ Passed The diff only changes a closure capture in an AppKit tinting path and adds no user-facing error, alert, or sensitive text.
Cmux Full Internationalization ✅ Passed Patch only changes self.color.set() to color.set() inside a drawing closure; no user-facing text, localization keys, or locale files changed.
Cmux Swiftui State Layout ✅ Passed The change is an AppKit NSView bridge and only adds an explicit color capture in a drawing closure; it introduces no SwiftUI state/layout patterns covered by the rule.
Cmux Architecture Rethink ✅ Passed PASS: Diff is a one-line explicit self.color.set() in a view-drawing closure; it’s a local correctness fix with no new timing, observer, or ownership split.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR only tweaks icon tinting in SidebarWorkspaceRowSlotViews.swift; it doesn't add or modify any NSWindow/NSPanel/WindowGroup code or window identifiers.
Cmux Source Artifacts ✅ Passed Only a hand-written Swift source file changed; no logs, caches, build output, temp dirs, or other source-control artifacts were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Only a one-line closure capture fix in a production Sources file; no #if DEBUG/test hooks or seam-style members were added.
Cmux No Ambient Global State ✅ Passed Diff only changes a closure capture inside an instance method; it adds no top-level func/var, singleton, or static-only namespace.
Title check ✅ Passed The title matches the core change: fixing a macOS build issue caused by explicit self capture in the sidebar icon closure.
Description check ✅ Passed The description covers the summary and testing, and the missing demo/review/checklist sections are non-critical for this code-only fix.
✨ 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 fix-sidebar-slot-self-capture

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 Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a Swift escaping-closure build error that broke clean macOS checkouts of main. The NSImage(size:flipped:) drawing closure in SidebarRowPullRequestIconView.draw referenced the color stored property without explicit self, which Swift rejects under its escaping-closure capture rules.

  • The fix adds a [color] value capture to the closure and keeps the existing color.set() call, so the closure uses the captured NSColor reference without needing self. The capture list and the call site are consistent.
  • No other code is changed; the fix is minimal and correct.

Confidence Score: 5/5

Safe to merge — single-character delta restoring a build-breaking omission with no behavioural change beyond what the original code intended.

The change is a minimal, isolated closure capture-list addition that unblocks clean macOS builds. The captured NSColor reference matches what the closure was already reading, the call site (color.set()) is consistent with the capture list, and no other logic is affected.

No files require special attention.

Important Files Changed

Filename Overview
Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift Adds [color] capture list to the NSImage drawing closure so color.set() uses the captured NSColor reference rather than requiring implicit self, resolving the Swift escaping-closure build error.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["draw(_ dirtyRect:)"] --> B{status == .closed?}
    B -- yes --> C["Fetch SF Symbol image"]
    C --> D["NSImage closure\n[color] capture"]
    D --> E["image.draw(.sourceOver)"]
    E --> F["color.set() – uses captured NSColor"]
    F --> G["drawRect.fill(.sourceAtop)"]
    G --> H["tinted.draw in rect"]
    B -- no --> I["Draw open / merged\nvector path with color.setStroke()"]
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["draw(_ dirtyRect:)"] --> B{status == .closed?}
    B -- yes --> C["Fetch SF Symbol image"]
    C --> D["NSImage closure\n[color] capture"]
    D --> E["image.draw(.sourceOver)"]
    E --> F["color.set() – uses captured NSColor"]
    F --> G["drawRect.fill(.sourceAtop)"]
    G --> H["tinted.draw in rect"]
    B -- no --> I["Draw open / merged\nvector path with color.setStroke()"]
Loading

Reviews (2): Last reviewed commit: "Use captured sidebar icon tint color" | Re-trigger Greptile

Comment on lines +99 to +101
let tinted = NSImage(size: image.size, flipped: false) { [color] drawRect in
image.draw(in: drawRect, from: .zero, operation: .sourceOver, fraction: 1)
color.set()
self.color.set()

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.

P2 The capture list [color] captures the property value by copy but then self.color.set() ignores that copy entirely and reads the property through self — so [color] is dead code and self is still captured strongly. The fix resolves the compiler error, but the capture list should either be removed (keep self.color.set()) or actually be used (drop the self. so the closure uses the copied value and avoids capturing self at all).

Suggested change
let tinted = NSImage(size: image.size, flipped: false) { [color] drawRect in
image.draw(in: drawRect, from: .zero, operation: .sourceOver, fraction: 1)
color.set()
self.color.set()
let tinted = NSImage(size: image.size, flipped: false) { [color] drawRect in
image.draw(in: drawRect, from: .zero, operation: .sourceOver, fraction: 1)
color.set()

@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/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift`:
- Around line 99-101: Update the NSImage drawing handler in the tinted image
construction to use the captured color snapshot via color.set() instead of
accessing self.color. Keep the existing image drawing behavior unchanged.
🪄 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: 7dc13c5f-9130-4a48-910c-dae0655fd40e

📥 Commits

Reviewing files that changed from the base of the PR and between 493d5ad and 9522349.

📒 Files selected for processing (1)
  • Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift

Comment thread Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift Outdated
@lawrencecchen
lawrencecchen merged commit e7c83de into main Jul 17, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the fix-sidebar-slot-self-capture branch July 17, 2026 10:19
@lawrencecchen
lawrencecchen restored the fix-sidebar-slot-self-capture branch July 18, 2026 10:24
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.

2 participants