Skip to content

Commit df8e239

Browse files
committed
fix(update): refresh brew tap before upgrade and verify result
`pythinker update` on a Homebrew install ran `brew upgrade pythinker-code` against the locally-cloned tap. A stale clone pins the old formula, so the upgrade silently no-ops ("0.37.0 already installed") while the updater — which only checked the subprocess exit code — still printed "Updated successfully!". - Run `brew update --quiet` to refresh the tap before `brew upgrade`, so a stale clone can no longer pin the old version. Best-effort: a failed refresh does not block the upgrade attempt. - After a brew upgrade exits 0, re-resolve the installed version via `brew list --versions` (the running process's own importlib.metadata cannot observe an in-place upgrade) and report a clear failure instead of success when the version did not advance to the target. Both paths are Homebrew-gated; uv/pip/pipx upgrades are unchanged.
1 parent 2308004 commit df8e239

3 files changed

Lines changed: 183 additions & 0 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ GitHub Releases page; `0.8.0` is the new starting line.
1515

1616
## Unreleased
1717

18+
- **Homebrew updater no longer no-ops or false-reports success.** `pythinker update` on a Homebrew install now runs `brew update` to refresh the tap before `brew upgrade`, so a stale local tap clone can't pin the old formula and silently no-op ("0.37.0 already installed"). After upgrading it re-checks the installed version via `brew list --versions` and reports a clear failure instead of "Updated successfully!" when the version did not actually advance.
19+
1820
## 0.38.0 (2026-06-08)
1921

2022
- **Quieter `/login`.** Logging in no longer prints a `RuntimeWarning` about an un-awaited `redraw_in_future` coroutine. The prompt redraw throttle now uses a coroutine-free path (`max_render_postpone_time`), eliminating the warning emitted during the login prompt handoff.

src/pythinker_code/ui/shell/update.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,36 @@ def _is_homebrew_upgrade_command(command: list[str]) -> bool:
648648
return len(command) >= 3 and command[:2] == ["brew", "upgrade"]
649649

650650

651+
def _installed_homebrew_version() -> str | None:
652+
"""Return the highest pythinker-code version Homebrew reports as installed.
653+
654+
Shelling out is required: the running interpreter's own
655+
``importlib.metadata`` still reports the pre-upgrade version until the
656+
process restarts, so it cannot confirm an in-place upgrade. ``None`` means
657+
"could not determine" (brew missing, formula not found, parse failure) — the
658+
caller treats that as inconclusive rather than a failed upgrade.
659+
"""
660+
try:
661+
result = subprocess.run(
662+
["brew", "list", "--versions", "pythinker-code"],
663+
capture_output=True,
664+
text=True,
665+
encoding="utf-8",
666+
errors="replace",
667+
env=get_clean_env(),
668+
timeout=60,
669+
)
670+
except (OSError, subprocess.SubprocessError):
671+
logger.exception("Failed to read installed Homebrew version:")
672+
return None
673+
if result.returncode != 0:
674+
return None
675+
versions = re.findall(r"\d+\.\d+\.\d+", result.stdout)
676+
if not versions:
677+
return None
678+
return max(versions, key=semver_tuple)
679+
680+
651681
def _native_update_asset_name(version: str) -> str | None:
652682
linux_package_kind = _installed_linux_package_kind()
653683
if _is_windows():
@@ -1316,6 +1346,30 @@ def _print(message: str) -> None:
13161346
_print(f"[{_t.muted}]The upgrade will continue in a new process.[/]")
13171347
sys.exit(0)
13181348

1349+
if _is_homebrew_upgrade_command(upgrade_command):
1350+
# `brew upgrade <formula>` resolves against the locally-cloned tap
1351+
# formula; a stale clone pins the old version and the upgrade silently
1352+
# no-ops ("already installed"). Refresh the tap first. Best-effort: if
1353+
# the refresh fails we still attempt the upgrade, and the post-upgrade
1354+
# version check below catches a no-op.
1355+
_print(f"[{_t.muted}]Refreshing Homebrew metadata: brew update[/]")
1356+
try:
1357+
# --quiet keeps the refresh from dumping the host's full outdated
1358+
# formula/cask list (often dozens of unrelated lines) before our
1359+
# upgrade output.
1360+
refresh_code = _run_upgrade_command(
1361+
["brew", "update", "--quiet"],
1362+
print_output=print_output,
1363+
output_callback=output_callback,
1364+
)
1365+
except OSError:
1366+
logger.exception("brew update failed to launch:")
1367+
else:
1368+
if refresh_code != 0:
1369+
logger.warning(
1370+
"brew update exited {code}; continuing with upgrade", code=refresh_code
1371+
)
1372+
13191373
try:
13201374
returncode = _run_upgrade_command(
13211375
upgrade_command,
@@ -1329,6 +1383,20 @@ def _print(message: str) -> None:
13291383
return UpdateResult.FAILED
13301384

13311385
if returncode == 0:
1386+
if _is_homebrew_upgrade_command(upgrade_command):
1387+
installed = _installed_homebrew_version()
1388+
if installed is not None and semver_tuple(installed) < semver_tuple(latest_version):
1389+
# brew exited 0 without changing anything (stale tap / no-op).
1390+
# Reporting success here is the bug we are fixing: don't.
1391+
_print(
1392+
f"[{_t.error}]Homebrew exited cleanly but pythinker-code is "
1393+
f"still {installed}, not {latest_version}.[/]"
1394+
)
1395+
_print(
1396+
f"[{_t.warning}]The Homebrew tap metadata looks stale. "
1397+
"Run 'brew update' and try '/update' again.[/]"
1398+
)
1399+
return UpdateResult.FAILED
13321400
_print(f"[{_t.success}]Updated successfully![/]")
13331401
_print(f"[{_t.warning}]Restart Pythinker CLI to use the new version.[/]")
13341402
return UpdateResult.UPDATED

tests/ui_and_conv/test_shell_update.py

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,3 +1262,116 @@ def test_update_prompt_text_renders_managed_channel_hint(monkeypatch):
12621262
rendered = text.plain
12631263
assert "docker" in rendered
12641264
assert update.MANAGED_CHANNEL_MARKER not in rendered
1265+
1266+
1267+
# ---------------------------------------------------------------------------
1268+
# Homebrew upgrade: refresh tap before upgrade + verify version advanced
1269+
#
1270+
# Regression: `brew upgrade <formula>` reads the locally-cloned tap formula.
1271+
# With a stale clone, the upgrade no-ops ("0.37.0 already installed") yet the
1272+
# updater (which only checked the exit code) printed "Updated successfully!".
1273+
# ---------------------------------------------------------------------------
1274+
1275+
1276+
def _brew_upgrade_do_update_env(monkeypatch, tmp_path):
1277+
"""Wire do_update onto the Homebrew upgrade path with no real network/brew."""
1278+
1279+
async def fake_get_latest(session):
1280+
return "999.0.0"
1281+
1282+
async def fake_unavailable(session, latest_version: str, upgrade_command: list[str]):
1283+
return None
1284+
1285+
monkeypatch.setattr(update, "LATEST_VERSION_FILE", tmp_path / "latest.txt")
1286+
monkeypatch.setattr(update, "_get_latest_version", fake_get_latest)
1287+
monkeypatch.setattr(update, "_update_candidate_unavailable_reason", fake_unavailable)
1288+
monkeypatch.setattr(
1289+
update, "_detect_upgrade_command", lambda: ["brew", "upgrade", "pythinker-code"]
1290+
)
1291+
1292+
1293+
@pytest.mark.asyncio
1294+
async def test_do_update_brew_refreshes_tap_before_upgrade(monkeypatch, tmp_path):
1295+
"""`brew update` must run before `brew upgrade` so a stale tap clone can't
1296+
pin the old formula version and silently no-op the upgrade."""
1297+
ran: list[list[str]] = []
1298+
1299+
def fake_run_upgrade_command(command, *, print_output: bool, output_callback):
1300+
ran.append(command)
1301+
return 0
1302+
1303+
_brew_upgrade_do_update_env(monkeypatch, tmp_path)
1304+
monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command)
1305+
# Version genuinely advanced after the upgrade.
1306+
monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "999.0.0")
1307+
1308+
result = await update.do_update(print_output=False)
1309+
1310+
assert result is update.UpdateResult.UPDATED
1311+
assert ran == [["brew", "update", "--quiet"], ["brew", "upgrade", "pythinker-code"]]
1312+
1313+
1314+
@pytest.mark.asyncio
1315+
async def test_do_update_brew_reports_failure_when_version_unchanged(monkeypatch, tmp_path):
1316+
"""A no-op `brew upgrade` exits 0; the updater must NOT claim success when
1317+
the installed version did not advance to the target."""
1318+
messages: list[str] = []
1319+
1320+
def fake_run_upgrade_command(command, *, print_output: bool, output_callback):
1321+
return 0
1322+
1323+
_brew_upgrade_do_update_env(monkeypatch, tmp_path)
1324+
monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command)
1325+
# brew exited 0 but the keg is still the old version (stale tap).
1326+
monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "0.37.0")
1327+
1328+
result = await update.do_update(print_output=False, output_callback=messages.append)
1329+
1330+
assert result is update.UpdateResult.FAILED
1331+
assert not any("Updated successfully" in m for m in messages)
1332+
assert any("still 0.37.0" in m for m in messages)
1333+
assert any("brew update" in m for m in messages)
1334+
1335+
1336+
@pytest.mark.asyncio
1337+
async def test_do_update_brew_continues_when_refresh_fails(monkeypatch, tmp_path):
1338+
"""A failing `brew update` (e.g. transient network) must not block the
1339+
upgrade attempt — the upgrade still runs and can still succeed."""
1340+
ran: list[list[str]] = []
1341+
1342+
def fake_run_upgrade_command(command, *, print_output: bool, output_callback):
1343+
ran.append(command)
1344+
return 1 if command == ["brew", "update", "--quiet"] else 0
1345+
1346+
_brew_upgrade_do_update_env(monkeypatch, tmp_path)
1347+
monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command)
1348+
monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "999.0.0")
1349+
1350+
result = await update.do_update(print_output=False)
1351+
1352+
assert result is update.UpdateResult.UPDATED
1353+
assert ran == [["brew", "update", "--quiet"], ["brew", "upgrade", "pythinker-code"]]
1354+
1355+
1356+
def test_installed_homebrew_version_returns_max_installed(monkeypatch):
1357+
"""Parses `brew list --versions`, returning the highest installed version."""
1358+
1359+
class FakeCompleted:
1360+
returncode = 0
1361+
stdout = "pythinker-code 0.37.0 0.38.0\n"
1362+
1363+
monkeypatch.setattr(update.subprocess, "run", lambda *a, **k: FakeCompleted())
1364+
1365+
assert update._installed_homebrew_version() == "0.38.0"
1366+
1367+
1368+
def test_installed_homebrew_version_returns_none_on_failure(monkeypatch):
1369+
"""A non-zero `brew list` (formula not found) yields None, not a crash."""
1370+
1371+
class FakeCompleted:
1372+
returncode = 1
1373+
stdout = ""
1374+
1375+
monkeypatch.setattr(update.subprocess, "run", lambda *a, **k: FakeCompleted())
1376+
1377+
assert update._installed_homebrew_version() is None

0 commit comments

Comments
 (0)