Skip to content

Fix escaping-closure capture in sidebar slot view tint - #8329

Merged
azooz2003-bit merged 1 commit into
mainfrom
fix-sidebar-slot-self-capture
Jul 17, 2026
Merged

azooz2003-bit merged 1 commit into
mainfrom
fix-sidebar-slot-self-capture

Conversation

@azooz2003-bit

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

Copy link
Copy Markdown
Collaborator

Clean Debug builds of main currently fail: the NSImage drawing handler added in #8270 is escaping and references the enclosing view's color without an explicit capture, which is a hard Swift error. Capture the color by value. Found while merging main into feat-ios-panes-ux; required CI being off this week is why it landed unnoticed.

🤖 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 a Debug build error by explicitly capturing the tint color in the NSImage drawing handler for sidebar slot views. Keeps the tinting behavior the same while removing the escaping-closure capture issue.

  • Bug Fixes
    • Added [color] capture list to the NSImage drawing handler in SidebarRowPullRequestIconView.
    • Resolves Swift error from implicit capture of self and restores clean Debug builds.

Written for commit 0160d99. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved reliability when rendering icons for closed pull requests in the sidebar.

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>
@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b38b363c-90f9-4ed9-abf9-f63286ccfc87

📥 Commits

Reviewing files that changed from the base of the PR and between 32c5e33 and 0160d99.

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

📝 Walkthrough

Walkthrough

The closed pull request icon drawing closure now explicitly captures its tint color. No other rendering logic or public declarations changed.

Changes

Pull request icon rendering

Layer / File(s) Summary
Closed icon color capture
Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift
The .closed status drawing closure explicitly captures color when creating the tinted NSImage.

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

Possibly related issues

Possibly related PRs

🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning It explains the fix and why, but omits the template's Testing, Demo Video, Review Trigger, and Checklist sections. Add the missing sections with testing performed, a demo video link or note, the review-trigger comment, and checklist items or N/A where appropriate.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the change: fixing the escaping-closure color capture in the sidebar tint code.
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 Changed code is an @MainActor NSView; the only diff is explicit [color] capture in an NSImage handler, with no new actor-isolation debt.
Cmux Swift Blocking Runtime ✅ Passed Diff only adds an explicit [color] capture in draw(_:); no waits, sleeps, syncs, locks, or delays were introduced.
Cmux Browser Automation Off-Main ✅ Passed Diff only changes SidebarWorkspaceRowSlotViews.swift to capture color by value; no browser.* automation routing or WebKit/AppKit worker-lane code was touched.
Cmux Expensive Synchronous Load ✅ Passed Diff only adds an explicit [color] capture in an NSImage drawing closure; no agent-history load, JSON parsing, or sync I/O was added or moved.
Cmux Cache Substitution Correctness ✅ Passed Patch only adds an explicit capture in a transient AppKit draw closure; it does not replace any fresh read with a cached value in a persistence/history/undo/snapshot path.
Cmux No Hacky Sleeps ✅ Passed Only a Swift capture-list fix landed; no sleeps, timers, waits, or runtime-script delays were added.
Cmux Algorithmic Complexity ✅ Passed Only change is an explicit capture list in an NSImage draw closure; no collection scans, rescans, or hot-path algorithm changes.
Cmux Swift Concurrency ✅ Passed Diff only adds an explicit capture in an NSImage drawing handler; no Dispatch/Combine/Task/completion async patterns were introduced, and AppKit callback boundaries are allowed.
Cmux Swift @Concurrent ✅ Passed Only a sync NSImage draw closure changed to capture color by value; no @concurrent, nonisolated async, or actor-hopping issues were introduced.
Cmux Swift Package Boundaries ✅ Passed Diff only touches a pure AppKit sidebar view; the rule explicitly اجازت small UI/AppKit glue and not reusable domain logic.
Cmux Swiftpm Lockfiles ✅ Passed Only a Swift source file changed; no Package.resolved, Package.swift, .gitignore, Xcode, or workflow files were touched.
Cmux Swift Logging ✅ Passed Diff only adds an explicit [color] capture in the image tint closure; no print/debugPrint/dump/NSLog or other logging changes.
Cmux User-Facing Error Privacy ✅ Passed The only change is adding an explicit capture list in a drawing closure; no user-facing errors, alerts, API bodies, or recovery copy were added or altered.
Cmux Full Internationalization ✅ Passed Diff only adds an explicit closure capture in Swift; no user-facing text or locale assets changed, so full-internationalization rules aren't triggered.
Cmux Swiftui State Layout ✅ Passed Pure AppKit bridge view; only change is explicit color capture in an NSImage drawing closure, with no new SwiftUI state/layout patterns.
Cmux Architecture Rethink ✅ Passed Tiny compile fix only: the escaping NSImage drawing closure now captures color by value; no timing, ownership, or lifecycle architecture change.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed Changed file is a leaf NSView; the diff only adds a capture list in an NSImage closure and touches no window/controller or cmux shortcut code.
Cmux Source Artifacts ✅ Passed The only changed path is a hand-written Swift source file; no logs, caches, build output, or other artifact paths were added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed Only change is a capture-list fix in production Swift; no #if DEBUG, test/debug accessor, or other seam was added.
Cmux No Ambient Global State ✅ Passed The change is only a closure capture-list edit inside SidebarRowPullRequestIconView.draw; no new file-scope funcs, vars, namespaces, or singletons were added.
✨ 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

