Skip to content

Pre release upgrade improvement - #143

Merged
graeme merged 13 commits into
mainfrom
pre-release-upgrade-improvement
Sep 5, 2026
Merged

Pre release upgrade improvement#143
graeme merged 13 commits into
mainfrom
pre-release-upgrade-improvement

Conversation

@graeme

@graeme graeme commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

PR: Upgrade detection and Upgrades tab fixes

Summary

Six fixes to how the app detects and presents upgrades, one commit each. The headline bug is that a formula revision bump advertised an upgrade to the version already installed.

Changes

Revision bumps showed no change. A revision bump leaves versions.stable untouched and moves only revision, so ffmpeg 9.0.1 revision 1 rendered as "v9.0.1 to v9.0.1". A new HomebrewPkgVersion renders Homebrew's own PkgVersion (version_revision), wired into the brew info mapping and the formula catalogue. Display only: the app reads brew's outdated boolean and never compares versions itself, so detection was always correct.

A failed check read as "no upgrades". The repository deliberately keeps cached packages loaded when a refresh fails, and only logged the error, so the tab claimed everything was up to date when it had never found out. Added refreshFailure, cleared only by a fetch that completes. The tab now shows an error state carrying brew's own message plus Try Again; where cached upgrades exist the list stays but the subtitle stops presenting the count as current.

HOMEBREW_NO_INSTALL_FROM_API never saw new versions. brew info is not in Homebrew's auto-update command list. With the API off, package data comes from tap clones, so the check answered from the user's last manual brew update, indefinitely. Now runs brew update --auto-update --quiet ahead of the fetch, on Homebrew's own 300s interval for that mode. Detection goes via brew config, since a Finder-launched app never sees a shell-exported HOMEBREW_ variable.

Smaller fixes. "Latest stable" is now "Latest version", since casks legitimately track unstable channels. Refresh moved out of the "all caught up" empty state, where it only appeared when there was nothing to refresh, into the header. The four phrasings of "up to date" now route through one UpgradesUpToDateCopy.

Testing

  • scripts/test passes
  • SwiftFormat, SwiftLint --strict and BrewUILint clean
  • xcodebuild -scheme Brew builds
  • Checked against real brew info --json=v2 --installed output: no package now renders as an upgrade to the same string, and all seven revisioned formulae carry the suffix
  • UI tests not run in this environment

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 changes, tests and this description. Root causes were confirmed against the local Homebrew install (auto-update.sh, brew config, real --json=v2 output) rather than assumed, and the gates above were run locally.

Follow-ups

  • The Installed tab reads the same repository and could surface refreshFailure; only the Upgrades tab does today.
  • The sidebar badge shows nothing when a check fails, which is indistinguishable from a zero count.

@graeme
graeme force-pushed the pre-release-upgrade-improvement branch from 23a45a9 to 5d0873a Compare September 2, 2026 22:13
graeme and others added 9 commits September 4, 2026 18:28
A revision bump repackages the same upstream release, so `versions.stable`
does not move — only the top-level `revision` does. Reading `versions.stable`
alone meant the Upgrades tab advertised a target identical to the installed
keg: ffmpeg 9.0.1 revision 1 rendered as "v9.0.1 → v9.0.1".

Decode `revision` from `brew info --json=v2` and from the formula catalogue,
and render Homebrew's own PkgVersion (`version_revision`) through a shared
`HomebrewPkgVersion` helper, so the upgrade target reads "v9.0.1 → v9.0.1_1"
and matches the keg brew will actually install.

The catalogue field is optional and revision 0 still renders bare, so payloads
without it decode and display exactly as before.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
"Latest stable" is wrong for casks: plenty of them intentionally track a beta
or nightly channel, and the row shows whatever version that cask publishes.
The same row is also the upgrade target for formulae with a packaging
revision, which is not a stable-version bump either.

Rename the row in both the Discover and Installed detail panes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
Refresh only existed inside the "all caught up" empty state, so it was
reachable precisely when there was nothing to re-check and unreachable
whenever the list was populated, stale or wrong — the cases where a user
actually wants to check again.

Move it into the header action row next to Upgrade All, where it is present
for every loaded state, give it ⌘R, and show a spinner while the fetch is in
flight (the repository stays `.loaded` during a revalidation, so the button
needs its own progress signal). The empty state keeps its copy and drops the
now-duplicate button.

The header moves to its own view; UpgradesPackagesView was over SwiftLint's
type-body-length limit with the extra button in place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
The tab claimed the same thing four different ways, all on screen at once:
"All packages are up to date" (subtitle), "Nothing to upgrade" (header
stand-in), "✅ You're all caught up" (empty-state title) and "All N packages
are at their latest versions." (empty-state subtitle), plus a fifth wording in
the VoiceOver label. Four phrasings read as four different claims.

Route every one of them through `UpgradesUpToDateCopy`: all render the same
"Everything is up to date" headline, and only the supporting count line
varies — itself reworded to the same vocabulary.

"Nothing to upgrade here" stays as it is: filters hiding every upgrade is a
different claim from having no upgrades, and a test pins it apart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
The inventory repository deliberately keeps cached packages on screen when a
refresh fails, and only logs the error. Nothing downstream could see that, so
a `brew info --installed --json=v2` that never completed rendered as a green
tick and "Everything is up to date" — the app reporting an answer it did not
have.

