Skip to content
Merged
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
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`[/red]"
)
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
152 changes: 152 additions & 0 deletions tests/test_readiness_messages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
"""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_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
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