From 22901c366d41342a08f30a08744b0fe382073fb9 Mon Sep 17 00:00:00 2001 From: Drew Cain Date: Thu, 11 Jun 2026 01:21:52 -0500 Subject: [PATCH] fix(cli): clean error on invalid share --expires-at and drop dead schemas Address review findings on `bm cloud share`: - `create` re-wrapped the `typer.Exit(1)` from `_parse_expires_at()` as a spurious "Unexpected error: 1" trailing line because the broad `except Exception` caught it (typer.Exit subclasses Exception). Parse and validate --expires-at before entering the try block so the error surfaces as a single clean message, and add the `except typer.Exit: raise` guard for parity with `update`/`revoke`. - Strengthen test_create_share_invalid_expires_at to assert "Unexpected error" is absent, which was masking the bug. - Drop unused PublicShareResponse/PublicShareListResponse schemas; responses are parsed as raw dicts, so the schemas were dead code. Co-Authored-By: Claude Signed-off-by: Drew Cain --- .../cli/commands/cloud/schemas.py | 27 ------------------- src/basic_memory/cli/commands/cloud/shares.py | 19 ++++++++----- tests/cli/test_share_commands.py | 4 +++ 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/basic_memory/cli/commands/cloud/schemas.py b/src/basic_memory/cli/commands/cloud/schemas.py index 08e8b2c7..b0389298 100644 --- a/src/basic_memory/cli/commands/cloud/schemas.py +++ b/src/basic_memory/cli/commands/cloud/schemas.py @@ -53,30 +53,3 @@ class BucketSnapshotRestoreResponse(BaseModel): restored: list[str] snapshot_version: str snapshot_id: UUID - - -class PublicShareResponse(BaseModel): - """Response model for a public share link. - - Mirrors PublicShareResponse in basic-memory-cloud - (apps/cloud/.../schemas/public_share_schemas.py). - """ - - id: UUID - token: str - project_name: str - note_permalink: str - note_external_id: str - enabled: bool - expires_at: datetime | None - share_url: str - view_count: int - last_viewed_at: datetime | None - created_at: datetime - - -class PublicShareListResponse(BaseModel): - """Response from listing public shares.""" - - shares: list[PublicShareResponse] - total: int diff --git a/src/basic_memory/cli/commands/cloud/shares.py b/src/basic_memory/cli/commands/cloud/shares.py index f7b49674..72185261 100644 --- a/src/basic_memory/cli/commands/cloud/shares.py +++ b/src/basic_memory/cli/commands/cloud/shares.py @@ -96,19 +96,22 @@ def create( bm cloud share create my-project notes/my-idea --expires-at 2025-12-31 """ + # Validate --expires-at before any async/API work so a parse error surfaces + # a single clean message and exits, rather than being re-wrapped by the broad + # handler below as "Unexpected error: 1" (typer.Exit subclasses Exception). + payload: dict = { + "project_name": project, + "note_permalink": permalink, + } + if expires_at is not None: + payload["expires_at"] = _parse_expires_at(expires_at) + async def _create(): try: config_manager = ConfigManager() config = config_manager.config host_url = config.cloud_host.rstrip("/") - payload: dict = { - "project_name": project, - "note_permalink": permalink, - } - if expires_at is not None: - payload["expires_at"] = _parse_expires_at(expires_at) - console.print("[blue]Creating share link...[/blue]") response = await make_api_request( @@ -122,6 +125,8 @@ def create( console.print("[green]Share link created successfully[/green]") _print_share_details(data) + except typer.Exit: + raise except SubscriptionRequiredError as e: console.print("\n[red]Subscription Required[/red]\n") console.print(f"[yellow]{e.args[0]}[/yellow]\n") diff --git a/tests/cli/test_share_commands.py b/tests/cli/test_share_commands.py index da9652ac..e5b860c3 100644 --- a/tests/cli/test_share_commands.py +++ b/tests/cli/test_share_commands.py @@ -142,6 +142,10 @@ class TestShareCreateCommand: assert result.exit_code == 1 assert "Invalid --expires-at" in result.stdout + # A parse error must produce a single clean message, not a + # spurious "Unexpected error: 1" from the broad handler + # re-catching typer.Exit. See issue #880 review. + assert "Unexpected error" not in result.stdout def test_create_share_note_not_found(self): runner = CliRunner()