Fixes a hard Swift compiler error in SidebarWorkspaceRowSlotViews.swift where the NSImage(size:flipped:drawingHandler:) escaping closure implicitly captured self by referencing the color property without an explicit capture list.

  • Adds [color] to the closure capture list so the view's color value is captured by value at closure creation time, satisfying Swift's requirement that escaping closures must explicitly name self or any of its members.
  • The fix is a single-token change and does not affect runtime drawing behavior; the captured NSColor reference is immutable in this context.

Confidence Score: 5/5

Single-line build-fix change; no behavioral regression possible.

The change is exactly one token added to an escaping closure capture list. It unblocks Debug builds without touching any logic, state, or API surface. image is a local constant (no implicit self reference), color is now captured by value rather than through self, and the drawn output is identical.

No files require special attention.

Important Files Changed

Filename Overview
Sources/Sidebar/AppKitList/Cells/SidebarWorkspaceRowSlotViews.swift Adds [color] capture list to the escaping NSImage drawing handler — correct minimal fix for the implicit-self capture error; image is a local constant and does not need explicit capture.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant View as SidebarRowPullRequestIconView
    participant NSImage as NSImage(drawingHandler:)
    participant Handler as Escaping Drawing Handler

    View->>NSImage: create tinted image [color] captured by value
    Note over Handler: color value captured at closure creation time
    NSImage-->>Handler: invokes drawingHandler(drawRect)
    Handler->>Handler: image.draw() — sourceOver
    Handler->>Handler: color.set()
    Handler->>Handler: drawRect.fill() — sourceAtop
    Handler-->>NSImage: return true
    NSImage-->>View: tinted NSImage
    View->>View: tinted.draw(in: rect)
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"}}}%%
sequenceDiagram
    participant View as SidebarRowPullRequestIconView
    participant NSImage as NSImage(drawingHandler:)
    participant Handler as Escaping Drawing Handler

    View->>NSImage: create tinted image [color] captured by value
    Note over Handler: color value captured at closure creation time
    NSImage-->>Handler: invokes drawingHandler(drawRect)
    Handler->>Handler: image.draw() — sourceOver
    Handler->>Handler: color.set()
    Handler->>Handler: drawRect.fill() — sourceAtop
    Handler-->>NSImage: return true
    NSImage-->>View: tinted NSImage
    View->>View: tinted.draw(in: rect)
Loading

Reviews (1): Last reviewed commit: "Fix escaping-closure capture in sidebar ..." | Re-trigger Greptile

@azooz2003-bit
azooz2003-bit merged commit 493d5ad into main Jul 17, 2026
6 checks passed
azooz2003-bit added a commit that referenced this pull request Jul 17, 2026
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: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
azooz2003-bit added a commit that referenced this pull request Jul 20, 2026
* Add chronological iOS notification feed

* Fix notification feed UI test selector

* Use native tab queries in feed UI test

* test(ios): cover notification return and search refresh

* fix(ios): preserve notification feed context

* test(ios): reproduce stale notification target

* fix(ios): make notification feed context durable

* test(ios): reproduce buried workspace context

* fix(ios): promote workspace in notification rows

* test(ios): reproduce dense notification rows

* fix(ios): compact notification feed rows

* test(ios): reproduce incomplete workspace search

* fix(ios): make workspace search match its context

* Fix iOS reconnect and build isolation (#8299)

* test(ios): cover reconnect overlap cleanup

* fix(ios): retire superseded reconnect sessions

* test(ios): isolate saved dev Mac instances

* fix(ios): enforce build compatibility boundaries

* test(ios): cover startup status auth race

* fix(ios): reuse connect token for identity check

* test(auth): preserve selected team during refresh outage

* fix(auth): keep selected team effective during startup

* test(auth): keep cached sessions restoring until ready

* fix(ios): wait for auth restore before reconnect

* test(ios): cover compatibility review regressions

* fix(ios): address compatibility review findings

* test(ios): use deterministic compatibility timestamps

---------

Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>

* test(ios): keep search refresh fixture stable on main

* Fix escaping-closure capture in sidebar slot view tint (#8329)

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: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

* test: cover unread feed actions and centered toolbar

* feat(ios): add unread feed actions and compact bulk read

* test(ios): lock notification toolbar geometry

* test(ios): require semantic notification row labels

* feat(ios): simplify notification feed rows

* test: cover notification feed merge regressions

* fix: harden notification feed state and rendering

* test: remove notification history settling delay

* test: repair merged mosh coverage

---------

Co-authored-by: cmux reload-cloud <cmux-reload-cloud@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant