Skip to content

fix: use weak var instead of weak let for macOS 26 / Swift 6 compat - #9653

Merged
teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
XueyanZhang:fix-macos26-weak-let
Oct 2, 2026
Merged

teamleaderleo merged 2 commits into
manaflow-ai:mainfrom
XueyanZhang:fix-macos26-weak-let

Conversation

@XueyanZhang

@XueyanZhang XueyanZhang commented Aug 5, 2026 •

Copy link
Copy Markdown
Contributor

Closes #9652

Summary

weak let is rejected by the Swift compiler on newer toolchains (e.g. the macOS 26 SDK): a weak reference can become nil at runtime when the referenced object is deallocated, so its storage must be mutable. The compiler error is

error: 'weak' must be a mutable variable, because it may change at runtime

CI runs on macOS 15.7.4 where these still compile, so this is latent there — it only surfaces on macOS 26, where it blocks the test-target compile (e.g. cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift:4922 fails first and aborts the whole cmux-unit build).

Changes

Replaces weak let with weak var at the four remaining sites. None of the variables are reassigned, so the change is purely a compile-time correctness fix with no behavioral difference:

File Variable
cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift weakRemovablePanel
Packages/macOS/CmuxCommandPalette/Tests/.../CommandPaletteInteractionMonitorTests.swift weakMonitor
Packages/macOS/CmuxSimulator/Tests/.../SimulatorWorkerClientContainmentTests.swift weakClient
Packages/iOS/CmuxMobileShell/Tests/.../MobileMacConnectionPoolTests.swift weakShell

The fifth site originally reported (Packages/iOS/CmuxMobileSupport/Sources/.../ComposerDictationController.swift) was already fixed on main (now weak var), so this PR covers the remainder.

Why no regression test

Compile-time fix only. On CI's macOS 15.7.4 the code already compiles, so there is no observable runtime behavior to assert; a "test" would just duplicate a compile that already passes. Per the project's test-quality policy, no fake regression test is added.

Test plan

  • swift build --build-tests for CmuxCommandPalette on macOS 26 — its test target (containing one of the weak var sites) compiles clean.
  • Mechanical correctness: every site is a direct weak let → weak var, the exact form the compiler error mandates; none of the variables are reassigned.
  • CI (macOS 15.7.4) — confirms no regression; the change is a no-op there (macOS 15 still accepts weak let).

Summary by CodeRabbit

  • Tests
    • Corrected weak-reference test declarations across iOS and macOS test suites.
    • Improved reliability of deallocation and cleanup assertions for shells, monitors, simulator clients, and panels.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 6aad80ff-c75b-4384-91d1-e9874f000595

📥 Commits

Reviewing files that changed from the base of the PR and between a7fb822 and 8ef4f92.

📒 Files selected for processing (1)
  • Packages/iOS/CmuxMobileShell/Tests/CmuxMobileShellTests/MobileMacConnectionPoolTests.swift
 ________________________________________________
< Love the optimism of `// should never happen`. >
 ------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       /   づ
📝 Walkthrough

Walkthrough

Four test weak-reference bindings changed from weak let to weak var. Existing deallocation and observer-cleanup assertions remain unchanged.

Changes

Weak reference binding fixes

Layer / File(s) Summary
Update weak reference declarations
Packages/iOS/CmuxMobileShell/Tests/..., Packages/macOS/CmuxCommandPalette/Tests/..., Packages/macOS/CmuxSimulator/Tests/..., cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift
Four test weak references now use mutable var bindings. Existing assertions remain unchanged.

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

Possibly related PRs