Record the failure on the repository (`refreshFailure`), cleared only by a
fetch that completes, so repainting a still-fresh cache cannot quietly turn
"couldn't check" back into "nothing to upgrade". The Upgrades tab reads it:

- nothing outdated + failed check → an error state naming brew's own message
  and saying plainly that an empty list here means unknown, with Try Again,
  and a warning glyph in place of the green tick;
- cached upgrades + failed check → the list still shows, but the subtitle
  stops presenting the count as current;
- filters hiding known upgrades still win, since the filter is then the honest
  explanation for the empty list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
`brew info` is not one of the commands brew auto-updates in front of (unlike
`brew upgrade` and `brew outdated`). Normally that costs nothing: brew
re-fetches the JSON API files on its own TTL whenever a command reads them.

Under `HOMEBREW_NO_INSTALL_FROM_API` there is no such refresh. Formula and
cask data come from tap git clones, so `brew info --installed --json=v2`
reports whatever the taps held the last time the user ran `brew update` in a
terminal — indefinitely. Upgrades that `brew upgrade` would find never appear
in the app at all.

Ask brew which source it is using (`brew config` prints
`HOMEBREW_NO_INSTALL_FROM_API: set` only when it is; the app's own process
environment never sees a shell-exported variable), and when the API is off run
`brew update --auto-update --quiet` ahead of the info fetch — the same command
`brew upgrade` runs, on Homebrew's own 5-minute interval for this mode.

A failed tap update is logged and not fatal: the taps keep their previous
contents, and the info fetch is what decides whether the check produced an
answer. The attempt is timestamped either way, so a persistently failing
update cannot stall every fetch behind it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
Why brew never refreshes the data `brew info` reads, why the app has to ask
`brew config` rather than its own environment for `HOMEBREW_*`, and why
`refreshFailure` can only be cleared by a completed fetch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
Cut the narration and the lines restating what the code says. What survives is
the handful of things that aren't visible from the code: why the repository
runs `brew update`, why `refreshFailure` can only be cleared by a completed
fetch, where the 300s interval comes from, and why the app can't read
HOMEBREW_* from its own environment. The reasoning that was in the source is
already in the commit messages and .ai/memory.md.

Also drops `tapUpdateResponse`, an unused test-support helper.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
main since split warning into two roles: status.warning is the text colour and
darkens under high contrast, status.warningBold stays vivid for glyphs, dots
and fills. The header's failed-check indicator was written before that and
colours an exclamationmark.triangle.fill with the text token, so under high
contrast it would render dark brown instead of the intended yellow with the
mark knocked out.

Route it through brewWarningGlyphStyle(), matching DoctorSeverityStyle and the
rule documented in docs/BrewUI-DesignSystem.md. The success tick keeps
status.success, which has no such split.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
@graeme
graeme force-pushed the pre-release-upgrade-improvement branch from 5d0873a to f965862 Compare September 4, 2026 09:12
graeme and others added 3 commits September 5, 2026 19:07
⌘R was a local shortcut on the Upgrades refresh button, so it did nothing on
Installed, Discover or Configuration — each of which caches and goes stale the
same way.

Promote it to a window-wide View menu command. MainWindowView publishes what
refreshing means via a focused scene value, following the SearchCommands
pattern, and force-refreshes the installed inventory, the Discover packages and
the brew config together.

Doctor is deliberately excluded: it shells out to a slow `brew doctor` run and
already has its own explicit "Run Again".

The Upgrades button keeps its click action, which still refreshes upgrades
alone, but loses the local shortcut so the two do not both claim ⌘R.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
The button swapped its whole label for a ProgressView, so it shrank to the
spinner's width mid-refresh and the header jumped.

Keep the label laid out and hide it with opacity, drawing the spinner over the
top. The button holds the size it has at rest, like Upgrade All beside it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
The state had a title and body but no glyph, so it did not read as a warning at
a glance the way Doctor's caution rows do.

Add the same `exclamationmark.triangle.fill` through `brewWarningGlyphStyle()`,
so it picks up the high-contrast knockout treatment for free. The empty-state
builder grows an optional icon; the two states that do not want one are
unchanged.

Call sites now pass `action:` by label. With a second closure parameter in the
signature, an unlabelled trailing closure forward-matches to `icon` instead.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
@graeme
graeme marked this pull request as ready for review September 5, 2026 09:25
The release archive failed to compile: `load(forceRefresh:)` is declared on the
repository protocols in BrewRepositoryInterfaces, and MainWindowView's only
import of that module sat inside `#if DEBUG` for the preview.

Imports are file-scope, so Debug builds saw it and compiled the new refreshAll
calls fine. Release strips the block, the import goes with it, and the calls no
longer resolve.

Move the import up to the file's own imports, where it belongs now that
non-preview code depends on it.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F8CTwBp39fDeDz7BnfEq6K
@graeme
graeme merged commit 711924e into main Sep 5, 2026
10 checks passed
@graeme
graeme deleted the pre-release-upgrade-improvement branch September 5, 2026 11:12
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