Fix Quick Look preview deactivation crash

[austinywang]

Fixes #4453.

Repro

Per the user override, I built and launched the tagged dev app locally before fixing:

CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-4453-quicklook-deactivated-crash --launch

I reproduced the crash in that dev app with:

printf '\000\001\002\003cmux quicklook repro\000\004' > /tmp/cmux-ql-repro.issue4453.bin
CMUX_TAG=issue-4453-quicklook-deactivated-crash scripts/cmux-debug-cli.sh open /tmp/cmux-ql-repro.issue4453.bin --workspace workspace:1

Before the fix, the tagged app aborted and wrote ~/Library/Logs/DiagnosticReports/cmux DEV-2026-05-20-172719.ips with the Quick Look assertion:

[QL] -[QLPreviewView setPreviewItem:blockingUntilLoading:timeoutDate:transition:]: item == nil || _reserved->internalState != QLPreviewDeactivatedInternalState

Test Approach

The first commit adds FilePreviewReviewFeedbackTests.testQuickLookSessionCloseDoesNotDeactivateMountedRepresentableView. It creates a Quick Look file preview, calls the session close path while the representable view is still mounted, then updates that same view. On the un-fixed implementation this crashes through the same QLPreviewView.setPreviewItem path; with the fix it leaves the retired view unconfigured.

Additional regression coverage verifies stale SwiftUI dismantle callbacks do not reset the active preview item, and that PanelOwnedNativeViewSession still performs one dismantle callback for a view that was previously retired by close().

Fix

PanelOwnedNativeViewSession now separates three native-view states:

• active owned view: may be configured by SwiftUI updates
• retired view: closed by the model and blocked from future configuration
• dismantled view: already torn down, so duplicate SwiftUI dismantle calls are ignored

FilePreviewQuickLookSession now clears previewItem and removes the view from its superview for both close and dismantle. It deliberately does not call QLPreviewView.close(): that API permanently rejects future preview items and also asserts when Quick Look considers the view inactive. The session retirement guard prevents stale updates from reconfiguring old views without entering Quick Look's deactivated state.

QuickLookPreviewView still routes SwiftUI dismantleNSView into the session so SwiftUI-owned teardown participates in the same lifecycle path.

Verification

• Reproduced the crash before fixing in the tagged dev app.
• Confirmed the first regression test failed pre-fix with Crash: cmux DEV ... libsystem_c.dylib: abort() called.
• Confirmed the stale-dismantle regression failed before the ownership fix, then passed.
• Confirmed the close-then-dismantle regression failed before retired/dismantled state separation, then passed.
• Confirmed the focused Quick Look lifecycle tests pass after the fix.
• Rebuilt with the required command and reran the CLI Quick Look repro; open returned OK, the tagged socket stayed reachable, and no newer DiagnosticReports crash was created.

No cloud-mac video was used.

Summary by CodeRabbit

• Bug Fixes

  • Improved cleanup for file preview sessions with explicit teardown to prevent stale native view state.
  • Strengthened lifecycle handling to avoid resetting active Quick Look preview items when retired views are removed.
  • Made teardown idempotent and prevented retired views from being reconfigured.

• Tests

  • Added tests covering Quick Look preview lifecycle and post-teardown preview-item behavior.
  • Added a temporary binary-file helper to support the tests.

[vercel]

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

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	May 21, 2026 1:10am
cmux-staging	Building	Preview, Comment	May 21, 2026 1:10am

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR separates Quick Look view lifecycles into close (soft retirement) and dismantle (hard teardown), tracks retired/dismantled view identities to avoid reconfiguring deactivated QLPreviewView instances, adds SwiftUI representable teardown, and adds tests exercising these transitions.

Changes

Quick Look Preview Lifecycle Management

Layer / File(s)	Summary
Base session lifecycle infrastructure Sources/Panels/PanelOwnedNativeViewSession.swift	PanelOwnedNativeViewSession adds an optional dismantleView callback, retiredViews and dismantledViews sets, avoids configuring retired views in update, close() marks the owned view as retired before cleanup, and introduces @discardableResult func dismantle(_:) -> Bool for idempotent dismantling.
Quick Look session handlers Sources/Panels/FilePreviewQuickLookSession.swift	FilePreviewQuickLookSession wires both closeView and dismantleView to a static releaseView that clears QLPreviewView.previewItem and removes the view from its superview (no longer calling QLPreviewView.close() there). Adds instance dismantle(_:) which forwards to the view session and clears cached FilePreviewQLItem only when dismantling succeeds.
SwiftUI representable teardown Sources/Panels/FilePreviewPanel.swift	QuickLookPreviewView adds makeCoordinator() and implements dismantleNSView(_:coordinator:) to invoke the panel's Quick Look session dismantle(_:) when SwiftUI removes the representable.
Tests and helpers cmuxTests/FilePreviewReviewFeedbackTests.swift	Imports Quartz, adds Quick Look lifecycle tests validating QLPreviewView.previewItem across close/dismantle and retired/active view transitions, and adds temporaryBinaryFile() test helper.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• manaflow-ai/cmux#4298: Introduced the initial PanelOwnedNativeViewSession infrastructure; this PR extends it with dismantle lifecycle and retirement tracking.

