Skip to content

Fix current main Swift CI regressions - #8892

Merged
lawrencecchen merged 4 commits into
mainfrom
fix-main-ci-post-8876
Jul 26, 2026
Merged

lawrencecchen merged 4 commits into
mainfrom
fix-main-ci-post-8876

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Jul 25, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fix the two deterministic Swift regressions that remain on current main:

  • mark ContentView.init as @MainActor, accept an optional injected CmuxFeatureFlags, and resolve .shared inside the actor-isolated body so Swift 6 does not evaluate a main-actor singleton in a nonisolated default argument;
  • check the stored optional self.actions before replacement in SidebarGroupHeaderTableCellView.configure, restoring the intended full apply for fresh/reused cells.

The original window-snapshot actor fix is already present on main and dropped out when this branch was synchronized.

Verification

  • Exact-head autoreview with Swift addendum: clean, 0.98 confidence.
  • Greptile: 5/5, safe to merge.
  • CodeRabbit, Cubic, Greptile, Socket statuses: successful.
  • Unresolved review threads: 0.
  • Tagged macOS 26 Debug build: mci89, workflow run 30185040587, BUILD_OK.
  • Merge-conflict gate against current main: clean.

Localization audit

The final PR diff changes no user-facing strings, localization keys, schema text, docs, menus, alerts, or tooltips. Resources/Localizable.xcstrings is not part of the current-main diff; the bot warning about menu.history.reopenClosedWorkspace refers to an upstream merge, not this PR.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026 •

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

ContentView initialization now uses main-actor isolation and optional feature-flag injection with a shared fallback. Sidebar group-header configuration bases full model re-application on the cell’s stored actions state.

Changes

ContentView actor isolation and feature flags

Layer / File(s) Summary
ContentView initialization contract
Sources/ContentView.swift
The initializer is annotated with @MainActor, accepts optional CmuxFeatureFlags, and falls back to .shared when no flags are supplied.

Sidebar model application

Layer / File(s) Summary
Group-header apply condition
Sources/Sidebar/AppKitList/Cells/SidebarGroupHeaderRowView.swift
requiresFullApply now checks self.actions instead of the incoming actions parameter.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: austinywang, azooz2003-bit


Important

Pre-merge checks failed

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

