diff --git a/src/basic_memory/cli/commands/cloud/core_commands.py b/src/basic_memory/cli/commands/cloud/core_commands.py index 586d0a9e..5a488a4b 100644 --- a/src/basic_memory/cli/commands/cloud/core_commands.py +++ b/src/basic_memory/cli/commands/cloud/core_commands.py @@ -71,6 +71,29 @@ def login(): ) raise typer.Exit(1) + # Trigger: the subscription-check call (/proxy/health) returned any error + # that is NOT a recognized subscription_required 403 — e.g. a 5xx while the + # tenant instance is still provisioning, a 403/401 whose body doesn't match + # the subscription_required shape, or a connection failure. + # Why: OAuth already succeeded and tokens are saved at this point, so a raw + # traceback (the old behavior) misleads users into thinking login itself + # failed. See #863. + # Outcome: surface a clean, actionable message and exit non-zero instead of + # crashing. make_api_request wraps every httpx error (status + transport) + # in CloudAPIError, so this single handler covers them all. + except CloudAPIError as e: + console.print("\n[yellow]Authenticated, but couldn't verify cloud access.[/yellow]\n") + console.print(f"[dim]{e}[/dim]\n") + console.print( + "Your workspace may still be provisioning. Wait a moment, then check with " + "[bold]bm cloud status[/bold] or retry [bold]bm cloud login[/bold].\n" + ) + console.print( + "[dim]If this persists, contact support at " + "[blue underline]https://basicmemory.com[/blue underline].[/dim]" + ) + raise typer.Exit(1) + run_with_cleanup(_login()) diff --git a/tests/cli/test_cloud_authentication.py b/tests/cli/test_cloud_authentication.py index e13f5336..34c96807 100644 --- a/tests/cli/test_cloud_authentication.py +++ b/tests/cli/test_cloud_authentication.py @@ -184,6 +184,43 @@ class TestLoginCommand: assert "Cloud authentication successful" in result.stdout assert "Cloud host ready: https://cloud.example.com" in result.stdout + def test_login_health_check_error_shows_clean_message(self, monkeypatch): + """Regression for #863: a non-subscription error from the post-login + /proxy/health check must produce a clean message, not a raw traceback. + + OAuth has already succeeded at this point; the tenant instance may still + be provisioning (5xx) or return some other non-subscription_required error. + """ + runner = CliRunner() + + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.CLIAuth", + lambda **_kwargs: _StubAuth(login_ok=True), + ) + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.get_cloud_config", + lambda: ("client_id", "domain", "https://cloud.example.com"), + ) + + async def fake_make_api_request(*_args, **_kwargs): + # e.g. tenant instance not ready yet -> proxy returns 503 + raise CloudAPIError("API request failed: 503 Service Unavailable", status_code=503) + + monkeypatch.setattr( + "basic_memory.cli.commands.cloud.core_commands.make_api_request", + fake_make_api_request, + ) + + result = runner.invoke(app, ["cloud", "login"]) + # Clean exit, no traceback leaking the exception class. + assert result.exit_code == 1 + assert result.exception is None or isinstance(result.exception, SystemExit) + # Collapse Rich's line-wrapping before matching multi-word phrases. + output = " ".join(result.stdout.split()) + assert "couldn't verify cloud access" in output + assert "bm cloud status" in output + assert "Traceback" not in output + def test_login_authentication_failure(self, monkeypatch): runner = CliRunner()