From 21deefddf9e3fe592d59cea6fb0d1ba97c8a46f5 Mon Sep 17 00:00:00 2001 From: elkaix Date: Wed, 1 Jul 2026 12:36:53 -0400 Subject: [PATCH] fix: clear stale update-success notice after restarting into an old install Homebrew installs can restart into the same pre-update binary when the formula bump doesn't actually land. The persisted "Restart to apply" status survived that restart forever because it only checked job state and target version, not which process wrote it. Scope the notice to the process that recorded the successful update (pid match) so a restart into a still-old version falls back to "Update available" instead of repeating an already-ineffective restart prompt. --- CHANGELOG.md | 3 ++ src/pythinker_code/ui/shell/__init__.py | 17 ++------- .../ui/shell/update_orchestrator.py | 10 +++++ tests/ui_and_conv/test_shell_welcome_info.py | 37 ++++++++++++++++++- tests/ui_and_conv/test_silent_auto_update.py | 26 ++++++++++--- 5 files changed, 72 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b35af0d..a956a1e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- Fix stale update-success notices so restarting into an older Homebrew install + shows `/update` again instead of a permanent "Restart to apply" banner. + ## 0.55.0 (2026-06-30) - **Local-model reliability fixes for compaction, Qwen3 reasoning, and stuck loops.** diff --git a/src/pythinker_code/ui/shell/__init__.py b/src/pythinker_code/ui/shell/__init__.py index ae4c8545..fcb34d08 100644 --- a/src/pythinker_code/ui/shell/__init__.py +++ b/src/pythinker_code/ui/shell/__init__.py @@ -81,9 +81,9 @@ ) from pythinker_code.ui.shell.update_orchestrator import ( SMOKE_CHECK_FAILED_PREFIX, - UpdateJobState, read_update_status, run_update_job, + update_restart_pending, ) from pythinker_code.ui.shell.visualize import ( ApprovalPromptDelegate, @@ -2213,12 +2213,7 @@ def _compute_update_notice(self) -> str | None: # surface that here instead of telling the user to re-run an update that # has already landed. status = read_update_status() - installed = ( - status is not None - and status.state is UpdateJobState.UPDATED - and status.target_version == target - ) - if installed and not self._installed_update_smoke_check_failed(): + if update_restart_pending(status, target): text = self._installed_update_restart_notice() else: text = f"↑ Update available — v{target} · /update" @@ -2434,13 +2429,7 @@ def _chip(markup: str, style: str) -> Text: # ponytail: mirror _compute_update_notice — if /update already landed # this session, show the restart line instead of "Update available". status = read_update_status() - installed = ( - status is not None - and status.state is UpdateJobState.UPDATED - and status.target_version == update_target - and not (status.message and status.message.startswith(SMOKE_CHECK_FAILED_PREFIX)) - ) - if installed: + if update_restart_pending(status, update_target): from pythinker_code.constant import VERSION as current_version return _chip( diff --git a/src/pythinker_code/ui/shell/update_orchestrator.py b/src/pythinker_code/ui/shell/update_orchestrator.py index 98236e57..e20afe60 100644 --- a/src/pythinker_code/ui/shell/update_orchestrator.py +++ b/src/pythinker_code/ui/shell/update_orchestrator.py @@ -223,6 +223,16 @@ def read_update_status() -> UpdateJobStatus | None: ) +def update_restart_pending(status: UpdateJobStatus | None, target_version: str | None) -> bool: + return ( + status is not None + and status.state is UpdateJobState.UPDATED + and status.target_version == target_version + and status.pid == os.getpid() + and not (status.message and status.message.startswith(SMOKE_CHECK_FAILED_PREFIX)) + ) + + def _optional_str(value: object) -> str | None: return value if isinstance(value, str) else None diff --git a/tests/ui_and_conv/test_shell_welcome_info.py b/tests/ui_and_conv/test_shell_welcome_info.py index 282bd9fd..a5ae94d0 100644 --- a/tests/ui_and_conv/test_shell_welcome_info.py +++ b/tests/ui_and_conv/test_shell_welcome_info.py @@ -1,4 +1,5 @@ import io +import os from rich.console import Console from rich.text import Text @@ -95,7 +96,7 @@ def test_welcome_banner_chip_shows_restart_after_successful_update(monkeypatch): result="ok", message=None, log_path="/dev/null", - pid=123, + pid=os.getpid(), ), ) @@ -109,6 +110,38 @@ def test_welcome_banner_chip_shows_restart_after_successful_update(monkeypatch): assert "Update available" not in text +def test_welcome_banner_chip_shows_update_after_restart_if_version_is_still_old(monkeypatch): + """A persisted success from a previous process must not leave old installs + stuck on 'Restart to apply' forever.""" + from pythinker_code.ui.shell.update_orchestrator import UpdateJobState, UpdateJobStatus + + monkeypatch.setattr(shell_module, "consume_whats_new", lambda: None) + monkeypatch.setattr(shell_module, "welcome_update_target", lambda: "0.51.0") + monkeypatch.setattr( + shell_module, + "read_update_status", + lambda: UpdateJobStatus( + job_id="test", + state=UpdateJobState.UPDATED, + started_at=1.0, + finished_at=2.0, + current_version="0.50.0", + target_version="0.51.0", + result="ok", + message=None, + log_path="/dev/null", + pid=os.getpid() + 1, + ), + ) + + chip = shell_module._welcome_banner_chip() + + assert chip is not None + text = chip.plain + assert "Update available" in text + assert "Restart to apply" not in text + + def test_welcome_banner_chip_shows_update_if_smoke_check_failed(monkeypatch): """If the post-update smoke check failed, keep showing 'Update available' — the install didn't land cleanly.""" @@ -129,7 +162,7 @@ def test_welcome_banner_chip_shows_update_if_smoke_check_failed(monkeypatch): result="smoke_failed", message="Updated, but smoke check did not pass: binary not executable", log_path="/dev/null", - pid=123, + pid=os.getpid(), ), ) diff --git a/tests/ui_and_conv/test_silent_auto_update.py b/tests/ui_and_conv/test_silent_auto_update.py index 1fb88079..662d4f85 100644 --- a/tests/ui_and_conv/test_silent_auto_update.py +++ b/tests/ui_and_conv/test_silent_auto_update.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import os import time from pathlib import Path from types import SimpleNamespace @@ -49,7 +50,7 @@ async def test_silent_update_success_refreshes_persistent_notice_not_toast( monkeypatch.setattr(shell_module, "_detect_upgrade_command", lambda: ["pip"]) async def fake_job(**kw): - assert kw["print_output"] is False + assert not kw["print_output"] assert kw["source"] == "startup-auto" return UpdateResult.UPDATED @@ -64,7 +65,8 @@ async def fake_job(**kw): assert _toasts == [] assert invalidated == [True] - assert shell._update_notice_cache == (0.0, None) + expected_cache = (0.0, None) + assert shell._update_notice_cache == expected_cache @pytest.mark.asyncio @@ -195,7 +197,7 @@ async def fake_job(**kw): monkeypatch.setattr(shell_module, "run_update_job", fake_job) await shell._silent_auto_update() - assert called is False + assert not called assert _toasts == [] @@ -317,10 +319,15 @@ def test_auto_update_override_reason_none_when_config_decides(monkeypatch): assert update_policy.auto_update_override_reason() is None -def _updated_status(target: str, *, message: str = "updated"): +def _updated_status(target: str, *, message: str = "updated", pid: int | None = None): from pythinker_code.ui.shell.update_orchestrator import UpdateJobState - return SimpleNamespace(state=UpdateJobState.UPDATED, target_version=target, message=message) + return SimpleNamespace( + state=UpdateJobState.UPDATED, + target_version=target, + message=message, + pid=os.getpid() if pid is None else pid, + ) def test_update_notice_available_points_to_slash_update(runtime, tmp_path, monkeypatch): @@ -354,6 +361,15 @@ def test_update_notice_installed_but_smoke_failed_falls_back(runtime, tmp_path, assert shell._compute_update_notice() == "↑ Update available — v9.9.9 · /update" +def test_update_notice_previous_process_success_falls_back(runtime, tmp_path, monkeypatch): + shell = _make_shell(runtime, tmp_path) + monkeypatch.setattr(shell_module, "welcome_update_target", lambda: "9.9.9") + monkeypatch.setattr(shell_module, "ascii_glyphs_enabled", lambda: False) + status = _updated_status("9.9.9", pid=os.getpid() + 1) + monkeypatch.setattr(shell_module, "read_update_status", lambda: status) + assert shell._compute_update_notice() == "↑ Update available — v9.9.9 · /update" + + def test_update_notice_none_when_up_to_date(runtime, tmp_path, monkeypatch): shell = _make_shell(runtime, tmp_path) monkeypatch.setattr(shell_module, "welcome_update_target", lambda: None)