diff --git a/src/basic_memory/cli/auto_update.py b/src/basic_memory/cli/auto_update.py index 80435da54..739457e95 100644 --- a/src/basic_memory/cli/auto_update.py +++ b/src/basic_memory/cli/auto_update.py @@ -221,6 +221,38 @@ def _preload_lazy_console_modules() -> None: """ import rich._emoji_codes # noqa: F401 import typer.rich_utils # noqa: F401 + from rich.cells import cell_len + + # Trigger: rich defers its Unicode cell-width table (`rich._unicode_data. + # unicode`) until the first character it cannot measure with the + # ASCII fast path in `_cell_len`. + # Why: status messages echo captured `brew`/`uv` output, which carries + # non-ASCII characters (curly quotes, em dashes, warning glyphs), so the + # deferred import lands after the upgrade removed our files. Importing the + # module by name would hard-code a table version; calling `cell_len` uses + # rich's own resolution and honors UNICODE_VERSION like the print path does. + # Outcome: the table rich will reach for is resolved and cached up front. + cell_len("\u2500\u2018\u2713") + + +def print_update_status(console: Console, text: str, style: str) -> None: + """Print an update status line that cannot fail the command. + + Trigger: the line is printed after an in-place upgrade may already have + replaced this installation on disk. + Why: `_preload_lazy_console_modules` can only preload the deferred imports + we know about today, and rich/typer are free to add more. By the time this + prints, the upgrade has already succeeded -- a status line must never be + what turns it into a traceback and a non-zero exit. + Outcome: fall back to a plain, unstyled write that needs no new imports. + """ + try: + console.print(f"[{style}]{text}[/{style}]") + except Exception as exc: + logger.warning( + f"Rich console print failed after update, falling back to plain output: {exc}" + ) + print(text) def _save_last_checked_timestamp(config_manager: ConfigManager, checked_at: datetime) -> None: @@ -468,11 +500,11 @@ def maybe_run_periodic_auto_update( }: out = console or Console() if result.status == AutoUpdateStatus.UPDATED: - out.print(f"[green]{result.message}[/green]") + print_update_status(out, f"{result.message}", "green") elif result.status == AutoUpdateStatus.FAILED: error_detail = f" {result.error}" if result.error else "" - out.print(f"[yellow]{result.message}{error_detail}[/yellow]") + print_update_status(out, f"{result.message}{error_detail}", "yellow") elif result.message: - out.print(f"[cyan]{result.message}[/cyan]") + print_update_status(out, f"{result.message}", "cyan") return result diff --git a/src/basic_memory/cli/commands/update.py b/src/basic_memory/cli/commands/update.py index b0cff27c3..22dc52d32 100644 --- a/src/basic_memory/cli/commands/update.py +++ b/src/basic_memory/cli/commands/update.py @@ -4,7 +4,7 @@ from rich.console import Console from basic_memory.cli.app import app -from basic_memory.cli.auto_update import AutoUpdateStatus, run_auto_update +from basic_memory.cli.auto_update import AutoUpdateStatus, print_update_status, run_auto_update console = Console() @@ -22,19 +22,21 @@ def update( if result.status == AutoUpdateStatus.FAILED: detail = f" {result.error}" if result.error else "" - console.print(f"[red]{result.message or 'Update failed.'}{detail}[/red]") + print_update_status(console, f"{result.message or 'Update failed.'}{detail}", "red") raise typer.Exit(1) if result.status == AutoUpdateStatus.UPDATED: - console.print(f"[green]{result.message or 'Basic Memory updated successfully.'}[/green]") + print_update_status( + console, f"{result.message or 'Basic Memory updated successfully.'}", "green" + ) return if result.status == AutoUpdateStatus.UP_TO_DATE: - console.print(f"[green]{result.message or 'Basic Memory is up to date.'}[/green]") + print_update_status(console, f"{result.message or 'Basic Memory is up to date.'}", "green") return if result.status == AutoUpdateStatus.UPDATE_AVAILABLE: - console.print(f"[cyan]{result.message or 'Update available.'}[/cyan]") + print_update_status(console, f"{result.message or 'Update available.'}", "cyan") return - console.print(f"[dim]{result.message or 'No update action was performed.'}[/dim]") + print_update_status(console, f"{result.message or 'No update action was performed.'}", "dim") diff --git a/tests/cli/test_auto_update.py b/tests/cli/test_auto_update.py index e9f85cdab..65e3e2503 100644 --- a/tests/cli/test_auto_update.py +++ b/tests/cli/test_auto_update.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re import subprocess import sys import urllib.error @@ -21,6 +22,7 @@ _check_homebrew_update_available, _is_interactive_session, _preload_lazy_console_modules, + print_update_status, detect_install_source, maybe_run_periodic_auto_update, run_auto_update, @@ -387,6 +389,72 @@ def test_preload_lazy_console_modules_imports_deferred_modules(monkeypatch): assert "typer.rich_utils" in sys.modules +class _UpgradedAwayFinder: + """Stand-in for the deleted install prefix: nothing new can be imported.""" + + def find_spec(self, fullname, path=None, target=None): # noqa: D102 + raise ModuleNotFoundError(f"No module named {fullname!r}") + + +def _cool_deferred_width_table(monkeypatch) -> None: + """Return rich to the cold state a freshly started process is in. + + rich caches the Unicode cell-width table aggressively, and earlier tests in + this session will already have warmed it -- without this the regression test + below passes whether or not the table was preloaded. + """ + import rich.cells + + for cached in ("cached_cell_len", "get_character_cell_size"): + clear = getattr(getattr(rich.cells, cached, None), "cache_clear", None) + if clear is not None: + clear() + + # rich >= 14.2 splits the tables into rich._unicode_data.unicode; + # older versions inline them and have nothing to unload. + unicode_data = sys.modules.get("rich._unicode_data") + clear_load = getattr(getattr(unicode_data, "load", None), "cache_clear", None) + if clear_load is not None: + clear_load() + for name in [n for n in sys.modules if n.startswith("rich._unicode_data.unicode")]: + monkeypatch.delitem(sys.modules, name, raising=False) + + +def test_status_message_survives_upgraded_away_install(monkeypatch): + # Regression (#1316): `brew upgrade` removes the running install's files, so + # the status message printed afterwards must not need any new import. The + # message is long and non-ASCII on purpose -- that is what makes rich wrap + # the line and reach for the deferred cell-width table. + output = StringIO() + console = Console(width=40, file=output) + console.print("warm up the print path") + _cool_deferred_width_table(monkeypatch) + + _preload_lazy_console_modules() + monkeypatch.setattr(sys, "meta_path", [_UpgradedAwayFinder(), *sys.meta_path]) + + # Deliberately not print_update_status: its fallback would mask the bug. + console.print( + "[red]Automatic update failed. Error: could not link \u2018basic-memory\u2019 " + "\u2014 the files were replaced while running. " + "detail " * 12 + "[/red]" + ) + + # rich wrapped and styled the line; normalize before checking the content. + rendered = re.sub(r"\x1b\[[0-9;]*m", "", output.getvalue()) + assert "could not link" in " ".join(rendered.split()) + + +def test_print_update_status_falls_back_to_plain_output(capsys): + # A status line must never be what fails a command whose upgrade succeeded. + class ExplodingConsole: + def print(self, *args, **kwargs): + raise ModuleNotFoundError("No module named 'rich._unicode_data.unicode17-0-0'") + + print_update_status(cast(Console, ExplodingConsole()), "Basic Memory was updated.", "green") + + assert "Basic Memory was updated." in capsys.readouterr().out + + def test_homebrew_outdated_triggers_upgrade(monkeypatch, tmp_path): config = _base_config(tmp_path) manager = StubConfigManager(config)