Treat "no update available" as a success in Attempt Update by austinywang · Pull Request #9435 · manaflow-ai/cmux

austinywang · 2026-08-03T04:34:28Z

Running "Attempt Update" from the command palette while already on the latest version showed a red pill: "Update Didn't Start — cmux couldn't start the update. Check your internet connection and try again." The check had succeeded and the network was fine; there was simply nothing to install.

Mechanism

AttemptUpdateCoordinator.handleStateChange classified .notFound as .installFailed in the .awaitingResult phase. The controller then ran setInstallDidNotStartError, whose final model.replaceActiveState(with: errorState) overwrote the model's correct .notFound state with the error, so the user never saw the true result of their own check.

.notFound in .awaitingResult now stops monitoring and returns .none, leaving the model's .notFound to render as the normal up-to-date result. This also makes the coordinator agree with InstallWatchdog, which already classified .notFound as resolved and non-stalled.

Deliberately unchanged: .idle in the same phase still returns .installFailed (the attempt ended with no result at all), and .notFound in .awaitingFreshCheck / .startingDownload still fails, since a notFound after the user accepted an install is genuinely unexpected. The issue's wider question about whether the three palette update commands should stay distinct is out of scope here.

Testing

Regression tests, split so CI goes red then green:

Commit 1 updates stopsMonitoringWhenFreshCheckFindsNothing to assert .none (it previously asserted .installFailed, locking the bug in) and adds reportsInstallFailedWhenFreshCheckEndsAtIdle to pin the .idle behavior that is intentionally kept.
Commit 2 adds UpdatePillUITests.testAttemptUpdateWithNoUpdateAvailableDoesNotShowError, which drives the real command palette entry point and asserts the "No Updates Available" pill appears instead of the error.
Commit 3 is the fix, plus a retarget of acceptedInstallFailureAcknowledgesReplacedSparkleTerminal so it exercises the accepted-install path (.startingDownload) that still fails, rather than the up-to-date path that no longer does.

Commands:

swift test in Packages/macOS/CmuxUpdater: 87 tests pass at the fix commit. At the pre-fix source, stopsMonitoringWhenFreshCheckFindsNothing and a new pipeline-level replay (upToDateAttemptShowsNoUpdateAvailableInsteadOfError) both fail, confirming they capture the bug.
Hosted test-e2e.yml run of UpdatePillUITests/testAttemptUpdateWithNoUpdateAvailableDoesNotShowError, dispatched at both the pre-fix commit and the fix head.

Dev build: ./scripts/reload.sh --tag sym9262 succeeded and the tagged app launched (the cloud builder failed on an unrelated stale zig toolchain). Driving the palette from the debug socket was not possible in this environment because the tagged window never became key, so the palette entry point is covered by the hosted XCUITest above rather than by a local socket transcript.

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

Summary by cubic

Treat “no update available” as a successful outcome for “Attempt Update,” so users see “No Updates Available” instead of the red “Update Didn’t Start” error. Fixes #9262.

Bug Fixes
- In the coordinator, .notFound during the fresh check now ends the attempt quietly (stop monitoring, return .none) and keeps the model’s .notFound state; .idle and unexpected .notFound in earlier phases still fail.
- Added regression coverage with unit tests and a UI test to assert the up‑to‑date pill appears and no error pill is shown.

^{Written for commit ea628a8. Summary will update on new commits.}

Summary by CodeRabbit

Bug Fixes
- Improved update checks when no newer version is available.
- “No Updates Available” is now shown without incorrectly displaying an update failure message.
- Update attempts that are already current now complete normally.
- Preserved error handling when an update check ends unexpectedly while idle.
Tests
- Added regression coverage for update prompts, terminal responses, and the command-palette update flow.

…port install failure

… show an error pill

Fixes the red "Update Didn't Start / check your internet connection" pill shown when Attempt Update runs while already on the latest version.

coderabbitai · 2026-08-03T04:34:59Z

📝 Walkthrough

Walkthrough

The updater now treats .notFound during a fresh update attempt as a successful up-to-date result. Coordinator, pipeline, and UI tests verify that the normal no-update state remains visible and that .idle still reports failure.

Changes

Up-to-date update attempt

Layer / File(s)	Summary
Coordinator terminal-state handling `Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/AttemptUpdateCoordinator.swift`, `Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/AttemptUpdateCoordinatorTests.swift`	Fresh checks that reach `.notFound` stop monitoring and return `.none`. Fresh checks that end at `.idle` still return `.installFailed`.
Pipeline and UI regression coverage `Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift`, `cmuxUITests/UpdatePillUITests.swift`	Tests verify accepted update attempts preserve `.notFound` and that the command-palette flow shows “No Updates Available” without “Update Didn’t Start”.

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

Possibly related PRs

