Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 35 additions & 3 deletions src/basic_memory/cli/auto_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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<version>`) 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:
Expand Down Expand Up @@ -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
14 changes: 8 additions & 6 deletions src/basic_memory/cli/commands/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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")
68 changes: 68 additions & 0 deletions tests/cli/test_auto_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from __future__ import annotations

import json
import re
import subprocess
import sys
import urllib.error
Expand All @@ -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,
Expand Down Expand Up @@ -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<version>;
# 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)
Expand Down
Loading