Suggested reviewers: austinywang, lawrencecchen

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR replaces four reported weak let declarations, and the fifth linked issue location is documented as already fixed on main.
Out of Scope Changes check ✅ Passed All four changes directly implement the linked issue and contain no unrelated code or behavior changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed The commit changes only four Swift test files from weak let to weak var; no production Swift or actor-isolation declaration changed.
Cmux Swift Blocking Runtime ✅ Passed The commit changes only four test files, each by replacing weak let with weak var; it adds no blocking or timing synchronization, and the production source is unchanged.
Cmux Browser Automation Off-Main ✅ Passed HEAD^..HEAD changes only four test files, each replacing weak let with weak var; no browser automation, WebKit, socket-worker, or policy-routing code changed.
Cmux Expensive Synchronous Load ✅ Passed HEAD-parent diff changes only four test files, each solely from weak let to weak var; no production Swift or expensive synchronous load/path change is present.
Cmux Cache Substitution Correctness ✅ Passed The commit only changes four test declarations from weak let to weak var; no production cache, persistence, history, undo, or snapshot read changed.
Cmux No Hacky Sleeps ✅ Passed The actual diff changes only four Swift test files from weak let to weak var; the rule covers non-Swift runtime changes, and no timing constructs were added.
Cmux Algorithmic Complexity ✅ Passed The HEAD diff changes only four test files, each by one weak let→weak var declaration; no collection scans, sorting, filtering, joins, or runtime algorithm changes were added.
Cmux Swift Concurrency ✅ Passed The diff only changes four existing weak let declarations to weak var; it adds no Dispatch, Combine, completion-handler, or fire-and-forget Task patterns.
Cmux Swift @Concurrent ✅ Passed The diff only changes four weak let declarations to weak var; it adds no @concurrent/nonisolated/async or call-site changes, so the concurrency rule is not violated.
Cmux Swift Package Boundaries ✅ Passed The commit changes only four test files from weak let to weak var; ComposerDictationController is unchanged and already uses weak var, so no production boundary violation exists.
Cmux Swiftpm Lockfiles ✅ Passed The PR diff contains only four Swift test-file weak-reference edits; it changes no Package.swift, Package.resolved, .gitignore, workflow, or Xcode package reference covered by the rule.
Cmux Swift Logging ✅ Passed The patch only changes four test declarations from weak let to weak var; it adds no logging and does not modify production Swift code.
Cmux User-Facing Error Privacy ✅ Passed The HEAD diff only changes four weak let declarations to weak var in test files; it adds no user-facing errors, alerts, command output, API bodies, or recovery copy.
Cmux Full Internationalization ✅ Passed The HEAD diff changes only four weak references in test files; it adds no user-facing text, localization keys, catalogs, web messages, or locale data.
Cmux Swiftui State Layout ✅ Passed The commit changes only four weak let declarations to weak var; it adds no SwiftUI state, layout, lazy-row store, or render-time mutation patterns.
Cmux Architecture Rethink ✅ Passed The diff only changes four test-local weak bindings from let to var; it adds no timing, polling, locks, observers, side channels, duplicate wiring, or lifecycle-owner changes.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff changes only four test files and four weak declarations; it adds no NSWindow, NSPanel, WindowGroup, identifier, or close-shortcut code.
Cmux Source Artifacts ✅ Passed All four changed paths are intentional Swift test files; the diff only changes weak let to weak var and adds no logs, caches, build output, temp folders, or other artifacts.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR commit changes only four test files; no production Sources path is modified, and added lines contain only weak var replacements with no debug/test seam.
Cmux No Ambient Global State ✅ Passed The HEAD diff changes only four local weak bindings in test functions; it adds no production Swift file, top-level mutable state, namespace type, or singleton.
Title check ✅ Passed The title clearly identifies the weak-reference declaration fix and its macOS 26 and Swift 6 compatibility purpose.
Description check ✅ Passed The description explains the cause, affected files, testing, and rationale for omitting a regression test; non-applicable template sections are reasonably omitted.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

A weak reference can become nil at runtime when the referenced object is
deallocated, so its storage must be mutable. Newer Swift toolchains (e.g.
the macOS 26 SDK) enforce this and reject weak let:

  error: 'weak' must be a mutable variable, because it may change at runtime

CI's macOS 15.7.4 still accepts weak let, so this is latent there; on
macOS 26 it aborts the test-target compile (e.g.
cmuxTests/AppDelegateEqualizeSplitsShortcutTests.swift:4922).

