From a368d06fd29cf6410b184343dc11c5c13506e81a Mon Sep 17 00:00:00 2001 From: phernandez Date: Sun, 1 Mar 2026 20:24:11 -0600 Subject: [PATCH] fix: improve cloud CLI status and error messages - Simplify `bm cloud status` output: remove verbose health check details (status/version/timestamp), show simple "Cloud connected" / "Cloud not connected" message instead - Improve `bm reindex --project` error for cloud projects: distinguish between "project not found" and "project is cloud-only" with a helpful message explaining reindexing is a local operation - Improve `bm project list` cloud error message: show the actual error and soften the credentials suggestion - Add tests for cloud status command (5 tests) Co-Authored-By: Claude Opus 4.6 Signed-off-by: phernandez --- .../cli/commands/cloud/core_commands.py | 52 ++---- src/basic_memory/cli/commands/db.py | 13 +- src/basic_memory/cli/commands/project.py | 7 +- tests/cli/test_cloud_status.py | 148 ++++++++++++++++++ 4 files changed, 181 insertions(+), 39 deletions(-) create mode 100644 tests/cli/test_cloud_status.py diff --git a/src/basic_memory/cli/commands/cloud/core_commands.py b/src/basic_memory/cli/commands/cloud/core_commands.py index 9682eab7..2dc2e25e 100644 --- a/src/basic_memory/cli/commands/cloud/core_commands.py +++ b/src/basic_memory/cli/commands/cloud/core_commands.py @@ -85,13 +85,13 @@ def logout(): @cloud_app.command("status") def status() -> None: - """Check cloud authentication state and cloud instance health.""" + """Check cloud authentication and connection status.""" config_manager = ConfigManager() config = config_manager.load_config() auth = CLIAuth(client_id=config.cloud_client_id, authkit_domain=config.cloud_domain) tokens = auth.load_tokens() - console.print("[bold blue]Cloud Authentication Status[/bold blue]") + console.print("[bold blue]Cloud Status[/bold blue]") console.print(f" Host: {config.cloud_host}") console.print( f" API Key: {'[green]configured[/green]' if config.cloud_api_key else '[yellow]not set[/yellow]'}" @@ -99,17 +99,12 @@ def status() -> None: oauth_status = "[yellow]not logged in[/yellow]" if tokens: - oauth_status = ( - "[green]token valid[/green]" - if auth.is_token_valid(tokens) - else "[yellow]token expired[/yellow]" - ) + if auth.is_token_valid(tokens): + oauth_status = "[green]token valid[/green]" + else: + oauth_status = "[yellow]token expired[/yellow]" console.print(f" OAuth: {oauth_status}") - # Get cloud configuration - _, _, host_url = get_cloud_config() - host_url = host_url.rstrip("/") - has_credentials = bool(config.cloud_api_key) or tokens is not None if not has_credentials: console.print( @@ -117,33 +112,20 @@ def status() -> None: ) return + # Quick connection check — just verify we can reach the cloud + _, _, host_url = get_cloud_config() + host_url = host_url.rstrip("/") + try: - console.print("\n[blue]Checking cloud instance health...[/blue]") - - # Make API request to check health - response = run_with_cleanup(make_api_request(method="GET", url=f"{host_url}/proxy/health")) - - health_data = response.json() - - console.print("[green]Cloud instance is healthy[/green]") - - # Display status details - if "status" in health_data: - console.print(f" Status: {health_data['status']}") - if "version" in health_data: - console.print(f" Version: {health_data['version']}") - if "timestamp" in health_data: - console.print(f" Timestamp: {health_data['timestamp']}") - - console.print("\n[dim]To sync projects, use: bm project bisync --name [/dim]") - - except CloudAPIError as e: - console.print(f"[yellow]Cloud health check failed: {e}[/yellow]") + run_with_cleanup(make_api_request(method="GET", url=f"{host_url}/proxy/health")) + console.print("\n[green]Cloud connected[/green]") + except CloudAPIError: + console.print("\n[yellow]Cloud not connected[/yellow]") console.print( - "[dim]Try re-authenticating with 'bm cloud login' or setting API key with 'bm cloud api-key save'.[/dim]" + "[dim]Try re-authenticating with 'bm cloud login' or 'bm cloud api-key save'.[/dim]" ) - except Exception as e: - console.print(f"[yellow]Unexpected health check error: {e}[/yellow]") + except Exception: + console.print("\n[yellow]Cloud not connected[/yellow]") @cloud_app.command("setup") diff --git a/src/basic_memory/cli/commands/db.py b/src/basic_memory/cli/commands/db.py index 3e2a806f..08595511 100644 --- a/src/basic_memory/cli/commands/db.py +++ b/src/basic_memory/cli/commands/db.py @@ -11,7 +11,7 @@ from sqlalchemy.exc import OperationalError from basic_memory import db from basic_memory.cli.app import app from basic_memory.cli.commands.command_utils import run_with_cleanup -from basic_memory.config import ConfigManager +from basic_memory.config import ConfigManager, ProjectMode from basic_memory.repository import ProjectRepository from basic_memory.services.initialization import reconcile_projects_with_config from basic_memory.sync.sync_service import get_sync_service @@ -169,7 +169,16 @@ async def _reindex(app_config, search: bool, embeddings: bool, project: str | No if project: projects = [p for p in projects if p.name == project] if not projects: - console.print(f"[red]Project '{project}' not found.[/red]") + # Check if it's a cloud-only project — those can't be reindexed locally + project_mode = app_config.get_project_mode(project) + if project_mode == ProjectMode.CLOUD: + console.print( + f"[yellow]Project '{project}' is a cloud project.[/yellow]\n" + "Reindexing is a local operation — cloud projects are " + "indexed on the server." + ) + else: + console.print(f"[red]Project '{project}' not found.[/red]") raise typer.Exit(1) for proj in projects: diff --git a/src/basic_memory/cli/commands/project.py b/src/basic_memory/cli/commands/project.py index 93a697a8..1f6d95a9 100644 --- a/src/basic_memory/cli/commands/project.py +++ b/src/basic_memory/cli/commands/project.py @@ -228,8 +228,11 @@ def list_projects( console.print(table) if cloud_error is not None: console.print( - "[yellow]Cloud project discovery failed. " - "Showing local projects only. Run 'bm cloud login' or 'bm cloud api-key save '.[/yellow]" + f"[yellow]Cloud project discovery failed: {cloud_error}[/yellow]" + ) + console.print( + "[dim]Showing local projects only. " + "Run 'bm cloud login' or 'bm cloud api-key save ' if this is a credentials issue.[/dim]" ) except Exception as e: console.print(f"[red]Error listing projects: {str(e)}[/red]") diff --git a/tests/cli/test_cloud_status.py b/tests/cli/test_cloud_status.py new file mode 100644 index 00000000..7d2ea2f8 --- /dev/null +++ b/tests/cli/test_cloud_status.py @@ -0,0 +1,148 @@ +"""Tests for cloud status command.""" + +from __future__ import annotations + +import time + +import httpx +import pytest +from typer.testing import CliRunner + +from basic_memory.cli.app import app +from basic_memory.cli.commands.cloud.api_client import CloudAPIError + + +# --- status command integration tests --- + + +class _FakeTokens: + """Provides canned token data for CLIAuth stubs.""" + + @classmethod + def valid(cls) -> dict: + return { + "access_token": "fake-access-token", + "refresh_token": "rt_test", + "expires_at": int(time.time()) + 3600, + } + + @classmethod + def expired(cls) -> dict: + return { + "access_token": "fake-access-token", + "refresh_token": "rt_test", + "expires_at": int(time.time()) - 3600, + } + + +def _patch_status_deps(monkeypatch, *, tokens=None, api_side_effect=None): + """Patch ConfigManager and CLIAuth for the status command.""" + + class FakeConfig: + cloud_client_id = "cid" + cloud_domain = "https://auth.example.com" + cloud_host = "https://cloud.example.com" + cloud_api_key = "bmc_test123" + + class FakeConfigManager: + config = FakeConfig() + + def load_config(self): + return self.config + + class FakeAuth: + def __init__(self, **_kwargs): + pass + + def load_tokens(self): + return tokens + + def is_token_valid(self, t): + return t.get("expires_at", 0) > time.time() + + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.ConfigManager", FakeConfigManager + ) + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.CLIAuth", FakeAuth + ) + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.get_cloud_config", + lambda: ("cid", "domain", "https://cloud.example.com"), + ) + + if api_side_effect is None: + # Default: cloud is reachable + async def _ok(*_a, **_kw): + return httpx.Response(200, json={"status": "ok"}) + + api_side_effect = _ok + + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.make_api_request", api_side_effect + ) + + +class TestStatusCommand: + def test_status_connected(self, monkeypatch): + _patch_status_deps(monkeypatch, tokens=_FakeTokens.valid()) + runner = CliRunner() + result = runner.invoke(app, ["cloud", "status"]) + + assert result.exit_code == 0 + assert "Cloud Status" in result.stdout + assert "cloud.example.com" in result.stdout + assert "token valid" in result.stdout + assert "Cloud connected" in result.stdout + + def test_status_expired_token(self, monkeypatch): + _patch_status_deps(monkeypatch, tokens=_FakeTokens.expired()) + runner = CliRunner() + result = runner.invoke(app, ["cloud", "status"]) + + assert result.exit_code == 0 + assert "token expired" in result.stdout + + def test_status_no_credentials(self, monkeypatch): + _patch_status_deps(monkeypatch, tokens=None) + + # Also clear the API key so there are no credentials at all + class FakeConfig: + cloud_client_id = "cid" + cloud_domain = "https://auth.example.com" + cloud_host = "https://cloud.example.com" + cloud_api_key = "" + + class FakeConfigManager: + config = FakeConfig() + + def load_config(self): + return self.config + + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.ConfigManager", FakeConfigManager + ) + + runner = CliRunner() + result = runner.invoke(app, ["cloud", "status"]) + + assert result.exit_code == 0 + assert "No cloud credentials found" in result.stdout + + @pytest.mark.parametrize( + "exc", + [ + CloudAPIError("connection refused"), + Exception("network timeout"), + ], + ) + def test_status_cloud_not_connected(self, monkeypatch, exc): + async def _fail(*_a, **_kw): + raise exc + + _patch_status_deps(monkeypatch, tokens=_FakeTokens.valid(), api_side_effect=_fail) + runner = CliRunner() + result = runner.invoke(app, ["cloud", "status"]) + + assert result.exit_code == 0 + assert "Cloud not connected" in result.stdout