Skip to content

Fix main CI by deferring file-drop overlay retries - #2979

Merged
austinywang merged 1 commit into
mainfrom
fix-main-ci-window-overlay
Apr 18, 2026
Merged

austinywang merged 1 commit into
mainfrom
fix-main-ci-window-overlay

Conversation

@austinywang

@austinywang austinywang commented Apr 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • remove the synchronous WindowSetupAccessor path added for file-drop overlay retries
  • retry file-drop overlay installation asynchronously from the existing deduped WindowAccessor
  • keep the retry bounded so startup can recover once the theme frame exists without mutating the window tree mid-attachment

Root cause

main CI started failing in the tests job on the two latest merge commits:

  • 24597524739 (26892bd0)
  • 24597559538 (060eafd6)

The shared regression started with 70618b0f (Fix file drop overlay startup installation).

The new WindowSetupAccessor called installFileDropOverlay while WindowObservingView.viewWillMove(toWindow:) / SwiftUI update was still attaching the window hierarchy. In the failing CI logs that produced:

  • NSWindow warning: adding an unknown subview: <cmux_DEV.FileDropOverlayView ...>
  • NSHostingView is being laid out reentrantly while rendering its SwiftUI content

Once that attachment path became re-entrant, unrelated unit suites started failing broadly because window/layout state and Ghostty surface setup were no longer stable.

Validation

  • required local setup completed: ./scripts/setup.sh
  • attempted tagged debug build twice via ./scripts/reload.sh --tag fix-main-ci-window-overlay
  • local verification was limited by an Xcode build-service crash/hang on this machine before compilation completed; PR CI should provide the authoritative signal

Note

Medium Risk
Changes window-setup timing and introduces bounded async retry logic during startup; risk is moderate due to potential for missed/late overlay installation or subtle lifecycle timing issues in SwiftUI/AppKit.

Overview
Defers file-drop overlay installation until the NSWindow view hierarchy is stable by adding installFileDropOverlayWhenReady, which retries installation on the next main-loop turn with a bounded attempt count.

Removes the synchronous retry path (WindowSetupAccessor) and installs the overlay from the existing window accessor callback, avoiding re-entrant mutations of the NSThemeFrame/hosting view tree during attachment.

Reviewed by Cursor Bugbot for commit 5c866bd. Bugbot is set up for automated code reviews on this repo. Configure here.


Summary by cubic

Defers file-drop overlay installation to the next run-loop and retries until the window is ready, preventing reentrant layout during SwiftUI attach. This fixes the CI failures caused by mutating the window hierarchy at startup.

  • Bug Fixes
    • Replaced synchronous setup with installFileDropOverlayWhenReady (async with 16 bounded retries).
    • Routed overlay install through the existing WindowAccessor; removed WindowSetupAccessor.
    • Eliminates NSWindow “unknown subview” and NSHostingView reentrant layout warnings seen in CI.

Written for commit 5c866bd. Summary will update on new commits.

Summary by CodeRabbit

Bug Fixes

  • Enhanced file drop functionality with improved overlay installation reliability using automatic retry logic. Ensures consistent and reliable operation across various system states and conditions.

Refactor

  • Consolidated internal window setup architecture by streamlining components and removing redundant patterns. All user-facing functionality is maintained while improving code maintainability.

@vercel

vercel Bot commented Apr 18, 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 Apr 18, 2026 6:19am

@coderabbitai

coderabbitai Bot commented Apr 18, 2026 •

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e143457-e628-493b-ab87-ddff3cc90070

📥 Commits

Reviewing files that changed from the base of the PR and between 060eafd and 5c866bd.

📒 Files selected for processing (2)
  • Sources/ContentView.swift
  • Sources/WindowAccessor.swift
💤 Files with no reviewable changes (1)
  • Sources/WindowAccessor.swift

📝 Walkthrough

Walkthrough

The pull request refactors the file-drop-overlay installation mechanism during app startup. It replaces the WindowSetupAccessor view-based retry pattern with a direct installFileDropOverlayWhenReady helper that defers installation attempts to the main dispatch queue, retrying up to 16 times on failure.

Changes

