fix(cli): stop reporting "up to date" when the Homebrew update check fails

src/basic_memory/cli/auto_update.py

@@ -27,6 +27,10 @@
 BREW_UPGRADE_TIMEOUT_SECONDS = 600
 
 
+class HomebrewCheckError(RuntimeError):
+    """Raised when `brew outdated` could not determine whether an update exists."""
+
+
 class InstallSource(str, Enum):
     """How the running CLI appears to have been installed."""
 
@@ -127,19 +131,36 @@ def _version_from_pypi() -> str:
 
 
 def _check_homebrew_update_available(silent: bool) -> tuple[bool, str | None]:
-    """Check whether Homebrew reports an outdated basic-memory formula."""
-    result = _run_subprocess(
-        ["brew", "outdated", "--quiet", PACKAGE_NAME],
-        timeout_seconds=BREW_OUTDATED_TIMEOUT_SECONDS,
-        silent=silent,
-        capture_output=True,
-    )
-    # Trigger: brew outdated exits 1 when the formula IS outdated (with name on stdout).
-    # Why: non-zero exit here means "outdated", not "error".
-    # Outcome: check stdout for the package name to determine outdated status.
+    """Check whether Homebrew reports an outdated basic-memory formula.
+
+    Raises:
+        HomebrewCheckError: brew could not answer the question (brew missing,
+            untrusted or stale tap, network failure, timeout).
+    """
+    try:
+        result = _run_subprocess(
+            ["brew", "outdated", "--quiet", PACKAGE_NAME],
+            timeout_seconds=BREW_OUTDATED_TIMEOUT_SECONDS,
+            silent=silent,
+            capture_output=True,
+        )
+    except (OSError, subprocess.SubprocessError) as exc:
+        raise HomebrewCheckError(f"could not run `brew outdated`: {exc}") from exc
+
+    # Trigger: brew outdated exits 1 both when the formula IS outdated (name on
+    # stdout) and when the check failed outright (empty stdout, reason on stderr).
+    # Why: reading a failed check as "not outdated" reports a stale install as up
+    # to date, which silently pins the user to an old version.
+    # Outcome: trust an empty stdout only when brew exited cleanly; otherwise the
+    # check is unanswered and the caller must find the answer elsewhere.
     stdout = (result.stdout or "").strip()
-    is_outdated = PACKAGE_NAME in stdout
-    return is_outdated, None
+    if PACKAGE_NAME in stdout:
+        return True, None
+    if result.returncode == 0:
+        return False, None
+
+    stderr = (result.stderr or "").strip()
+    raise HomebrewCheckError(stderr or f"`brew outdated` exited {result.returncode}")
 
 
 def _check_pypi_update_available() -> tuple[bool, str]:
@@ -246,8 +267,21 @@ def run_auto_update(
     try:
         # --- Availability check ---
         latest_version: str | None = None
+        homebrew_check_error: str | None = None
         if source == InstallSource.HOMEBREW:
-            update_available, latest_version = _check_homebrew_update_available(silent=silent)
+            try:
+                update_available, latest_version = _check_homebrew_update_available(silent=silent)
+            except HomebrewCheckError as exc:
+                # Trigger: brew cannot answer (missing/untrusted tap, no brew, network).
+                # Why: an unanswered check must never be reported as up to date. PyPI is
+                # sound for the negative answer -- the tap can only lag PyPI, so "nothing
+                # newer exists" holds. It is NOT sound for installing: release.yml
+                # publishes to PyPI in `release`, and the homebrew formula job `needs:
+                # release`, so a newer PyPI version may not be installable via brew yet.
+                # Outcome: ask PyPI, but remember the answer came from there.
+                homebrew_check_error = str(exc)
+                logger.warning(f"Homebrew update check failed, falling back to PyPI: {exc}")
+                update_available, latest_version = _check_pypi_update_available()
         else:
             update_available, latest_version = _check_pypi_update_available()
 
@@ -290,6 +324,26 @@ def run_auto_update(
                 ),
             )
 
