Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
3 changes: 2 additions & 1 deletion src/pieces/core/onboarding.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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"
Expand Down
5 changes: 4 additions & 1 deletion src/pieces/core/open_command.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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):
Expand Down
56 changes: 56 additions & 0 deletions src/pieces/readiness_messages.py
Original file line number Diff line number Diff line change
@@ -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.<x>`` 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`"
)
3 changes: 2 additions & 1 deletion src/pieces/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
146 changes: 146 additions & 0 deletions tests/test_readiness_messages.py
Original file line number Diff line number Diff line change
@@ -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."
)
Loading