From b6970c2fe5b90e04c49cd24c86bf237929ef89c9 Mon Sep 17 00:00:00 2001 From: Gavin Date: Thu, 28 May 2026 13:13:59 -0700 Subject: [PATCH 1/2] feat(cli): clarify PiecesOS readiness wording --- src/pieces/core/onboarding.py | 3 +- src/pieces/core/open_command.py | 5 +- src/pieces/readiness_messages.py | 56 ++++++++++++ src/pieces/settings.py | 3 +- tests/test_readiness_messages.py | 146 +++++++++++++++++++++++++++++++ 5 files changed, 210 insertions(+), 3 deletions(-) create mode 100644 src/pieces/readiness_messages.py create mode 100644 tests/test_readiness_messages.py diff --git a/src/pieces/core/onboarding.py b/src/pieces/core/onboarding.py index 2866bf55..5f28954b 100644 --- a/src/pieces/core/onboarding.py +++ b/src/pieces/core/onboarding.py @@ -12,6 +12,7 @@ import sys from pieces.core.cli_loop import run_command, extract_text +from pieces.readiness_messages import pieces_os_not_reachable from pieces.settings import Settings from pieces.urls import URLs @@ -266,7 +267,7 @@ def onboarding_command(**kwargs): Settings.logger.print("Whenever you want to exit the onboarding flow type `exit`.") if not Settings.pieces_client.open_pieces_os(): - Settings.logger.print("❌ PiecesOS is not running") + Settings.logger.print(Markdown(pieces_os_not_reachable())) Settings.logger.print( Markdown( "**PiecesOS** is a **required** background service" diff --git a/src/pieces/core/open_command.py b/src/pieces/core/open_command.py index 3c26a6c6..3dcd39a4 100644 --- a/src/pieces/core/open_command.py +++ b/src/pieces/core/open_command.py @@ -1,6 +1,9 @@ +from rich.markdown import Markdown + from pieces.urls import URLs from pieces.settings import Settings from pieces.copilot.ltm import enable_ltm +from pieces.readiness_messages import pieces_os_not_reachable def open_command(**kwargs): from pieces._vendor.pieces_os_client.models.inactive_os_server_applet import InactiveOSServerApplet @@ -14,7 +17,7 @@ def open_command(**kwargs): health = Settings.pieces_client.open_pieces_os() if (drive or copilot or settings or ltm) and not health: - Settings.logger.print("PiecesOS is not running") + Settings.logger.print(Markdown(pieces_os_not_reachable())) return if ltm and enable_ltm(auto_enable=True): diff --git a/src/pieces/readiness_messages.py b/src/pieces/readiness_messages.py new file mode 100644 index 00000000..fdeed7d4 --- /dev/null +++ b/src/pieces/readiness_messages.py @@ -0,0 +1,56 @@ +"""User-facing PiecesOS readiness messages. + +Lives at top-level (NOT under ``pieces.core``) to avoid a circular import +through ``pieces.settings``. Every module under ``pieces.core`` imports +``Settings`` at module top, so ``pieces.settings -> pieces.core.`` would +re-enter a partially-loaded ``pieces.settings`` and raise ``ImportError``. + +Constraints (do not relax without re-review): + +- stdlib-only imports. Do not import ``Settings``. Do not import anything + under ``pieces.core``. Do not import ``pieces.urls`` (it is safe today + but keeping zero ``pieces.*`` imports here removes the entire cycle + surface). +- Do not claim install state. The CLI cannot tell "not installed" apart + from "installed but not running" from ``open_pieces_os()`` alone. +- Do not mention ``.port.txt``. The CLI does not read it. +- Always end with the same four next steps: ``pieces install``, + ``pieces open``, retry, share ``pieces version`` output with support. +- Long-form is multi-line Markdown for + ``Settings.logger.print(Markdown(...))``. +- Short-form is a single-line Rich-markup string safe for + ``Progress.update(..., description=...)``. +""" + + +def pieces_os_not_reachable() -> str: + """Return the long-form canonical readiness message as Markdown. + + Intended to be wrapped in ``rich.markdown.Markdown`` before being + handed to ``Settings.logger.print``. + """ + return ( + "**PiecesOS is not reachable from the CLI.**\n\n" + "**Possible reasons:**\n\n" + "- PiecesOS may need to be installed or updated\n" + "- PiecesOS may not be running\n" + "- PiecesOS may still be starting up\n\n" + "**Try:**\n\n" + "1. Install or update PiecesOS: `pieces install`\n" + "2. Launch PiecesOS: `pieces open`\n" + "3. If you just installed it, wait a few seconds and retry your command\n" + "4. If the problem persists, share the output of `pieces version` " + "with Pieces support\n" + ) + + +def pieces_os_not_reachable_short() -> str: + """Return the one-line Rich-markup readiness message. + + Safe to pass directly to ``rich.progress.Progress.update`` as the + ``description`` keyword; must remain single-line (no ``\\n``). + """ + return ( + "[red]Could not reach PiecesOS - try `pieces install`, " + "then `pieces open`" + ) diff --git a/src/pieces/settings.py b/src/pieces/settings.py index 5f0e3588..7985bf05 100644 --- a/src/pieces/settings.py +++ b/src/pieces/settings.py @@ -20,6 +20,7 @@ from pieces.gui import ( print_version_details, ) +from pieces.readiness_messages import pieces_os_not_reachable_short from pieces.urls import URLs from pieces.config.constants import PIECES_DATA_DIR from pieces.config.managers import ( @@ -183,5 +184,5 @@ def open_pieces_widget(cls): progress.update(pos_task, visible=False) return True - progress.update(pos_task, description="[red]PiecesOS is not installed") + progress.update(pos_task, description=pieces_os_not_reachable_short()) return False diff --git a/tests/test_readiness_messages.py b/tests/test_readiness_messages.py new file mode 100644 index 00000000..2237706a --- /dev/null +++ b/tests/test_readiness_messages.py @@ -0,0 +1,146 @@ +"""Tests for ``src/pieces/readiness_messages.py``. + +These tests pin the wording contract of the two readiness helpers, +guard against a circular-import regression between ``pieces.settings`` +and the ``pieces.core`` submodules that import ``Settings`` at module +top, and assert the three in-scope call sites do not reintroduce the +old vague/overclaiming strings. +""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +from pieces.readiness_messages import ( + pieces_os_not_reachable, + pieces_os_not_reachable_short, +) + + +REPO_ROOT = Path(__file__).resolve().parents[1] + +# The three in-scope call sites that were updated to use the readiness +# helpers. Out-of-scope locations (vendored SDK, MCP gateway) are +# intentionally not listed here because they live behind a different +# review boundary. +EDITED_CALL_SITE_PATHS = ( + "src/pieces/core/open_command.py", + "src/pieces/core/onboarding.py", + "src/pieces/settings.py", +) + +# Overclaiming legacy strings that the readiness helpers replaced. +# These must not reappear in any of the edited call sites. +FORBIDDEN_LEGACY_STRINGS = ( + "PiecesOS is not running", + "PiecesOS is not installed", +) + + +class TestPiecesOSNotReachable: + """Wording assertions for the long-form Markdown helper.""" + + def test_contains_pieces_install(self): + assert "pieces install" in pieces_os_not_reachable() + + def test_contains_pieces_open(self): + assert "pieces open" in pieces_os_not_reachable() + + def test_contains_pieces_version(self): + assert "pieces version" in pieces_os_not_reachable() + + def test_contains_retry_case_insensitive(self): + assert "retry" in pieces_os_not_reachable().lower() + + def test_does_not_mention_port_txt(self): + assert ".port.txt" not in pieces_os_not_reachable() + + def test_does_not_overclaim_install_state(self): + # The CLI cannot distinguish "not installed" from + # "installed but not running" via open_pieces_os() alone. + assert "is not installed" not in pieces_os_not_reachable() + + +class TestPiecesOSNotReachableShort: + """Wording and shape assertions for the short-form Rich-markup helper.""" + + def test_returns_single_line(self): + # Must be safe to drop into Rich Progress description=... + assert "\n" not in pieces_os_not_reachable_short() + + def test_does_not_mention_port_txt(self): + assert ".port.txt" not in pieces_os_not_reachable_short() + + def test_does_not_overclaim_install_state(self): + assert "is not installed" not in pieces_os_not_reachable_short() + + def test_contains_pieces_install(self): + assert "pieces install" in pieces_os_not_reachable_short() + + def test_contains_pieces_open(self): + assert "pieces open" in pieces_os_not_reachable_short() + + +def test_import_smoke_no_circular(): + """Regression guard: importing settings, the two edited core call + sites, and the new helper in a single fresh interpreter must not + raise ``ImportError`` from a circular import. + + Runs in a subprocess so module caching from the test session cannot + mask a real cycle. + """ + src_dir = REPO_ROOT / "src" + + env = os.environ.copy() + existing_pythonpath = env.get("PYTHONPATH", "") + env["PYTHONPATH"] = ( + f"{src_dir}{os.pathsep}{existing_pythonpath}" + if existing_pythonpath + else str(src_dir) + ) + + result = subprocess.run( + [ + sys.executable, + "-c", + ( + "import pieces.settings, pieces.core.open_command, " + "pieces.core.onboarding, pieces.readiness_messages" + ), + ], + capture_output=True, + text=True, + env=env, + ) + + assert result.returncode == 0, ( + f"circular-import regression detected\n" + f"stdout:\n{result.stdout}\n" + f"stderr:\n{result.stderr}" + ) + + +@pytest.mark.parametrize("relative_path", EDITED_CALL_SITE_PATHS) +@pytest.mark.parametrize("forbidden", FORBIDDEN_LEGACY_STRINGS) +def test_edited_call_sites_drop_legacy_overclaim_strings( + relative_path: str, forbidden: str +) -> None: + """Regression guard: the three call sites we replaced with the + readiness helpers must not reintroduce the overclaiming legacy + strings. + + Scope: only the three in-scope edited files. Vendored SDK and MCP + gateway sites live behind a different review boundary and are not + asserted here. + """ + file_path = REPO_ROOT / relative_path + contents = file_path.read_text(encoding="utf-8") + + assert forbidden not in contents, ( + f"{relative_path} reintroduced the overclaiming legacy string " + f"{forbidden!r}. Use the helpers in " + f"src/pieces/readiness_messages.py instead." + ) From 2b377fd0ebb5728cb31b7d9fcc28bea52b085707 Mon Sep 17 00:00:00 2001 From: Gavin Date: Thu, 28 May 2026 15:07:16 -0700 Subject: [PATCH 2/2] fix(cli): close readiness progress rich tag --- src/pieces/readiness_messages.py | 2 +- tests/test_readiness_messages.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/pieces/readiness_messages.py b/src/pieces/readiness_messages.py index fdeed7d4..68b385b7 100644 --- a/src/pieces/readiness_messages.py +++ b/src/pieces/readiness_messages.py @@ -52,5 +52,5 @@ def pieces_os_not_reachable_short() -> str: """ return ( "[red]Could not reach PiecesOS - try `pieces install`, " - "then `pieces open`" + "then `pieces open`[/red]" ) diff --git a/tests/test_readiness_messages.py b/tests/test_readiness_messages.py index 2237706a..bad33469 100644 --- a/tests/test_readiness_messages.py +++ b/tests/test_readiness_messages.py @@ -83,6 +83,12 @@ def test_contains_pieces_install(self): def test_contains_pieces_open(self): assert "pieces open" in pieces_os_not_reachable_short() + def test_closes_red_markup_tag(self): + msg = pieces_os_not_reachable_short() + assert msg.startswith("[red]") + assert msg.endswith("[/red]") + assert "\n" not in msg + def test_import_smoke_no_circular(): """Regression guard: importing settings, the two edited core call