From df8e23996032cde2fc3cd11faec44709045de7b3 Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Mon, 8 Jun 2026 12:21:50 -0400 Subject: [PATCH 1/2] fix(update): refresh brew tap before upgrade and verify result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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. --- CHANGELOG.md | 2 + src/pythinker_code/ui/shell/update.py | 68 +++++++++++++++ tests/ui_and_conv/test_shell_update.py | 113 +++++++++++++++++++++++++ 3 files changed, 183 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 831f8895..e437cab0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- **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. + ## 0.38.0 (2026-06-08) - **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. diff --git a/src/pythinker_code/ui/shell/update.py b/src/pythinker_code/ui/shell/update.py index af9caf17..f01f4569 100644 --- a/src/pythinker_code/ui/shell/update.py +++ b/src/pythinker_code/ui/shell/update.py @@ -648,6 +648,36 @@ def _is_homebrew_upgrade_command(command: list[str]) -> bool: return len(command) >= 3 and command[:2] == ["brew", "upgrade"] +def _installed_homebrew_version() -> str | None: + """Return the highest pythinker-code version Homebrew reports as installed. + + Shelling out is required: the running interpreter's own + ``importlib.metadata`` still reports the pre-upgrade version until the + process restarts, so it cannot confirm an in-place upgrade. ``None`` means + "could not determine" (brew missing, formula not found, parse failure) — the + caller treats that as inconclusive rather than a failed upgrade. + """ + try: + result = subprocess.run( + ["brew", "list", "--versions", "pythinker-code"], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + env=get_clean_env(), + timeout=60, + ) + except (OSError, subprocess.SubprocessError): + logger.exception("Failed to read installed Homebrew version:") + return None + if result.returncode != 0: + return None + versions = re.findall(r"\d+\.\d+\.\d+", result.stdout) + if not versions: + return None + return max(versions, key=semver_tuple) + + def _native_update_asset_name(version: str) -> str | None: linux_package_kind = _installed_linux_package_kind() if _is_windows(): @@ -1316,6 +1346,30 @@ def _print(message: str) -> None: _print(f"[{_t.muted}]The upgrade will continue in a new process.[/]") sys.exit(0) + if _is_homebrew_upgrade_command(upgrade_command): + # `brew upgrade ` resolves against the locally-cloned tap + # formula; a stale clone pins the old version and the upgrade silently + # no-ops ("already installed"). Refresh the tap first. Best-effort: if + # the refresh fails we still attempt the upgrade, and the post-upgrade + # version check below catches a no-op. + _print(f"[{_t.muted}]Refreshing Homebrew metadata: brew update[/]") + try: + # --quiet keeps the refresh from dumping the host's full outdated + # formula/cask list (often dozens of unrelated lines) before our + # upgrade output. + refresh_code = _run_upgrade_command( + ["brew", "update", "--quiet"], + print_output=print_output, + output_callback=output_callback, + ) + except OSError: + logger.exception("brew update failed to launch:") + else: + if refresh_code != 0: + logger.warning( + "brew update exited {code}; continuing with upgrade", code=refresh_code + ) + try: returncode = _run_upgrade_command( upgrade_command, @@ -1329,6 +1383,20 @@ def _print(message: str) -> None: return UpdateResult.FAILED if returncode == 0: + if _is_homebrew_upgrade_command(upgrade_command): + installed = _installed_homebrew_version() + if installed is not None and semver_tuple(installed) < semver_tuple(latest_version): + # brew exited 0 without changing anything (stale tap / no-op). + # Reporting success here is the bug we are fixing: don't. + _print( + f"[{_t.error}]Homebrew exited cleanly but pythinker-code is " + f"still {installed}, not {latest_version}.[/]" + ) + _print( + f"[{_t.warning}]The Homebrew tap metadata looks stale. " + "Run 'brew update' and try '/update' again.[/]" + ) + return UpdateResult.FAILED _print(f"[{_t.success}]Updated successfully![/]") _print(f"[{_t.warning}]Restart Pythinker CLI to use the new version.[/]") return UpdateResult.UPDATED diff --git a/tests/ui_and_conv/test_shell_update.py b/tests/ui_and_conv/test_shell_update.py index d7eaf006..107df817 100644 --- a/tests/ui_and_conv/test_shell_update.py +++ b/tests/ui_and_conv/test_shell_update.py @@ -1262,3 +1262,116 @@ def test_update_prompt_text_renders_managed_channel_hint(monkeypatch): rendered = text.plain assert "docker" in rendered assert update.MANAGED_CHANNEL_MARKER not in rendered + + +# --------------------------------------------------------------------------- +# Homebrew upgrade: refresh tap before upgrade + verify version advanced +# +# Regression: `brew upgrade ` reads the locally-cloned tap formula. +# With a stale clone, the upgrade no-ops ("0.37.0 already installed") yet the +# updater (which only checked the exit code) printed "Updated successfully!". +# --------------------------------------------------------------------------- + + +def _brew_upgrade_do_update_env(monkeypatch, tmp_path): + """Wire do_update onto the Homebrew upgrade path with no real network/brew.""" + + async def fake_get_latest(session): + return "999.0.0" + + async def fake_unavailable(session, latest_version: str, upgrade_command: list[str]): + return None + + monkeypatch.setattr(update, "LATEST_VERSION_FILE", tmp_path / "latest.txt") + monkeypatch.setattr(update, "_get_latest_version", fake_get_latest) + monkeypatch.setattr(update, "_update_candidate_unavailable_reason", fake_unavailable) + monkeypatch.setattr( + update, "_detect_upgrade_command", lambda: ["brew", "upgrade", "pythinker-code"] + ) + + +@pytest.mark.asyncio +async def test_do_update_brew_refreshes_tap_before_upgrade(monkeypatch, tmp_path): + """`brew update` must run before `brew upgrade` so a stale tap clone can't + pin the old formula version and silently no-op the upgrade.""" + ran: list[list[str]] = [] + + def fake_run_upgrade_command(command, *, print_output: bool, output_callback): + ran.append(command) + return 0 + + _brew_upgrade_do_update_env(monkeypatch, tmp_path) + monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command) + # Version genuinely advanced after the upgrade. + monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "999.0.0") + + result = await update.do_update(print_output=False) + + assert result is update.UpdateResult.UPDATED + assert ran == [["brew", "update", "--quiet"], ["brew", "upgrade", "pythinker-code"]] + + +@pytest.mark.asyncio +async def test_do_update_brew_reports_failure_when_version_unchanged(monkeypatch, tmp_path): + """A no-op `brew upgrade` exits 0; the updater must NOT claim success when + the installed version did not advance to the target.""" + messages: list[str] = [] + + def fake_run_upgrade_command(command, *, print_output: bool, output_callback): + return 0 + + _brew_upgrade_do_update_env(monkeypatch, tmp_path) + monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command) + # brew exited 0 but the keg is still the old version (stale tap). + monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "0.37.0") + + result = await update.do_update(print_output=False, output_callback=messages.append) + + assert result is update.UpdateResult.FAILED + assert not any("Updated successfully" in m for m in messages) + assert any("still 0.37.0" in m for m in messages) + assert any("brew update" in m for m in messages) + + +@pytest.mark.asyncio +async def test_do_update_brew_continues_when_refresh_fails(monkeypatch, tmp_path): + """A failing `brew update` (e.g. transient network) must not block the + upgrade attempt — the upgrade still runs and can still succeed.""" + ran: list[list[str]] = [] + + def fake_run_upgrade_command(command, *, print_output: bool, output_callback): + ran.append(command) + return 1 if command == ["brew", "update", "--quiet"] else 0 + + _brew_upgrade_do_update_env(monkeypatch, tmp_path) + monkeypatch.setattr(update, "_run_upgrade_command", fake_run_upgrade_command) + monkeypatch.setattr(update, "_installed_homebrew_version", lambda: "999.0.0") + + result = await update.do_update(print_output=False) + + assert result is update.UpdateResult.UPDATED + assert ran == [["brew", "update", "--quiet"], ["brew", "upgrade", "pythinker-code"]] + + +def test_installed_homebrew_version_returns_max_installed(monkeypatch): + """Parses `brew list --versions`, returning the highest installed version.""" + + class FakeCompleted: + returncode = 0 + stdout = "pythinker-code 0.37.0 0.38.0\n" + + monkeypatch.setattr(update.subprocess, "run", lambda *a, **k: FakeCompleted()) + + assert update._installed_homebrew_version() == "0.38.0" + + +def test_installed_homebrew_version_returns_none_on_failure(monkeypatch): + """A non-zero `brew list` (formula not found) yields None, not a crash.""" + + class FakeCompleted: + returncode = 1 + stdout = "" + + monkeypatch.setattr(update.subprocess, "run", lambda *a, **k: FakeCompleted()) + + assert update._installed_homebrew_version() is None From 1f2ae4d20785a6f1fcebcb6781e9dfd5fbe9422e Mon Sep 17 00:00:00 2001 From: Mohamed Elkholy Date: Mon, 8 Jun 2026 12:34:00 -0400 Subject: [PATCH 2/2] fix(update): align brew list timeout with file's 5s convention `_installed_homebrew_version` used a 60s timeout for the local `brew list --versions` query; the file's other local package-state queries (dpkg-query, rpm) use 5s. A local DB read never needs 60s, and a shorter timeout fails fast if brew hangs. Addresses CodeRabbit review. --- src/pythinker_code/ui/shell/update.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pythinker_code/ui/shell/update.py b/src/pythinker_code/ui/shell/update.py index f01f4569..a5619d02 100644 --- a/src/pythinker_code/ui/shell/update.py +++ b/src/pythinker_code/ui/shell/update.py @@ -665,7 +665,7 @@ def _installed_homebrew_version() -> str | None: encoding="utf-8", errors="replace", env=get_clean_env(), - timeout=60, + timeout=5, ) except (OSError, subprocess.SubprocessError): logger.exception("Failed to read installed Homebrew version:")