Skip to content

Self upgrade mechanism - #152

Merged
graeme merged 35 commits into
mainfrom
self-upgrade-mechanism
Sep 9, 2026
Merged

graeme merged 35 commits into
mainfrom
self-upgrade-mechanism

Conversation

@graeme

@graeme graeme commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

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

  • Detection is a lookup in data already fetched: brew info --installed --json=v2 returns the app's own cask with an outdated flag. No second brew call, no timer, no version comparator.
  • New BrewFeatureSelfUpdate: coordinator, presentation and a banner on Installed, Upgrades and Discover. "Later" is stored per version, so the next release shows it again.
  • An app cannot replace its own bundle while running. HelperSelfUpdateHandoff launches HomebrewUpdateHelper and then quits, in that order, because the helper polls for the pid to disappear. The helper runs brew upgrade --cask homebrew-app, records the outcome and reopens the app, which acknowledges it on the next launch.
  • The helper reads a JSON spec on disk, and BrewSelfUpdateContract is dependency free, so nothing in the app's module graph can leave the helper unable to bring the app back.

Why this split

The AnimatedSplitView change 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.
  • Release build of the Brew scheme: ** BUILD SUCCEEDED **.
  • End to end against a local tap: installed as a cask at 0.2.1, published 0.2.2, pressed Upgrade. The app quit, the helper upgraded the cask, and it reopened at 0.2.2 with the Gatekeeper approval inherited, so the relaunch was not prompted.
  • SelfUpdateUITests build but were not run for this write-up.

PR checklist

  • Have you followed this repository's contribution and workflow guidance?
  • Have you explained what changed and why this should land now?
  • Have you run relevant local checks for the changed scope?
  • Are changes scoped and free of unrelated modifications?

  • AI was used to generate or assist with generating this PR.
  • Claude Code wrote the implementation and tests; every commit carries a Co-Authored-By trailer. Verified by hand: the suites above and the real cask upgrade.

Follow-ups

  • The version stamp is racy: scripts/stamp-app-version runs in a phase with no declared outputs, so the plist can be regenerated after it and ship 0.0.0 as the running version.

graeme and others added 23 commits September 8, 2026 17:53
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
@graeme
graeme force-pushed the self-upgrade-mechanism branch from e10c5ee to 4a6adfb Compare September 8, 2026 09:01
graeme and others added 5 commits September 8, 2026 22:33
`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
`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
graeme and others added 7 commits September 8, 2026 23:56
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
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
@graeme
graeme marked this pull request as ready for review September 9, 2026 12:41
@graeme
graeme merged commit 504812a into main Sep 9, 2026
11 checks passed
@graeme
graeme deleted the self-upgrade-mechanism branch September 9, 2026 12:46
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.

2 participants