manaflow-ai/cmux#9346: Continues the .notFound handling fix and related regression coverage.

🚥 Pre-merge checks | ✅ 25

✅ Passed checks (25 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: treating no available update as a successful Attempt Update outcome.
Description check	✅ Passed	The description provides a detailed summary, rationale, implementation details, testing evidence, and explicit scope boundaries.
Linked Issues check	✅ Passed	The changes satisfy issue `#9262` by preserving .notFound, stopping monitoring, showing the normal result, and retaining failures for unexpected states.
Out of Scope Changes check	✅ Passed	All code and test changes directly support issue `#9262` and the stated Attempt Update behavior.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation	✅ Passed	The production diff only changes `.awaitingResult` state handling; existing `@MainActor` on `AttemptUpdateCoordinator` is unchanged, and all other changes are tests.
Cmux Swift Blocking Runtime	✅ Passed	Production diff only changes AttemptUpdateCoordinator state handling; it adds no blocking or timing primitive. Task.yield and waitForExistence occur only in regression tests.
Cmux Browser Automation Off-Main	✅ Passed	The PR changes only updater logic and update tests; it does not modify browser socket automation targets or add browser.* commands, waits, WebKit/AppKit worker code, or policy tests.
Cmux Expensive Synchronous Load	✅ Passed	The only production Swift change edits `@MainActor` coordinator state handling; it adds no agent-history load, filesystem scan, JSON parsing, or interactive synchronous loader call.
Cmux Cache Substitution Correctness	✅ Passed	The production diff only changes AttemptUpdateCoordinator state handling; it does not replace any fresh persistence, history, undo, or snapshot read with a cache.
Cmux No Hacky Sleeps	✅ Passed	The patch changes only two Swift files; it introduces no covered TypeScript, JavaScript, shell, or build/runtime delay. Added waits are Swift test scaffolding.
Cmux Algorithmic Complexity	✅ Passed	The production diff only adds a constant-time enum switch branch in AttemptUpdateCoordinator; it adds no collection iteration, sorting, filtering, joins, or rescans.
Cmux Swift Concurrency	✅ Passed	The PR adds no legacy async pattern. Production changes are synchronous state handling; tests use awaited Task.yield and XCTest callbacks, both allowed boundaries.
Cmux Swift `@Concurrent`	✅ Passed	The diff changes only synchronous `@MainActor` coordinator logic; added async code is actor-bound test polling with Task.yield, with no `@concurrent/nonisolated` async or heavy helper call.
Cmux Swift Package Boundaries	✅ Passed	The production change is in Packages/macOS/CmuxUpdater/Sources/CmuxUpdater, behind the CmuxUpdater SwiftPM target; its tests use CmuxUpdaterTests. No app-target domain logic was added.
Cmux Swiftpm Lockfiles	✅ Passed	PR changes only updater source/tests and UI tests; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode package-reference files changed.
Cmux Swift Logging	✅ Passed	The PR adds no Swift logging statements or logger declarations; production code only changes state handling, and added output appears only in tests/UI assertions.
Cmux User-Facing Error Privacy	✅ Passed	The production diff only changes update-state handling and adds no user-facing error or diagnostic text; vendor and environment-name references occur only in tests/comments.
Cmux Full Internationalization	✅ Passed	Production diff changes only coordinator state handling and comments; no user-facing text or locale files were added. Existing `.notFound` output uses `String(localized:defaultValue:)`, and added U...
Cmux Swiftui State Layout	✅ Passed	The PR changes updater coordination and XCTest/Testing coverage only; it adds no SwiftUI views, state wrappers, GeometryReader, lazy rows, or render-time state writes.
Cmux Architecture Rethink	✅ Passed	The PR makes a small local state-transition fix in AttemptUpdateCoordinator: `.notFound` ends monitoring and preserves the model state; it adds no timing, flags, observers, side channels, or duplic...
Cmux Swift Auxiliary Window Close Shortcuts	✅ Passed	PR changes updater logic and tests only; no standalone NSWindow, NSPanel, NSWindowController, Window, or WindowGroup code was added or changed. Auxiliary-window lint passes.
Cmux Source Artifacts	✅ Passed	All six changed paths are hand-written Swift source or tests; the diff adds no logs, media files, caches, build output, temp folders, or artifact directories.
Cmux No Test Or Debug Seam In Production Source	✅ Passed	The only production diff changes `.notFound` and `.idle` handling in AttemptUpdateCoordinator; it adds no DEBUG guard, test/debug member, visibility widening, or wrapper accessor.
Cmux No Ambient Global State	✅ Passed	The production diff only changes handleStateChange in AttemptUpdateCoordinator; it adds no top-level function, mutable global, static namespace, or singleton.

✨ 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-9262-attempt-update-shows-update-didn-t-start

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.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift`:
- Around line 417-418: Replace the fixed Task.yield loop in
UpdateControllerPipelineTests with a deterministic completion signal or bounded
predicate confirming attempt handling is complete, such as inactive coordination
and a disarmed watchdog, before asserting the absent state. In
cmuxUITests/UpdatePillUITests.swift lines 205-208, wait for a deterministic
UI-test signal that the command-palette attempt reached its terminal state
before asserting error absence; use fulfillment from the triggering event rather
than scheduler yields.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 5c49d9b9-55e6-417e-b731-2038bd1e1a85

📥 Commits

Reviewing files that changed from the base of the PR and between 84f5755 and ea628a8.

📒 Files selected for processing (4)

Packages/macOS/CmuxUpdater/Sources/CmuxUpdater/AttemptUpdateCoordinator.swift
Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/AttemptUpdateCoordinatorTests.swift
Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift
cmuxUITests/UpdatePillUITests.swift

coderabbitai · 2026-08-03T04:36:51Z

+        // Give any erroneous reaction a chance to replace the state.
+        for _ in 0..<2_000 { await Task.yield() }


🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Wait for deterministic terminal handling before asserting the regression result.

The fixed yield loop and immediate absence check do not prove that queued .notFound handling is complete. Both tests can pass before the prior erroneous reaction replaces the normal result.

Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift#L417-L418: Replace the fixed Task.yield() loop with a completion signal or a bounded predicate for completed attempt handling, such as inactive coordination and a disarmed watchdog.

cmuxUITests/UpdatePillUITests.swift#L205-L208: Assert error absence only after a deterministic UI-test signal confirms that the command-palette attempt reached its terminal state.

As per coding guidelines, “For scheduler or asynchronous ordering, fulfill continuations or expectations from the triggering event.”

📍 Affects 2 files

Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift#L417-L418 (this comment)

cmuxUITests/UpdatePillUITests.swift#L205-L208

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Packages/macOS/CmuxUpdater/Tests/CmuxUpdaterTests/UpdateControllerPipelineTests.swift` around lines 417 - 418, Replace the fixed Task.yield loop in UpdateControllerPipelineTests with a deterministic completion signal or bounded predicate confirming attempt handling is complete, such as inactive coordination and a disarmed watchdog, before asserting the absent state. In cmuxUITests/UpdatePillUITests.swift lines 205-208, wait for a deterministic UI-test signal that the command-palette attempt reached its terminal state before asserting error absence; use fulfillment from the triggering event rather than scheduler yields.

Source: Coding guidelines

… update An accepted install attempt (Attempt Update palette command, Install and Relaunch buttons) re-resolves the feed with a fresh check. #9435 stopped the fresh check's own "no update" result from becoming the misleading "Update Didn't Start / check your internet connection" error, but two paths of the same class still raised it on an up-to-date machine: 1. AttemptUpdateCoordinator.awaitingFreshCheck treated a notFound from a still-finishing prior session as installFailed. Now it ends the attempt quietly: a completed check saying "no update" is a truthful, visible outcome regardless of which session produced it. The install watchdog still bounds genuinely stalled attempts. 2. updateDriverDidFinishCycle raised the error over a .notFound state observed while the coordinator had not yet drained that transition. notFound now counts as a visible terminal there, like .error. After an install is actually accepted (startingDownload phase), notFound still fails the attempt; nothing changes for real stalls. Fixes the remaining variants of #9262. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

austinywang added 3 commits August 2, 2026 21:03

Add failing test: Attempt Update with no update available must not re…

edcb5cb

…port install failure

Add failing UI test: Attempt Update with no update available must not…

78671dd

… show an error pill

Treat 'no update available' as a success in Attempt Update

ea628a8

Fixes the red "Update Didn't Start / check your internet connection" pill shown when Attempt Update runs while already on the latest version.

coderabbitai Bot reviewed Aug 3, 2026

View reviewed changes

austinywang merged commit 8cc5dc4 into main Aug 3, 2026
8 checks passed

austinywang deleted the issue-9262-attempt-update-shows-update-didn-t-start branch August 3, 2026 21:11

azooz2003-bit mentioned this pull request Sep 1, 2026

updater: keep "You're up to date" when an install attempt resolves no update #11318

Closed

Navigation Menu

Repository navigation

Treat "no update available" as a success in Attempt Update - #9435

Treat "no update available" as a success in Attempt Update#9435
austinywang merged 3 commits into
mainfrom
issue-9262-attempt-update-shows-update-didn-t-start

austinywang commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Aug 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		// Give any erroneous reaction a chance to replace the state.
		for _ in 0..<2_000 { await Task.yield() }

Conversation

austinywang commented Aug 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mechanism

Testing

Summary by cubic

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

austinywang commented Aug 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Aug 3, 2026 •

edited

Loading