fix(cli): stop reporting "up to date" when the Homebrew update check fails - #1298
Conversation
`_check_homebrew_update_available()` decided outdated-or-not purely from
`brew outdated`'s stdout and discarded the return code. Every failure mode
of that command produces the same shape -- non-zero exit, empty stdout,
reason on stderr -- so a failed check was read as "not outdated" and the
caller reported `UP_TO_DATE`. A hard failure to determine the latest
version was reported to the user as success, silently pinning Homebrew
installs to whatever version they happened to have.
Observed with newer Homebrew's third-party tap trust requirement
("Refusing to load formula ... from untrusted tap"), but a stale tap, a
missing formula, brew off PATH, or a network failure all produce the same
empty-stdout shape.
Model the three real outcomes instead of two: exit 0 with empty stdout is
up to date, the package name on stdout is outdated, and anything else is
unanswered and now raises `HomebrewCheckError`. `run_auto_update()` catches
it and falls back to the PyPI comparison, which can answer the question --
the tap can only lag PyPI, never lead it -- and yields a real
`latest_version`, so Homebrew users no longer see "Update available
(latest: unknown)". `source` is unchanged, so remediation still points at
`brew upgrade basic-memory`. If PyPI is also unreachable the existing
handler reports FAILED rather than a false all-clear.
Closes #1297
Signed-off-by: Drew Cain <groksrc@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2f66fb7bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
`brew outdated` writes progress output ("==> Downloading Homebrew API
data") to stderr on the successful outdated path, so a non-empty stderr is
not an error signal on its own. The outdated-case test asserted against an
empty stderr, leaving nothing to catch a future "stderr means failure"
reading. Use brew's real stderr instead.
This locks the three real result shapes into the suite: exit 0 with empty
stdout (up to date), exit 1 with the tap-qualified formula name on stdout
(outdated), and exit 1 with empty stdout plus an `Error:` stderr
(unanswered). Exit 1 is shared by the middle and last, so stdout stays the
discriminator and the return code only breaks the tie when stdout is empty.
Signed-off-by: Drew Cain <groksrc@gmail.com>
|
Follow-up (e136bad) pinning one more empirically-confirmed detail into the tests.
So neither "non-zero exit" nor "non-empty stderr" is an error signal on its own. The fix keys on stdout first and only consults the return code to break the tie when stdout is empty; stderr is used solely as message text after the decision is made. Two consequences worth flagging for review:
Verified the suite rejects the naive implementation: replacing the body with |
The PyPI fallback for a failed `brew outdated` set update_available=True, and run_auto_update() then proceeded to run `brew upgrade`. That is unsafe two ways: release.yml publishes to PyPI in the `release` job while the Homebrew formula job `needs: release`, so PyPI can carry a version the tap cannot install yet; and whatever hid the brew answer (untrusted tap, brew missing) also blocks the upgrade itself, so the command is doomed. Keep the fallback for the negative answer -- the tap can only lag PyPI, never lead it, so "nothing newer exists" is sound -- but when availability was inferred from PyPI because brew could not answer, report the update plus the brew failure and let the user upgrade deliberately. Reported by Codex review on #1298. Signed-off-by: Drew Cain <groksrc@gmail.com>
Signed-off-by: phernandez <paul@basicmachines.co>
|
Completed the remaining #1297 version-reporting gap in c91190a. The Homebrew check now uses Verified against real Homebrew output for an outdated formula, an up-to-date formula, and the untrusted-tap failure. The focused suite is now 31 passing, with Ruff and the repository typecheck clean. @codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The PyPI fallback for a failed `brew outdated` set update_available=True, and run_auto_update() then proceeded to run `brew upgrade`. That is unsafe two ways: release.yml publishes to PyPI in the `release` job while the Homebrew formula job `needs: release`, so PyPI can carry a version the tap cannot install yet; and whatever hid the brew answer (untrusted tap, brew missing) also blocks the upgrade itself, so the command is doomed. Keep the fallback for the negative answer -- the tap can only lag PyPI, never lead it, so "nothing newer exists" is sound -- but when availability was inferred from PyPI because brew could not answer, report the update plus the brew failure and let the user upgrade deliberately. Reported by Codex review on #1298. Signed-off-by: Drew Cain <groksrc@gmail.com>
2 participants
Closes #1297
Why
brew outdated --quietused empty stdout for both an up-to-date result and several real failures, so Basic Memory could report a failed check asUP_TO_DATE.Update available (latest: unknown).What Changed
brew outdated --json=v2to report Homebrew's owncurrent_versionfor a confirmed outdated formula.Implementation Details
HomebrewCheckErrorfor the existing fallback path.basicmachines-co/basic-memory/basic-memoryare matched by their final formula component.Testing
Automated
uv run pytest tests/cli/test_auto_update.py -q: 31 passed.uv run ruff check src/basic_memory/cli/auto_update.py tests/cli/test_auto_update.py: passed.just typecheck: passed.Manual
brew outdated --json=v2 ada-urlexits 1 with a populated formula entry andcurrent_version.brew outdated --json=v2 cmuxexits 0 with an empty formula list.Risks / Follow-ups
FAILED; it is never reported as up to date.