Poem

🐰 I nudged the preview's sleepy view,

Close says "retire", dismantle says "adieu".
SwiftUI tells when it leaves the room,
Now Quick Look won't panic or boom. 🥕

---

Caution

Pre-merge checks failed

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

• Ignore

❌ Failed checks (1 error, 1 warning)

Check name	Status	Explanation	Resolution
Cmux No Hacky Sleeps	❌ Error	PR adds sleep/setTimeout/setInterval in TypeScript, JavaScript, and shell scripts for timeouts, backoff, and polling.	Review timeout/retry logic per runtime-no-hacky-sleeps.md; consider event-driven or cancellation-aware alternatives to fixed delays.
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 (15 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR fully addresses the objectives from #4453: prevents setPreviewItem on deactivated QLPreviewView by implementing retired/dismantled view state separation, wires representable dismantle into the session, and includes comprehensive regression tests.
Out of Scope Changes check	✅ Passed	All changes are directly focused on fixing the Quick Look deactivation crash: retirement/dismantle state in PanelOwnedNativeViewSession, teardown logic in FilePreviewQuickLookSession, dismantle callback in QuickLookPreviewView, and three targeted regression tests.
Cmux Swift Actor Isolation	✅ Passed	FilePreviewQuickLookSession and PanelOwnedNativeViewSession are @MainActor. QuickLookPreviewView is NSViewRepresentable (intentionally MainActor). No improper actor isolation introduced.
Cmux Swift Blocking Runtime	✅ Passed	Core Quick Look fix files contain no blocking primitives. asyncAfter for focus animation and NSLock for drag registry are allowed exceptions per swift-blocking-runtime rules.
Cmux Swift Concurrency	✅ Passed	PR introduces only synchronous @MainActor lifecycle callbacks for AppKit/SwiftUI, not legacy async patterns. No Dispatch queues, Combine, fire-and-forget Tasks, or unsafe completion handlers added.
Cmux Swift @Concurrent	✅ Passed	All Swift changes follow concurrent annotation rules: synchronous functions properly @MainActor-isolated; async waitForPanelSave intentionally inherits caller's actor for UI-bound coordination only.
Cmux Swift File And Package Boundaries	✅ Passed	Adds +59 lines fixing #4453: +16 to FilePreviewPanel.swift (UI glue, under 250-line threshold for oversized file). Focused bug fix with small amount of code to large file, preserving extraction path.
Cmux Swift Logging	✅ Passed	No logging violations found in the PR. All production source files contain zero instances of print, debugPrint, dump, or NSLog statements.
Cmux User-Facing Error Privacy	✅ Passed	No user-facing errors or sensitive info exposed. Structural teardown logic changes with only internal developer comments and test code.
Cmux Full Internationalization	✅ Passed	PR introduces only internal code changes: lifecycle management methods and session tracking for Quick Look view teardown. No new user-facing strings, localization keys, or string catalog entries.
Cmux Swiftui State Layout	✅ Passed	PR introduces no new SwiftUI state patterns violating swiftui-state-layout.md. QuickLookPreviewView is an AppKit bridge view allowed by rules; other changes are model-layer or test code.
Cmux Architecture Rethink	✅ Passed	Proper architectural fix separating close/dismantle lifecycles with clear ownership, named invariants, and guarded updates. No prohibited patterns found.
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR modifies lifecycle of embedded NSViewRepresentable within existing FilePreviewPanel; no new NSWindow, NSPanel, NSWindowController, or WindowGroup created.
Title check	✅ Passed	The title 'Fix Quick Look preview deactivation crash' clearly and concisely describes the main change—fixing a specific crash caused by Quick Look preview deactivation.
Description check	✅ Passed	The PR description provides comprehensive coverage of the issue, reproduction steps, fix details, and verification approach.

✨ 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-4453-quicklook-deactivated-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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

Fixes a crash where SwiftUI could call updateNSView on a QLPreviewView after the session was closed, driving the view into a permanently-deactivated state that caused QLPreviewView.setPreviewItem to abort. The fix introduces session retirement so that update() calls for closed views are silently ignored, and adds a separate dismantle() path wired through a new NSViewRepresentable Coordinator so that irreversible teardown is deferred until SwiftUI actually removes the view.

• PanelOwnedNativeViewSession now tracks retired and dismantled views via ObjectIdentifier sets, guards update() against retired views, and exposes a dismantle() method that returns whether the view being dismantled was the current owner (so callers can decide whether to clear associated model state).
• FilePreviewQuickLookSession splits the old closeView into a non-destructive releaseView (clears previewItem, removes from superview — intentionally never calls QLPreviewView.close()) shared by both the close and dismantle paths, and gates item = nil in dismantle() on the session's return value to avoid clobbering a freshly-started session's item when a retired view is later torn down.
• Regression tests covering close-then-update (verifies no crash), dismantle-of-retired-view (verifies active item survives), and the base PanelOwnedNativeViewSession close/dismantle sequencing are added alongside a temporaryBinaryFile() test helper.

Confidence Score: 5/5

Safe to merge. The retirement guard and dismantle path are correct across all close/reopen/late-dismantle orderings, and the targeted regression tests validate each path.

All changed code stays on the main actor, the ObjectIdentifier-based retirement sets are properly cleared when a view is re-acquired via view(), and the conditional item = nil in dismantle() correctly protects an active session from a late dismantle of a previously-retired view. The three new regression tests directly exercise the crash path, the active-item-preservation path, and the base session sequencing. No correctness issues were found.

No files require special attention.

Important Files Changed

Filename	Overview
Sources/Panels/PanelOwnedNativeViewSession.swift	Adds retirement tracking via two ObjectIdentifier sets, guards update() against retired views, and adds dismantle() returning whether the dismantled view was the active owner. Logic is correct across the close→view→dismantle and close→dismantle-before-reopen orderings.
Sources/Panels/FilePreviewQuickLookSession.swift	Splits the former closeView into a shared releaseView (no QLPreviewView.close() call — intentional per inline comment), adds dismantle() that only clears item when the return value confirms the active view was dismantled. Correctly protects against the close-then-new-session-then-late-dismantle scenario.
Sources/Panels/FilePreviewPanel.swift	Adds makeCoordinator(), dismantleNSView(_:coordinator:), and a lightweight Coordinator that holds a panel reference. Standard NSViewRepresentable teardown plumbing; no logic concerns.
cmuxTests/FilePreviewReviewFeedbackTests.swift	Adds three targeted regression tests plus a temporaryBinaryFile() helper. Tests cover the crash path, the active-item-preservation path, and the base session sequencing. Assertions match the intended invariants.

Sequence Diagram

sequenceDiagram
    participant SwiftUI
    participant QLPV as QuickLookPreviewView
    participant FQLS as FilePreviewQuickLookSession
    participant PONS as PanelOwnedNativeViewSession
    participant QL as QLPreviewView

    SwiftUI->>QLPV: makeNSView
    QLPV->>FQLS: view(panel)
    FQLS->>PONS: view(configure)
    PONS->>QL: makeView
    PONS-->>FQLS: view1
    FQLS->>QL: configure previewItem
    FQLS-->>QLPV: view1

    Note over FQLS,PONS: Session closes
    FQLS->>PONS: close()
    PONS->>PONS: retire view1
    PONS->>QL: "releaseView previewItem=nil"
    FQLS->>FQLS: "item=nil"

    Note over SwiftUI,QL: SwiftUI calls updateNSView before dismantling
    SwiftUI->>QLPV: updateNSView(view1)
    QLPV->>FQLS: update(view1)
    FQLS->>PONS: update(view1 configure)
    PONS->>PONS: retiredViews contains view1 - early return
    Note over QL: setPreviewItem NOT called - no crash

    Note over SwiftUI,QL: SwiftUI tears down representable
    SwiftUI->>QLPV: dismantleNSView(view1 coordinator)
    QLPV->>FQLS: dismantle(view1)
    FQLS->>PONS: dismantle(view1)
    PONS->>PONS: insert dismantledViews returns false
    PONS->>QL: "releaseView previewItem=nil"
    FQLS->>FQLS: false - item unchanged

Loading

_{Reviews (2): Last reviewed commit: "fix: avoid manual Quick Look close durin..." | Re-trigger Greptile}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 2985e74. Configure here.}

[cubic-dev-ai]

No issues found across 4 files

_{Re-trigger cubic}