Cohort / File(s) Summary
File Drop Overlay Retry Mechanism
Sources/ContentView.swift
Added a new private installFileDropOverlayWhenReady(on:tabManager:remainingAttempts:) helper function that defers overlay installation via DispatchQueue.main.async and retries up to 16 times when initial attempts fail. Updated window setup hook to use this retry helper instead of direct installation call.
Window Setup Accessor Removal
Sources/WindowAccessor.swift
Removed the entire WindowSetupAccessor NSViewRepresentable struct along with its coordinator state and repeating-callback-until-success logic. The remaining WindowAccessor continues unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Hops and hoops, the overlay tries,
Deferred in queues, it multiplies,
Sixteen attempts through async halls,
No more repeats from view install calls!
A cleaner path for files to drop. 📦✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description provides a comprehensive summary of changes and root cause analysis, but lacks required sections including testing methodology, demo video (if applicable), review trigger commands, and checklist items. Add the Testing, Demo Video, Review Trigger, and Checklist sections as specified in the repository template, particularly documenting how the change was tested locally and verifying all checklist items.
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 (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix main CI by deferring file-drop overlay retries' accurately summarizes the main change: addressing CI failures by switching from synchronous to asynchronous deferred retry logic for overlay installation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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-main-ci-window-overlay

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 and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR removes WindowSetupAccessor — a second NSViewRepresentable that synchronously retried installFileDropOverlay on every SwiftUI updateNSView call — and replaces it with installFileDropOverlayWhenReady, an async-retry helper invoked from within the existing deduped WindowAccessor. The first installation attempt is still synchronous (and returns early if the theme frame isn't available yet), while every subsequent retry is dispatched via DispatchQueue.main.async, ensuring the window hierarchy is fully settled before mutation. The retry chain is bounded at 16 attempts, which prevents runaway async loops during edge-case startup hangs.

Confidence Score: 5/5

Safe to merge — the fix correctly defers window-tree mutation to post-layout and is bounded.

No P0/P1 issues found. The async-retry approach is idempotent (installFileDropOverlay checks for an existing overlay via objc_getAssociatedObject before adding a new one), deduplication in WindowAccessor prevents multiple concurrent chains, and the 16-attempt ceiling is generous for typical startup timing. All remaining considerations are P2.

No files require special attention.

Important Files Changed

Filename Overview
Sources/WindowAccessor.swift Removes WindowSetupAccessor (~42 lines) entirely; the remaining WindowAccessor and WindowObservingView are unchanged.
Sources/ContentView.swift Adds installFileDropOverlayWhenReady (async, bounded-retry wrapper) and moves the call site into the existing deduped WindowAccessor block instead of a separate WindowSetupAccessor modifier.

Sequence Diagram

sequenceDiagram
    participant SwiftUI as SwiftUI Layout
    participant WA as WindowAccessor(dedupeByWindow:true)
    participant IFDOWR as installFileDropOverlayWhenReady
    participant IFDOW as installFileDropOverlay
    participant MQ as DispatchQueue.main.async

    SwiftUI->>WA: viewWillMove(toWindow:) / viewDidMoveToWindow()
    WA->>IFDOWR: call (remainingAttempts=16)
    IFDOWR->>IFDOW: synchronous attempt
    alt themeFrame not yet ready
        IFDOW-->>IFDOWR: return false
        IFDOWR->>MQ: schedule retry (remainingAttempts=15)
        MQ->>IFDOWR: call (remainingAttempts=15)
        IFDOWR->>IFDOW: attempt #2
        IFDOW-->>IFDOWR: return true (themeFrame ready)
        note over IFDOWR: guard fails, return, no more retries
    else themeFrame already available
        IFDOW-->>IFDOWR: return true
        note over IFDOWR: guard fails, return immediately
    end
Loading

Reviews (1): Last reviewed commit: "Fix deferred file-drop overlay installat..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai 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.

No issues found across 2 files

@austinywang
austinywang merged commit c6d36a5 into main Apr 18, 2026
19 checks passed
rodchristiansen pushed a commit to rodchristiansen/cmux that referenced this pull request Sep 2, 2026
…ow-overlay

Fix main CI by deferring file-drop overlay retries

This branch was successfully deployed

1 active deployment
Preview — 5c866bd4 Deployed Apr 18, 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.

1 participant