Repository navigation
Self upgrade mechanism - #152
Merged
Merged
Conversation
The app ships as a cask, so its own outdated state is ordinary inventory data: SelfUpdateStatus carries the cask's `outdated` flag verbatim rather than comparing version strings, which nothing here knows how to do. `brew upgrade --cask homebrew-app` is a BrewCommand value under a new `upgradeApp` operation kind, not a command type. PackageOperationSubject excludes it — the app's own cask is in no inventory a package surface observes. The three ports (status, preferences, handoff) are protocols in BrewRepositoryInterfaces so the coordinator can be tested without a bundle, a defaults suite or a subprocess. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`brew info --installed --json=v2` already reports the app's cask with an `outdated` flag, so detection is a lookup in data the app fetches anyway rather than a second brew call on a timer. The running version comes through RunningAppVersionReading: reading Bundle.main directly would resolve to the test runner under test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
Dismissal is stored per version, so "Later" defers one release rather than muting the banner for good. Both take a defaultsKeyPrefix fed through the same seam as the caches: unprefixed, a UI test pressing Later would write into the real app's preferences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
An app cannot replace its own bundle while running, so the upgrade is a handoff: HelperSelfUpdateHandoff launches HomebrewUpdateHelper from Contents/Helpers and then terminates, in that order, because the helper polls for the pid to disappear. The helper waits for exit, runs `brew upgrade --cask homebrew-app`, and reopens the app. Everything the helper needs travels in a JSON spec, by path rather than argv: a UI-test relaunch environment carries a whole fixture tree and would blow ARG_MAX. BrewSelfUpdateContract is dependency-free on purpose, so nothing in the app's module graph can leave the helper unable to bring the app back. `brew` is resolved by the app, not guessed by the helper — the app already knows which one it has been talking to, and a missing brew fails the handoff before the quit rather than after. SelfUpdateUpgradeRunner and the transcript log live in BrewSelfUpdateHelperCore so they can be unit-tested; the helper itself is a command-line target with no test host. The transcript goes to a file because the app is not running while the upgrade is, so stderr goes nowhere anyone will look. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The banner belongs to no existing feature: it sits above the Installed, Upgrades and Discover lists, which two different modules own, so it gets its own — BrewFeatureSelfUpdate, which deliberately depends on neither. The whole flow runs from the banner. Upgrade and Later are on it, so there is no detail pane to open, nothing competing with the package selection for the detail column, and no arbitration between "showing a package" and "showing the app". SelfUpdateCoordinator is the one object it binds to, composing detection, preferences and the handoff. The banner reads \.selfUpdateCoordinator from the environment itself rather than being handed it, so placing it is one line and no feature threads a coordinator down through its columns. The Upgrade button's tooltip is the brew command it will run: the app does not hide what Homebrew is about to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
…ists Pinned above each list rather than inside it, so it survives the All/Formulae/Casks scopes and search — and Discover, where someone browsing for packages is just as well placed to take the upgrade. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
BrewApp builds the coordinator and injects it, and consumes the helper's notice before the caches are built — makeCatalogueCache sweeps every UITesting.-prefixed default, which under -uiTesting includes the notice the helper just wrote. A failed upgrade relaunches an app that looks identical to an ordinary start, so it gets its own alert rather than silence. A dev build never has its own cask outdated, so the DEBUG menu can simulate the update, a newer version to re-show a dismissed banner, and skipping the real `brew upgrade --cask` while still quitting, handing off and relaunching for real. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The banner is asserted on all three lists, and Later is checked to hold across a tab change — dismissal is the app's state, not a list's. The upgrade tests are not simulations of the handshake: the app really terminates, the real helper waits for the pid, and the app it brings back is a different process. selfUpdateRunsBrew and selfUpdateBrewFails run the real subprocess against the fake brew, so which outcome alert comes back is decided by brew's exit code rather than by a flag. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
Including what was built and removed: the detail pane, the selection arbitration it needed, and the shared detail components extracted to serve it — two buttons on the banner deleted all of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
AnimatedSplitView gave the bottom pane whatever height it asked for and handed the remainder to the top, so a short window left the feature column a sliver — with the console up on Upgrades the list vanished under it entirely, and there is no scroll view there to rescue it. The bottom pane is the accessory, so it is the one that gives way: fittedSplitBottomHeight hands it only what is left above mainPaneMinHeight, and it eats into that only once it has nothing left to give. So the console opens at whatever fits — 150 in a short window, its full 250 from 777 up — and growing the window back restores the height that was asked for, since the fitting happens per layout rather than being stored. Both floors are measured, not guessed. Upgrades has the tallest chrome and none of it scrolls: self-update banner 118 + header 226 + scope picker 49 = 393, against 54pt rows. mainPaneMinHeight is 520 — the chrome plus two rows — and UpgradesChromeBudgetTests measures the real views through NSHostingView so a header that outgrows the budget fails there rather than clipping in the app. The window minimum follows: 520 + the console's 150 floor + 7pt of split chrome = 677, rounded to 680. Sizing it against the console's default instead would have demanded 800, a window nearly as tall as a 13" laptop's screen, for the sake of a pane happy to open smaller. The window itself never resizes in response to the console. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
CONVENTIONS.md asks that inline `//` explain why, not what, and enough of this branch's comments had drifted the other way to be worth a pass. The recurring shape was a doc comment whose first sentence repeated the declaration before reaching its point: "Runs `brew upgrade --cask homebrew-app` on behalf of the app", "Appends the helper's transcript to a file", "Composes detection, preferences and the handoff". Where the sentence after it carried the reason, that one stays and the opener goes. The rest were duplicated rationale — the same "empty in production" note on both the spec field and the property feeding it, dismissal-per-version explained on the protocol and again on the view model — or trivia the signature already gives, like "Called by the update helper". Package.swift loses the target comments this branch added. The swift-subprocess justification stays: CONVENTIONS.md requires it, and the two older target notes are not this branch's to remove. What survives is what a reader cannot recover from the code: why the drain starts before the wait, why a signalled child becomes 128 + signal, why Latin-1 is the fallback encoding, why the helper is launched before the quit, why the spec travels by path rather than argv, and how BrewLayout's 680 and 520 were measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`swiftformat --lint` is the first step of the Swift quality workflow and it fails the whole job on this one line, so nothing else in the branch gets checked until it goes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`selfUpgradeCommand()` went onto `BrewMutatingCommandFactory` with three implementations, and `BrewOperationID.selfUpgrade` alongside it, on the assumption the self-upgrade would be submitted like any other command. It isn't: `HelperSelfUpdateHandoff` puts `BrewCommands.selfUpgrade().arguments` straight into the handoff spec and the helper runs it after the app has gone. So the factory method has three definitions and no call site, and the operation id is only ever matched, never constructed — which also made the `CommandJob` and `PackageOperationSubject` arms for it dead. `BrewCommands.selfUpgrade()` and `BrewOperationKind.upgradeApp` stay; the handoff uses both. Periphery is baseline-gated on this repo, so newly unused declarations are exactly what it is meant to stop. The `PackageOperationSubject` comment goes with the case, and it was wrong anyway: the app's own cask is very much in the inventory those surfaces observe. Keeping it out of them is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The banner is the safe way to upgrade the app: quit, replace the bundle, relaunch. Nothing was stopping the unsafe way. `homebrew-app` is an installed cask like any other, so it sat in the Installed and Upgrades lists with its own Upgrade button, one click from a `brew upgrade --cask homebrew-app` that replaces the bundle underneath the running app. The scenario fixture shows it: the branch's own UI test data puts it in the list. "Upgrade All" was worse, because it needs no aim — `brew upgrade` and `brew upgrade --cask` name no packages, so both sweep it in. `InstalledInventoryObserving.userManagedPackages` is the filter, and `outdatedPackages`/`outdatedCount` now go through it, which carries the sidebar badge and the Upgrades subtitle with them. `state` deliberately stays unfiltered: it is where `BrewSelfUpdateStatusProvider` finds the cask, so filtering there would leave the banner blind. The batch needs a guard of its own, since a filtered list cannot un-name what a bare `brew upgrade` covers. `upgradeSelection` falls back to `.explicit(rows)` when the app's cask is outdated and the scoped selection would cover it, reusing `BrewUpgradeSelection.covers` rather than adding a predicate. `--formula` is left alone; it cannot reach a cask. `upgradeAll()` grows an empty-rows guard because that fallback makes `.explicit([])` reachable, and it renders as a bare `brew upgrade` — upgrading everything, which is the opposite of what an empty list means. The header hides the button in that state today, so this is insurance, but it is the kind worth having. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The helper polls for the app's pid to disappear and gives up after 30s. Giving up meant recording a failure and relaunching anyway — but a timeout says the app is *still running*, so the relaunch created a second instance of it (`createsNewApplicationInstance` guarantees that), and the failure notice waited for a launch that had already happened. Two copies of the app driving brew is a worse outcome than the stalled upgrade that got us there. It now logs and exits, leaving the running app alone; the user can press Upgrade again. `NSApplication.terminate` returning without terminating is the way in. It is unlikely here — no delegate cancels it — but it is the reachable case, and it is not the only one: anything that outlives the wait lands in the same arm. The ordering was untestable, sitting in an Xcode command-line target with no test host, which is why the flaw survived. `SelfUpdateHelperRun` moves the sequence into the package target behind four injected effects plus a clock, matching how `SelfUpdateUpgradeRunner` already earns its place there, and `UpdateHelper` becomes the adapter supplying the real AppKit and `UserDefaults` calls. The tests drive a virtual clock through the run's own `sleep`, so the 30s wait costs nothing to assert on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`isBannerVisible` was `isUpdateAvailable && dismissedVersion != latestVersion`. With no latest version that reads `nil != nil`, which is false, so an update Homebrew flags as outdated without a version string showed no banner at all — indistinguishable from one the user had already dismissed. It is reachable: `BrewSelfUpdateStatusProvider` maps an empty `cask.latestVersion` to nil while passing `outdated` through verbatim, so the two are independent. `SelfUpdatePresentation` already carries "Upgrade the Homebrew app" for precisely that state, and it could never be reached on a first showing. Copy with no path to it is the tell that the guard was accidental rather than intended. Both sides are now explicit. Dismissing a version-less update sets a flag on the coordinator instead of storing nil, because nil is also "never dismissed" and the two must not collapse into each other. That dismissal lasts the session only — there is no version to key a stored one to, and persisting it under some sentinel would hide the banner for every later version-less update as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
"A new version of Homebrew is available" and "Homebrew is up to date" both read as Homebrew itself in an app whose whole job is reporting on Homebrew — and which shows brew's own updates elsewhere. The banner and the alerts are about the app updating itself, so they say so. `SelfUpdatePresentation` was also the only string-producing view model in the tree still returning bare literals. Its siblings — `UpgradesViewModel`, `InstalledViewModel`, `DiscoverViewModel` — all use `String(localized:comment:)`, as does this feature's own `SelfUpdateHelperUnavailable`. Because these reach `Text` as `String` rather than as a literal, they would not have been picked up later either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`.axid(.selfUpdateOutcomeAlert)` sat at the end of `MainWindowView`'s body, which decorates the whole `NavigationSplitView` rather than either alert — modifiers there apply to the composed view, and an alert is presented in its own window. So the identifier existed on every launch, and `testUpgradingQuitsRelaunchesAndAcknowledgesOnTheNextLaunch` waited for something that was already there. It passed whether or not the app came back reporting anything, which for the branch's only test of the simulated path is the wrong thing to be sure of. The identifier moves onto the alerts' shared OK button, where it lands in the alert's own element tree, and is renamed to say so. The test now asserts the success title like its two siblings, then taps acknowledge and waits for the alert to go — which is the "acknowledges" its name has been claiming. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The Debug menu's "Skip the Real Upgrade" toggle defaulted to off, so the first thing a developer does after flipping "Simulate Homebrew Update Available" — press Upgrade — ran a real `brew upgrade --cask homebrew-app` against whatever is installed in /Applications, then relaunched the DerivedData build. The toggle's own comment says a dev build has no business doing that, which is the argument for it being on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`MainWindowView` took the coordinator as an init parameter *and* `BrewApp` put the same object in the environment for `SelfUpdateBanner` to find. Two routes to one object, and the parameter existed only because the outcome alerts happen to hang off this view. They can read the environment key like the banner does, which also spares the preview its explicit `nil`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`BrewApp` references nothing from it — the installed repository it wires up is `BrewRepositories`. The app target links the module anyway, so the import compiled and said nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The comment describes a filter that lives in `runningAppUnderTest()`, but sat on the bundle identifier above it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The transcript went to `~/Library/Logs/Homebrew/self-update.log`, chosen so it would sit alongside Homebrew's own logs. That directory belongs to the `brew` CLI, and main has since settled the general rule: the app is unsandboxed, `~/Library` is shared ground, and every root it writes to is namespaced under its bundle identifier — which is exactly the collision being avoided. `Caches` and `Application Support` already follow it; this was the one root that did not. The rule now says so in `.ai/memory.md` rather than being inferable only from the two stores that happened to implement it, and it is stated as `<root>/sh.brew.app/…` for any root added later. A log left behind at the old path is orphaned rather than migrated. It is a diagnostic transcript truncated on every run, so there is nothing in it worth carrying across. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
MikeMcQuaid
approved these changes
Sep 8, 2026
graeme
force-pushed
the
self-upgrade-mechanism
branch
from
September 8, 2026 09:01
e10c5ee to
4a6adfb
Compare
`simulatedUpgradeDuration` let the helper sleep instead of running `brew upgrade --cask`, for the DEBUG "Skip the Real Upgrade" toggle and for a UI-test scenario. A path that steps over the subprocess proves nothing about it, and it duplicated a switch the UI-test seam already provides through fixtures. Gone from the contract, the helper and the handoff. The DEBUG menu keeps one toggle, "Show the Self-Update Banner". A dev build is never the installed cask, so pressing Upgrade on the simulated banner can no longer reach the real handoff — the alternative was quitting a debug build to upgrade whatever copy is installed in /Applications. `DebugSelfUpdateHandoff` refuses it and surfaces a message on the banner instead. Self-update construction moves out of BrewApp.init into an extension behind a SelfUpdateLaunchContext, which the removal would otherwise have pushed over SwiftLint's function-length and parameter-count limits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzYJmZP7T6Eo7GAyyVUb5Y
BREW_UITEST_FAKE_SELF_UPDATE and usesFakeSelfUpdate existed only to route a UI-test launch through the simulated helper, which no longer exists. `testUpgradingQuitsRelaunchesAndAcknowledgesOnTheNextLaunch` now runs against `.selfUpdateRunsBrew` — the real `brew upgrade --cask homebrew-app` against the fake `brew` — making `testTheHelperReallyRunsBrewAndTheAppComesBackReportingSuccess` a subset of it, so that test is gone too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzYJmZP7T6Eo7GAyyVUb5Y
SwiftUI presents only the first `.alert` modifier attached to a view. MainWindowView had one per outcome, so the failure alert was permanently unreachable behind the success one — a failed self-update relaunched the app in silence, which is exactly what SelfUpdateUITests.testAnUpgradeThatExitsNonZeroIsReportedOnTheNextLaunch caught. One `.alert(_:isPresented:presenting:)` now carries both outcomes. Its copy moves out of inline literals into SelfUpdateOutcomePresentation, alongside the banner copy, and is unit-tested there. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzYJmZP7T6Eo7GAyyVUb5Y
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HzYJmZP7T6Eo7GAyyVUb5Y
`brew update` refreshes taps; `brew upgrade` installs newer versions. The app's own self-upgrade runs `brew upgrade --cask homebrew-app`, so "update" was the wrong word in every name it appeared in. Renamed the targets (BrewFeatureSelfUpgrade, BrewSelfUpgradeContract, BrewSelfUpgradeHelperCore, HomebrewUpgradeHelper), every SelfUpdate* type, and the members that carried the verb — performUpgrade(), beginUpgrade(), isUpgradeAvailable. The outdated badge's accessibility label was the last "Update" in user-facing copy and now reads "Upgrade available". Left alone: updateNSView, observeRowUpdates, the caches' update methods and `brew update --auto-update`, which all genuinely refresh rather than upgrade. This moves persisted state: the defaults keys become selfUpgrade.dismissedVersion and selfUpgrade.lastUpgradeOutcome, the transcript becomes self-upgrade.log, and the accessibility identifiers become selfupgrade.*. A dismissal recorded before the rename is forgotten once, re-showing the banner; there is no migration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
The Installed, Upgrades and Discover lists each imported BrewFeatureSelfUpgrade to place the banner themselves, which made one feature depend on another sibling. Composition belongs one level up: the shell already imports every feature to build the split view, so it is the only place allowed to know both exist. `\.packageListBanner` is the seam — a PackageListBanner value in BrewAppEnvironment, the same shape as RefreshAllAction. MainWindowView fills it with the banner; the lists render whatever is in it, at the top of their own column where it was, and cannot name it. The default is empty, so previews, unit tests and any tab the shell does not fill it for get nothing. `.safeAreaInset(edge: .top)` on the feature column would be the tidier hook, but that column is the whole detail area — the banner covered the top of the list and the detail pane both. PackageListBannerSlotTests hosts each list with a fixed-height probe in the slot and asserts the list makes exactly that much room: a slot that is read but never placed still compiles. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EanrVE1jHpcrChwoWHg8pQ
`BrewRunOptions.environment` merges over the inherited environment, after the colour variables the output channel sets, so a caller that pins one of those means it. `LoginShellBrewCommandRunner` already forwards the whole options value, so a pinned variable lands on the shell and is exported on to brew from there. The upgrade helper is the caller that needs it: it is spawned by the app but outlives it, so the fake `brew`'s fixture tree cannot reach it by inheritance. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AAUrMoEfVgkvCK1ZFKLbj
`SelfUpgradeRunner` had its own `Foundation.Process`, pipe drain, line splitter, UTF-8 fallback and timeout. The recorded reason was that `BrewCommandService` exists to stream a pty into the console UI and nobody is watching this one — true of `.pseudoTerminal`, but `.pipes` is the path `brew doctor` and `brew config` already take. It runs through `BrewCommandRunning` now; `BrewSelfUpgradeHelperCore` gains `BrewCLI` and `BrewCore`, which the Xcode helper target picks up transitively. Two requirements came back with it. The login shell. Every brew invocation goes through `LoginShellBrewCommandRunner` because a Finder-launched process inherits none of the `HOMEBREW_*` a profile exports. The helper is spawned by the app, so it inherits that same stripped environment, and the one command that replaces the app was running without the user's mirror, proxy, `HOMEBREW_GITHUB_API_TOKEN` or `HOMEBREW_CASK_OPTS`. `usesLoginShell` carries the app's own `.live()` / `.uiTesting(brewURL:)` choice across the process boundary; under `-uiTesting` it stays false, because wrapping the fake `brew` in a developer's dotfiles is what `BrewCommandExecutionContext.uiTesting` exists to avoid. The timeout. `Process` gives the child no session, so the old runner signalled `brew` alone and then waited for end-of-input on a pipe still held by the `curl` or `git` it left behind — the drain never ends, no outcome is recorded, and the app is never brought back. That is the outcome `upgradeTimeout` exists to prevent, and `a timeout kills the descendants brew left behind` covers it. `createSession` plus a teardown to the process group takes the whole tree, and gives brew two seconds of SIGTERM before the kill rather than stopping it mid-`ditto` of an app bundle. The timeout races the run and cancels it, and the loser is awaited so brew is gone before the helper relaunches. Wall-clock deadlines made the tests that matter pass vacuously under the parallel suite — the fake `brew` had not spawned yet — so the runner takes an injected `sleep`, like `SelfUpgradeHelperRun`, and the tests fire it off a readiness file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AAUrMoEfVgkvCK1ZFKLbj
The handoff quits the app the moment the helper is launched, which kills the `brew` subprocess an install is streaming through the command center. The helper then starts a second brew against the same Homebrew. `runningPhases()` already reports what is in flight, so the handoff asks before writing anything, and throws instead — which surfaces on the banner, where `SelfUpgradeCoordinator` puts a handoff error. It asks for mutating work only: `doctorRead` is the one kind the center schedules that changes nothing, and it runs long enough that counting it would block every upgrade attempted while the Doctor tab is loading. The same commit hands the handoff `usesLoginShell`, which is `uiTesting == nil`: the two things that separate `BrewCommandExecutionContext.live()` from `.uiTesting(brewURL:)`, the locator and the shell wrap, now both reach the helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AAUrMoEfVgkvCK1ZFKLbj
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013AAUrMoEfVgkvCK1ZFKLbj
Swift engineers read this, so a comment restating the code is noise. Trimmed the comments added on this branch to one or two lines, kept only where the reason is genuinely not derivable from the code, and left one copy of each fact rather than repeating it at every site that touches it. Deleted outright the ones that only narrated the line beneath them: the shell composing the banner into the slot, the single .alert carrying both outcomes, the debug provider overriding detection, the debug toggle showing the banner, and the test docs that said no more than the test's own name. The duplicates collapsed onto a single home: "a failed upgrade relaunches looking like an ordinary start" onto SelfUpgradeOutcome; "SwiftUI presents only the first .alert" onto SelfUpgradeOutcomePresentation; "the helper is spawned by the app but outlives it" onto the spec field that carries the environment; "the helper's defaults are a different domain" onto defaultsSuiteName; and the slot's decoupling rationale onto PackageListBannerEnvironment, leaving the lists and their tests to say only what they measure. The two layout essays went with them — BrewSpacing kept the arithmetic that derives each constant and dropped the retelling of how the panes yield, which fittedSplitBottomHeight already documents. Comments only: no code, and swiftformat reported nothing further to change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GqQCTW3RRvpaVA3yfaqMZF
The concurrent-lookup dedup test polled with a 2-second wall-clock deadline for the stub API client's first fetch. On a loaded CI runner the two async-let child tasks can take longer than that just to get scheduled, so the deadline expired with zero calls recorded — the flake behind the last two red runs. Worse, when the deadline expired early, resumeFormula found a nil continuation and no-oped, leaving the awaited lookups to hang forever; the CI runs only failed cleanly because the fetch happened to land in the gap. The stub now exposes waitForFormulaFetch(), resumed on entry to fetchFormulaCatalogue, and the test awaits that instead. Both run on the stub actor, so resumeFormula can no longer beat the continuation store. Verified with five full-suite runs under 16-core busy-load — the condition that previously failed most runs — all green, with the test taking 2.2-2.5s wall clock, past the old deadline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VUmmUy73y4Y346se32YqBC
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR: Update the Homebrew app from inside the Homebrew app
Summary
The app ships as a cask, so Homebrew already reports when it is outdated. This adds the banner, the handoff and the helper that performs the upgrade, so an update needs no terminal.
Changes
brew info --installed --json=v2returns the app's own cask with anoutdatedflag. No second brew call, no timer, no version comparator.BrewFeatureSelfUpdate: coordinator, presentation and a banner on Installed, Upgrades and Discover. "Later" is stored per version, so the next release shows it again.HelperSelfUpdateHandofflaunchesHomebrewUpdateHelperand then quits, in that order, because the helper polls for the pid to disappear. The helper runsbrew upgrade --cask homebrew-app, records the outcome and reopens the app, which acknowledges it on the next launch.BrewSelfUpdateContractis dependency free, so nothing in the app's module graph can leave the helper unable to bring the app back.Why this split
The
AnimatedSplitViewchange rides along: the banner is what pushed the non-scrolling Upgrades chrome past the point where the old split starved the list.Testing
scripts/test: 992 tests in 119 suites, plus 19 in 2 suites for BrewUILint. SwiftLint strict and BrewUILint clean.** BUILD SUCCEEDED **.SelfUpdateUITestsbuild but were not run for this write-up.PR checklist
Co-Authored-Bytrailer. Verified by hand: the suites above and the real cask upgrade.Follow-ups
scripts/stamp-app-versionruns in a phase with no declared outputs, so the plist can be regenerated after it and ship0.0.0as the running version.