❌ Failed checks (1 error, 1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Cmux Full Internationalization ❌ Error PR adds menu.history.reopenClosedWorkspace, but its new Resources/Localizable.xcstrings entry only has en/ja while the catalog supports many locales. Add translated values for every supported locale in Resources/Localizable.xcstrings (and any other touched catalog entries) before shipping.
Description check ⚠️ Warning The description has Summary and Verification, but it omits required template sections for Testing, Demo Video, Review Trigger, and the Checklist. Add the missing sections with test steps, any demo link or N/A, the review-trigger comment block, and completed checklist items.
Cmux No Ambient Global State ❓ Inconclusive pending investigation Need to inspect diff and review-bot rule file before deciding.
✅ Passed checks (22 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 Changes are UI-only and MainActor-aligned; ContentView resolves CmuxFeatureFlags.shared on-main, and the sidebar cell is already @MainActor.
Cmux Swift Blocking Runtime ✅ Passed PR diff only changes ContentView init to @MainActor and nil-default featureFlags; no waits, sleeps, syncs, or locks were added.
Cmux Browser Automation Off-Main ✅ Passed Only ContentView and sidebar header changed; rule-scoped browser automation files and policies were untouched.
Cmux Expensive Synchronous Load ✅ Passed The diff only main-actor-isolates ContentView init and fixes sidebar header reuse logic; no expensive agent-history load or JSON/scan code was added or moved.
Cmux Cache Substitution Correctness ✅ Passed Snapshot/render-grid cache state adds historyRows+rowSpaceRevision freshness checks and cold fallback; the ContentView/sidebar changes are UI/reuse fixes, not cache substitutions.
Cmux No Hacky Sleeps ✅ Passed Non-Swift changes are a Bun test runner and tests; no added sleeps, polling, or fixed waits, and workflow YAML is out of scope.
Cmux Algorithmic Complexity ✅ Passed Diff adds one fixed-size command contribution and constant-time dispatch; the sidebar change only flips a guard source, with no nested scans or rescans.
Cmux Swift Concurrency ✅ Passed Actor-isolation and reuse-logic fixes only; no new DispatchQueue/Combine/completion-handler/fire-and-forget patterns were introduced in the touched SwiftUI/AppKit code.
Cmux Swift @Concurrent ✅ Passed The diff only changes sync, UI-bound code: ContentView.init is @MainActor, and the sidebar header fix is a synchronous configure guard change.
Cmux Swift Package Boundaries ✅ Passed Only changed app-target UI/AppKit glue: ContentView init/MainActor and reused sidebar cell apply logic; no reusable domain logic crossed a package boundary.
Cmux Swiftpm Lockfiles ✅ Passed Diff only changes two Swift source files; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project lockfile changes to police.
Cmux Swift Logging ✅ Passed PR Swift diff only changes MainActor/init and sidebar reuse logic; no added print/debugPrint/dump/NSLog/Logger/os_log or sensitive logging.
Cmux User-Facing Error Privacy ✅ Passed No user-facing errors, alerts, command output, or recovery copy were added or changed; the PR only adjusts actor isolation and a reuse check.
Cmux Swiftui State Layout ✅ Passed No new SwiftUI state/layout anti-patterns were added; ContentView only changed init/defaults and command actions, and the sidebar change is an AppKit bridge snapshot/closure cell.
Cmux Architecture Rethink ✅ Passed PASS: the patch only MainActor-isolates ContentView init and resolves shared flags there; no new timing, observer, lock, or split-owner pattern was introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed MobilePairingWindowController already uses cmux.mobilePairingWindow and is listed in cmuxAuxiliaryWindowIdentifiers; this PR only changes sizing behavior.
Cmux Source Artifacts ✅ Passed Changed paths are intentional source/config/test/docs, and the only unusual entry is a deliberate ghostty submodule pointer update.
Cmux No Test Or Debug Seam In Production Source ✅ Passed No new test/debug seam in production sources; the changes are init/actor and reuse-logic fixes, and the new DEBUG iOS preview file is genuine debug-only UI scaffolding.
Title check ✅ Passed The title is concise and accurately summarizes the main change: fixing current main Swift CI regressions.
✨ 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-post-8876

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 25, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes two deterministic Swift CI regressions: a Swift 6 actor isolation violation in ContentView.init and a cell-reuse logic bug in SidebarGroupHeaderTableCellView.configure.

  • ContentView.init actor isolation fix: Marks the init @MainActor and changes the featureFlags parameter from CmuxFeatureFlags = .shared to CmuxFeatureFlags? = nil, resolving .shared inside the @MainActor body. This prevents Swift 6 from flagging access to the main-actor-isolated .shared singleton from a non-isolated default-argument expression.
  • Sidebar header cell-reuse fix: Changes let requiresFullApply = actions == nil to let requiresFullApply = self.actions == nil. The parameter actions is a non-optional type so comparing it to nil was always false, silently suppressing full-apply on the first configure of any reused or fresh cell. Reading the stored optional self.actions (set to nil by suspendPresentation() on reuse) restores the intended first-apply path.

Confidence Score: 5/5

Both changes are minimal, targeted, and correct — safe to merge.

The ContentView.init change correctly moves a main-actor-isolated singleton access inside a @MainActor-annotated initializer, which is the canonical Swift 6 fix for this class of isolation error. The self.actions == nil change fixes a clearly unintentional comparison of a non-optional parameter to nil (always false), restoring the intended full-apply path for reused and fresh cells. Neither change introduces new state, new synchronization, or new architectural concerns; both narrow an existing correctness gap.

Files Needing Attention: No files require special attention; both changed files have isolated, easily-verified one-or-two-line fixes.

Important Files Changed

Filename Overview
Sources/ContentView.swift Marks ContentView.init @mainactor and defers CmuxFeatureFlags.shared resolution to within the init body; correct Swift 6 actor isolation fix with no behavioral regression.
Sources/Sidebar/AppKitList/Cells/SidebarGroupHeaderRowView.swift One-line fix: reads self.actions (optional stored property, nil after reuse) instead of the non-optional actions parameter; restores the first-apply guard for recycled sidebar header cells.

Sequence Diagram

sequenceDiagram
    participant TableView as NSTableView
    participant Cell as SidebarGroupHeaderTableCellView

    Note over Cell: prepareForReuse()
    Cell->>Cell: "suspendPresentation() → self.actions = nil"
    Cell->>Cell: "self.model = nil"

    TableView->>Cell: configure(model:actions:...)
    Cell->>Cell: "requiresFullApply = self.actions == nil (true after reuse)"
    Cell->>Cell: "self.actions = actions"
    Cell->>Cell: "guard requiresFullApply || modelChanged || hoverChanged"
    Cell->>Cell: applyModel(model) — full apply on first configure
    Cell->>Cell: "needsLayout = true"

    Note over Cell: Second configure (same cell, reused again)
    TableView->>Cell: configure(model:actions:...)
    Cell->>Cell: "requiresFullApply = self.actions == nil (false, already set)"
    Cell->>Cell: "guard modelChanged || hoverChanged → skips if unchanged"
Loading

Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile

@cursor

cursor Bot commented Jul 25, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@lawrencecchen
lawrencecchen merged commit 265d176 into main Jul 26, 2026
6 checks passed
@lawrencecchen
lawrencecchen deleted the fix-main-ci-post-8876 branch July 26, 2026 02:53
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