+        if homebrew_check_error is not None:
+            # Trigger: availability was inferred from PyPI because brew could not answer.
+            # Why: the tap may not carry this version yet, and whatever hid the brew
+            # answer (untrusted tap, brew missing) will also block `brew upgrade`.
+            # Outcome: report the update and the reason instead of running a doomed
+            # upgrade; the user resolves the brew problem and upgrades deliberately.
+            return AutoUpdateResult(
+                status=AutoUpdateStatus.UPDATE_AVAILABLE,
+                source=source,
+                checked=True,
+                update_available=True,
+                updated=False,
+                latest_version=latest_version,
+                message=(
+                    f"Update available (latest: {latest_version or 'unknown'}), but the "
+                    f"Homebrew check failed: {homebrew_check_error} "
+                    f"{_manual_update_hint(source)}"
+                ),
+            )
+
         # --- Automatic install ---
         command = (
             ["uv", "tool", "upgrade", PACKAGE_NAME]

tests/cli/test_auto_update.py

@@ -4,15 +4,18 @@
 
 import subprocess
 import sys
+import urllib.error
 from datetime import datetime, timedelta, timezone
 from io import StringIO
 from typing import Any, cast
 
+import pytest
 from rich.console import Console
 
 from basic_memory.cli.auto_update import (
     AutoUpdateResult,
     AutoUpdateStatus,
+    HomebrewCheckError,
     InstallSource,
     _check_homebrew_update_available,
     _is_interactive_session,
@@ -23,6 +26,11 @@
 )
 from basic_memory.config import BasicMemoryConfig
 
+UNTRUSTED_TAP_STDERR = (
+    "Error: Refusing to load formula basicmachines-co/basic-memory/basic-memory "
+    "from untrusted tap basicmachines-co/basic-memory."
+)
+
 
 class StubConfigManager:
     """Simple in-memory ConfigManager stub for updater tests."""
@@ -138,11 +146,19 @@ def test_force_bypasses_auto_update_disabled(monkeypatch, tmp_path):
 
 
 def test_check_homebrew_update_available_exit_code_1_means_outdated(monkeypatch):
-    """brew outdated exits 1 when the formula is outdated, not on error."""
+    """brew outdated exits 1 when the formula is outdated, not on error.
+
+    Exit 1 is shared with the failure case, so stdout is the discriminator: the
+    tap-qualified formula name means outdated. brew also writes progress chatter
+    to stderr on this path, so a non-empty stderr must not be read as an error.
+    """
 
     def _fake_run(command, **kwargs):
         return subprocess.CompletedProcess(
-            command, 1, stdout="basicmachines-co/basic-memory/basic-memory\n", stderr=""
+            command,
+            1,
+            stdout="basicmachines-co/basic-memory/basic-memory\n",
+            stderr="==> Downloading Homebrew API data",
         )
 
     monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
@@ -161,6 +177,149 @@ def _fake_run(command, **kwargs):
     assert is_outdated is False
 
 
+def test_check_homebrew_update_available_failed_check_is_not_up_to_date(monkeypatch):
+    """A failed `brew outdated` exits non-zero with empty stdout -- it is not an answer."""
+
+    def _fake_run(command, **kwargs):
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr=UNTRUSTED_TAP_STDERR)
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
+
+    with pytest.raises(HomebrewCheckError) as excinfo:
+        _check_homebrew_update_available(silent=False)
+
+    assert "untrusted tap" in str(excinfo.value)
+
+
+def test_check_homebrew_update_available_reports_missing_brew(monkeypatch):
+    """brew not on PATH must surface as an unanswered check, not as up to date."""
+
+    def _raise_not_found(command, **kwargs):
+        raise FileNotFoundError(command[0])
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _raise_not_found)
+
+    with pytest.raises(HomebrewCheckError):
+        _check_homebrew_update_available(silent=False)
+
+
+def test_failed_homebrew_check_falls_back_to_pypi(monkeypatch, tmp_path):
+    """Regression: a failed brew check reported the install as up to date.
+
+    The untrusted tap is one trigger; a stale tap, a missing formula, or a network
+    failure produce the same empty-stdout shape.
+    """
+    config = _base_config(tmp_path)
+    manager = StubConfigManager(config)
+
+    def _fake_run(command, **kwargs):
+        assert command[:2] == ["brew", "outdated"]
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr=UNTRUSTED_TAP_STDERR)
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
+    monkeypatch.setattr(
+        "basic_memory.cli.auto_update._check_pypi_update_available",
+        lambda: (True, "9.9.9"),
+    )
+
+    result = run_auto_update(
+        check_only=True,
+        config_manager=_config_manager(manager),
+        executable="/opt/homebrew/Cellar/basic-memory/0.18.0/bin/python",
+    )
+
+    assert result.status == AutoUpdateStatus.UPDATE_AVAILABLE
+    assert result.latest_version == "9.9.9"
+    assert "brew upgrade basic-memory" in (result.message or "")
+
+
+def test_failed_homebrew_check_does_not_auto_upgrade(monkeypatch, tmp_path):
+    """Availability inferred from PyPI must not trigger an automatic `brew upgrade`.
+
+    release.yml publishes to PyPI in the `release` job and the Homebrew formula job
+    `needs: release`, so PyPI can carry a version the tap cannot install yet. On top
+    of that, whatever hid the brew answer (untrusted tap, missing brew) also blocks
+    the upgrade -- so acting on the PyPI answer runs a doomed command.
+    """
+    config = _base_config(tmp_path)
+    manager = StubConfigManager(config)
+    calls: list[list[str]] = []
+
+    def _fake_run(command, **kwargs):
+        calls.append(command)
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr=UNTRUSTED_TAP_STDERR)
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
+    monkeypatch.setattr(
+        "basic_memory.cli.auto_update._check_pypi_update_available",
+        lambda: (True, "9.9.9"),
+    )
+
+    result = run_auto_update(
+        config_manager=_config_manager(manager),
+        executable="/opt/homebrew/Cellar/basic-memory/0.18.0/bin/python",
+    )
+
+    assert result.status == AutoUpdateStatus.UPDATE_AVAILABLE
+    assert result.updated is False
+    assert result.latest_version == "9.9.9"
+    assert ["brew", "upgrade", "basic-memory"] not in calls
+    assert "untrusted tap" in (result.message or "")
+    assert "brew upgrade basic-memory" in (result.message or "")
+
+
+def test_failed_homebrew_check_still_trusts_a_pypi_negative(monkeypatch, tmp_path):
+    """The fallback stays authoritative for "nothing newer exists".
+
+    The tap can only lag PyPI, never lead it, so a PyPI "up to date" is sound even
+    when brew could not answer. Only the positive answer is unsafe to act on.
+    """
+    config = _base_config(tmp_path)
+    manager = StubConfigManager(config)
+
+    def _fake_run(command, **kwargs):
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr=UNTRUSTED_TAP_STDERR)
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
+    monkeypatch.setattr(
+        "basic_memory.cli.auto_update._check_pypi_update_available",
+        lambda: (False, "0.0.0"),
+    )
+
+    result = run_auto_update(
+        config_manager=_config_manager(manager),
+        executable="/opt/homebrew/Cellar/basic-memory/0.18.0/bin/python",
+    )
+
+    assert result.status == AutoUpdateStatus.UP_TO_DATE
+    assert result.update_available is False
+
+
+def test_failed_homebrew_check_reports_failure_when_pypi_is_unreachable(monkeypatch, tmp_path):
+    """With neither source able to answer, report the failure rather than success."""
+    config = _base_config(tmp_path)
+    manager = StubConfigManager(config)
+
+    def _fake_run(command, **kwargs):
+        return subprocess.CompletedProcess(command, 1, stdout="", stderr=UNTRUSTED_TAP_STDERR)
+
+    def _pypi_unreachable():
+        raise urllib.error.URLError("network unreachable")
+
+    monkeypatch.setattr("basic_memory.cli.auto_update._run_subprocess", _fake_run)
+    monkeypatch.setattr(
+        "basic_memory.cli.auto_update._check_pypi_update_available", _pypi_unreachable
+    )
+
+    result = run_auto_update(
+        config_manager=_config_manager(manager),
+        executable="/opt/homebrew/Cellar/basic-memory/0.18.0/bin/python",
+    )
+
+    assert result.status == AutoUpdateStatus.FAILED
+    assert result.update_available is False
+
+
 def test_preload_lazy_console_modules_imports_deferred_modules(monkeypatch):
     # Regression: the in-place upgrade deletes the running install's files, so
     # any module rich/typer defers until print time must already be loaded or