Replaces weak let with weak var at all five sites. None are reassigned, so
this is a compile-time correctness fix with no behavioral change. No
regression test: the difference is only observable on a newer SDK than
CI's, where the code already compiles.
@XueyanZhang
XueyanZhang force-pushed the fix-macos26-weak-let branch from 2ce8a8f to a7fb822 Compare August 6, 2026 17:58
@cursor

cursor Bot commented Aug 6, 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.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@XueyanZhang

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main. Note: the fifth site (ComposerDictationController.swift) was already fixed upstream, so this covers the remaining four weak let sites. @lawrencecchen could you take a look?

@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thanks for this! We checked the weak var change against current main and would like to land it. The one thing left is the CLA (CLA.md): if you're OK with it, comment exactly I have read the CLA Document v2.2 and I hereby sign the CLA and we'll take it from there.

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@XueyanZhang

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document v2.2 and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Sep 28, 2026
@XueyanZhang

Copy link
Copy Markdown
Contributor Author

Thank! Sorry there was a delay XD

@teamleaderleo teamleaderleo added bug Something isn't working S2: major A crash, hang, lost state, broken connection, or a regression on a path people use area: build-and-ci Build system, CI workflows, test infrastructure ready-to-land Reviewed and ready to land when CI is green labels Sep 30, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Review: The four weak-reference declarations are the required Swift 6 compile fix and preserve test intent. Fixed: no additional changes needed. Left: required CI is still blocked/pending.

@teamleaderleo
teamleaderleo enabled auto-merge (squash) September 30, 2026 16:52
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Taking this: updating the reviewed weak-reference compile fix with main and checking the remaining package test sites.

  • OrchardSpoon g1 🌀

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@teamleaderleo
teamleaderleo merged commit 59821f4 into manaflow-ai:main Oct 2, 2026
7 of 8 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for 8ef4f92056, merged 2026-10-02 08:55:59 UTC

  • Not verified at merge: ci-status (not reported)
  • Verified: Web complexity
  • Full suite: runs on main after merge.

Labeled merged-unverified: if main breaks near this merge, look here first.

@github-actions github-actions Bot added the merged-unverified A judging check was not green at merge; see the merge receipt comment label Oct 2, 2026
@teamleaderleo

Copy link
Copy Markdown
Collaborator

Thanks @XueyanZhang. I updated this with main. Three package lifetime tests still need your weak var fix; the app-host test is already fixed on main. Both independent reviews are clean, and I am checking the current CI.

  • OrchardSpoon g1 🌀

rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 2, 2026
644fd5e Remove inline Open in cmux action from port rows (manaflow-ai#16350)
59821f4 fix: use weak var instead of weak let for macOS 26 / Swift 6 compat (manaflow-ai#9653)
e7a4e0a fix(cmux-tui): satisfy reconnect clippy lint (manaflow-ai#16758)
ee61823 fix(cloud): name the first machine workspace workspace-1 (manaflow-ai#16754)
8f28c09 test: isolate fake-socket CLI tests from the launching cmux shell (manaflow-ai#16562)
22d59ac Fix Return key for machine deletion confirmation (manaflow-ai#16683)
5049234 Cloud: 5 VMs per seat (4 vCPU/8 GB), Max 16 vCPU/32 GB, no free machines (manaflow-ai#16207)
13d77d6 fix: make dashboard team switching finish before refresh (manaflow-ai#16680)
3952ab3 Fix initial Cloud workspace layout restore (manaflow-ai#16690)
4ac2ec4 Fix optimistic selection for Cloud workspace creation (manaflow-ai#16672)
b34697f Remove Cloud agent star button (manaflow-ai#16700)

# Conflicts:
#	.github/workflows/ci-guards.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: build-and-ci Build system, CI workflows, test infrastructure bug Something isn't working merged-unverified A judging check was not green at merge; see the merge receipt comment ready-to-land Reviewed and ready to land when CI is green S2: major A crash, hang, lost state, broken connection, or a regression on a path people use

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test targets fail to compile on macOS 26 / newer Swift: weak let rejected

2 participants