Skip to content

Fix updater getting stuck preparing a check - #16664

Merged
austinywang merged 1 commit into
mainfrom
issue-16660-updater-preparing-hang
Oct 2, 2026
Merged

austinywang merged 1 commit into
mainfrom
issue-16660-updater-preparing-hang

Conversation

@austinywang

@austinywang austinywang commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

A manual update check could stay on Preparing Update Check… indefinitely when Sparkle reported repeated cycle-finished callbacks while its session was still unavailable. Each callback cancelled and restarted the same readiness poll, so the bounded timeout never necessarily elapsed.

Fix

Keep one readiness poll alive for the current request. Repeated Sparkle callbacks now reuse that poll instead of resetting its deadline. The task is cleared explicitly when it starts the check or reports the not-ready error, so retries remain available.

Closes #16660

Validation

  • swift test --package-path Packages/macOS/CmuxUpdater --filter UpdateControllerPipelineTests (18 passed)
  • git diff --check

Changelog

Fixed update checks getting stuck on “Preparing Update Check…”.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes update checks getting stuck on “Preparing Update Check…” when Sparkle reports repeated cycle-finished callbacks while its session is unavailable.

Previously, each callback cancelled and restarted the same readiness poll, so the bounded timeout never elapsed. Now a single readiness poll stays alive for the current request, and repeated callbacks reuse it instead of resetting its deadline. The poll task is cleared explicitly when the check starts or when the not-ready error is reported, so retries remain available.

Closes #16660.

Written for commit de64650. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved update-check timing by avoiding duplicate readiness checks and preserving pending checks during readiness retries.

@cursor

cursor Bot commented Oct 2, 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.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

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

@coderabbitai

coderabbitai Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

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

🧰 Additional context used
📚 Code guidelines (2)
.github/review-bot-rules/swift-architectural-rethink.md — configured
.github/review-bot-rules/source-control-artifacts.md — configured

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

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

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4102ab60-7cc8-4e95-8aff-2b16bf38bdd0

📥 Commits

Reviewing files that changed from the base of the PR and between 6529dfd and de64650.

📒 Files selected for processing (1)
  • Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The updater now reuses an active readiness task, clears its task reference when readiness succeeds or times out, and preserves the readiness retry when a cycle finishes with a pending check.

Changes

Readiness check handling

Layer / File(s) Summary
Readiness task lifecycle
Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift
waitForReadinessThenCheck returns when a readiness task is already running. The controller clears the task reference when readiness succeeds or times out. When a cycle finishes with a pending check, it no longer cancels the readiness retry before calling beginCheckWhenReady.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to de646

Cancel returns the updater to idle and a later manual check can retry. No material user-facing risk was identified in this change; it is mergeable subject to normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to de646

The change restores a bounded update-readiness wait. A cancellation-and-replacement interleaving could nevertheless leave a newer wait untracked, weakening cleanup of subsequent update requests. No new installation permission or update-policy bypass was established.

Retained concerns

  • Low · reliability · inferred: A canceled readiness task can start after a replacement request and clear the replacement task's handle before consuming shared pending intent. The new terminal clearing can consequently leave the replacement wait outside cancellation ownership, weakening failure containment and recovery for later check or install requests. Production occurrence is unconfirmed; unauthorized installation was not established.
Security review details

Security Blast Radius

  • inferred — The demonstrated exposure concerns one controller's update requests and its host application's Sparkle flow. The inspected changes do not demonstrate an expanded remote entrypoint or new privilege source.

Trust Boundaries and Controls

  • inferred — The stale-task interleaving consumes an existing later intent rather than manufacturing installation authority. Install intent originates in the accepted-install flow, while request-entry policy checks remain unchanged; no introduced authorization bypass was established.

Resilience and Maintainability Implications

  • inferred — Main-actor serialization prevents simultaneous state mutation, but does not bind a queued task to its original request. Newly clearing the shared handle can weaken cancellation ownership across replacement requests, even though the underlying stale-intent behavior predates this PR.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the primary change: fixing updater checks that become stuck while preparing.
Description check ✅ Passed The description explains the problem, fix, issue reference, validation commands, and user-facing changelog. It omits the template's Demo Video and Checklist sections, but the core information is compl…
Linked Issues check ✅ Passed For [#16660], waitForReadinessThenCheck keeps one readiness task and does not restart its deadline when Sparkle sends repeated cycle-finished callbacks. The bounded wait clears readyCheckTask, sta…
Out of Scope Changes check ✅ Passed The whole-PR diff changes only Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. The changes prevent repeated readiness-poll restarts, preserve the pending check throug…
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS: The pull request changes only Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. The diff adjusts updater readiness polling and pending checks. It does not change …
Cmux Swift Actor Isolation ✅ Passed PASS. The diff changes only readiness-task control flow in an existing @MainActor UpdateController. The new task closure is explicitly @MainActor, and all accesses to updater, readyCheckTask…
Cmux Swift Blocking Runtime ✅ Passed PASS. The PR changes the lifecycle of an existing bounded readiness poll. It adds no new sleep, timer, blocking wait, lock, or dispatch synchronization. The diff prevents repeated cancellation and res…
Cmux Browser Automation Off-Main ✅ Passed The pull request changes only Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. The diff contains updater readiness logic and one @MainActor task, but no browser.* …
Cmux Expensive Synchronous Load ✅ Passed The pull request changes only UpdateController+Checking.swift. The diff adds readiness-task reuse and state cleanup, and removes a readiness-task cancellation before retrying a check. It adds no `Re…
Cmux Cache Substitution Correctness ✅ Passed The diff changes a transient readiness Task for the updater UI flow. It does not replace an authoritative persistence, history, undo, or snapshot read with a cache, and it introduces no cold-cache or …
Cmux No Hacky Sleeps ✅ Passed PASS — The pull request changes only Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift, which is Swift. This check applies only to production changes in TypeScript, Java…
Cmux Algorithmic Complexity ✅ Passed PASS: The pull request changes only UpdateController+Checking.swift. The added readiness loop performs a fixed 20-iteration retry (readyRetryCount = 20) over no collection. The task guard prevents…
Cmux Swift Concurrency ✅ Passed The diff does not introduce or materially expand a prohibited legacy async pattern. The readiness Task already existed in the base revision. Its property remains stored and cancellable through `canc…
Cmux Swift @Concurrent ✅ Passed PASS. The changed readiness task remains explicitly Task { @MainActor ... } and runs on the @MainActor-isolated UpdateController. The diff changes task reuse, cleanup, and callback handling; it …
Cmux Swift Package Boundaries ✅ Passed The only changed file is Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. CmuxUpdater is a SwiftPM library target with a dedicated CmuxUpdaterTests target, and `Up…
Cmux Swiftpm Lockfiles ✅ Passed PASS: The authoritative PR diff contains only Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. It changes no Package.swift, Package.resolved, .gitignore, Xcode p…
Cmux Swift Logging ✅ Passed PASS: The PR changes only readiness-task control flow in UpdateController+Checking.swift. It adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger declaration, or log message. …
Cmux User-Facing Error Privacy ✅ Passed PASS. The diff changes readiness-task handling and removes a retry cancellation. It adds only a developer comment mentioning Sparkle. The existing timeout error remains generic: “Updater is still star…
Cmux Full Internationalization ✅ Passed PASS: The PR changes only readiness-task control flow and adds developer comments. It does not add or modify user-facing Swift text, localization keys, catalogs, web messages, metadata, or changelog c…
Cmux Swiftui State Layout ✅ Passed PASS — The pull request changes only UpdateController+Checking.swift, a Foundation/Sparkle controller extension. The diff adds readiness-task handling and removes a cancellation; it does not add or …
Cmux Architecture Rethink ✅ Passed PASS. The PR makes a small local correctness change in the existing UpdateController readiness lifecycle. The readiness poll, readyCheckTask, and pending intent remain owned by the same controller…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only readiness and cycle handling in UpdateController+Checking.swift. The diff adds no NSWindow, NSPanel, NSWindowController, SwiftUI Window, WindowGroup, or auxiliary-windo…
Cmux Source Artifacts ✅ Passed The pull request changes one path: Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift. The diff contains hand-written Swift source changes only. No logs, caches, generate…
Cmux No Test Or Debug Seam In Production Source ✅ Passed The only changed file is Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/UpdateController+Checking.swift under production Sources/. The diff only changes readiness-task lifecycle: it reuses `readyC…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@austinywang
austinywang merged commit 4adc8e4 into main Oct 2, 2026
63 of 64 checks passed
@austinywang
austinywang deleted the issue-16660-updater-preparing-hang branch October 2, 2026 03:45
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Merge receipt for de64650ec5, merged 2026-10-02 03:45:34 UTC

  • Not verified at merge: ci-status (not reported), macOS compile admission (in progress), swift-package-tests (in progress)
  • Verified: CI fast guards, detect-ios-changes, Fast static checks, GhosttyKit release check, guards (18), ios-tests, linux-preflight, macOS admission gate, package-conventions-lint, runner, Web complexity, web-validation
  • Skipped by policy: admission-placement, browser, Claude wrapper regressions, Dogfood build #​${{ github.event.pull_request.number }}, full-suite-coverage, ios-simulator, ios-simulator-build, mobile-core-package, remote-daemon, suite-coverage, ui-tests, web, and 3 more
  • 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
rustybret pushed a commit to rustybret/bmux that referenced this pull request Oct 2, 2026
541c735 fix(remote): reject unknown Eternal Terminal equals options (manaflow-ai#15987)
ecb963b fix(cli): reject trailing remotes list/remove arguments (manaflow-ai#15978)
17a8a94 ci: pass the frame pacing fling count as an argument (manaflow-ai#16617)
aa6f57e app sign-ins confirm the account, so sign out then sign in can pick another one (manaflow-ai#16661)
4adc8e4 Fix updater readiness wait reset loop (manaflow-ai#16664)
6f77178 Keep only Invite in Cloud sidebar header (manaflow-ai#16636)
72f2915 notify: add --desktop flag to post to the panel without a native banner (manaflow-ai#14688)
4ba0d8a Expose per-surface prompt and unread state to custom sidebars (manaflow-ai#11142)
b3da20c Allow browser drags across Cloud workspaces (manaflow-ai#16390)
6529dfd Stop retrying Cloud terminals on stale replay daemons (manaflow-ai#16327)
b10f7e2 test: create the requested cwd in the stale-reported split test (manaflow-ai#16653)
9b5b35f Fix Computer Use onboarding readiness after permissions are granted (manaflow-ai#14281)
c45da7e Merge pull request manaflow-ai#16623 from manaflow-ai/fix-ios-cloudvpn-appstore-signing
6e67724 fix: close CloudVPN profile and identity gaps
7e9d6ab fix: sign CloudVPN in App Store exports
1984d1e test: cover App Store CloudVPN signing

# Conflicts:
#	.github/workflows/cmux-next-frame-pacing.yml
#	.github/workflows/ios-app-store.yml
#	.github/workflows/ios-appstore-upload.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged-unverified A judging check was not green at merge; see the merge receipt comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Updater can remain stuck on Preparing Update Check

1 participant