Skip to content

Commit cce4e6b

Browse files
committed
fix(cli): address cloud review feedback
Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 0c1d86e commit cce4e6b

File tree

6 files changed

+107
-14
lines changed

6 files changed

+107
-14
lines changed

src/basic_memory/cli/commands/cloud/cloud_utils.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ async def create_cloud_project(
6969
7070
Args:
7171
project_name: Name of project to create
72+
workspace: Optional workspace override for tenant-scoped project creation
7273
7374
Returns:
7475
CloudProjectCreateResponse with project details from API
@@ -127,17 +128,18 @@ async def project_exists(
127128
128129
Args:
129130
project_name: Name of project to check
131+
workspace: Optional workspace override for tenant-scoped project lookup
130132
131133
Returns:
132134
True if project exists, False otherwise
135+
136+
Raises:
137+
CloudUtilsError: If the project list cannot be fetched from cloud
133138
"""
134-
try:
135-
projects = await fetch_cloud_projects(
136-
project_name=project_name,
137-
workspace=workspace,
138-
api_request=api_request,
139-
)
140-
project_names = {p.name for p in projects.projects}
141-
return project_name in project_names
142-
except Exception:
143-
return False
139+
projects = await fetch_cloud_projects(
140+
project_name=project_name,
141+
workspace=workspace,
142+
api_request=api_request,
143+
)
144+
project_names = {p.name for p in projects.projects}
145+
return project_name in project_names

src/basic_memory/cli/commands/cloud/project_sync.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,10 @@ def bisync_project_command(
195195
# Update config — sync_entry is guaranteed non-None because
196196
# _get_sync_project validated local_sync_path (which comes from sync_entry)
197197
sync_entry = config.projects.get(name)
198-
assert sync_entry is not None
198+
if sync_entry is None:
199+
raise RuntimeError(
200+
f"Sync entry for project '{name}' unexpectedly missing after validation"
201+
)
199202
sync_entry.last_sync = datetime.now()
200203
sync_entry.bisync_initialized = True
201204
ConfigManager().save_config(config)

src/basic_memory/cli/commands/cloud/upload_command.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from basic_memory.cli.app import cloud_app
1010
from basic_memory.cli.commands.command_utils import run_with_cleanup
1111
from basic_memory.cli.commands.cloud.cloud_utils import (
12+
CloudUtilsError,
1213
create_cloud_project,
1314
project_exists,
1415
sync_project,
@@ -79,8 +80,14 @@ def upload(
7980
async def _upload():
8081
resolved_workspace = resolve_configured_workspace(project_name=project)
8182

83+
try:
84+
project_already_exists = await project_exists(project, workspace=resolved_workspace)
85+
except CloudUtilsError as e:
86+
console.print(f"[red]Failed to check cloud project '{project}': {e}[/red]")
87+
raise typer.Exit(1)
88+
8289
# Check if project exists
83-
if not await project_exists(project, workspace=resolved_workspace):
90+
if not project_already_exists:
8491
if create_project:
8592
console.print(f"[blue]Creating cloud project '{project}'...[/blue]")
8693
try:
@@ -126,8 +133,10 @@ async def _upload():
126133
else:
127134
console.print(f"[green]Successfully uploaded to '{project}'[/green]")
128135

129-
# Sync project if requested (skip on dry run)
130-
# Force full scan after bisync to ensure database is up-to-date with synced files
136+
# Sync project if requested (skip on dry run).
137+
# Trigger: upload adds new files the watcher has not observed locally.
138+
# Why: force_full ensures those freshly uploaded files are indexed immediately.
139+
# Outcome: upload keeps its eager reindex while sync/bisync stay incremental.
131140
if sync and not dry_run:
132141
console.print(f"[blue]Syncing project '{project}'...[/blue]")
133142
try:

tests/cli/cloud/test_cloud_api_client_and_utils.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
make_api_request,
1111
)
1212
from basic_memory.cli.commands.cloud.cloud_utils import (
13+
CloudUtilsError,
1314
create_cloud_project,
1415
fetch_cloud_projects,
1516
project_exists,
@@ -216,6 +217,20 @@ async def api_request(**kwargs):
216217
]
217218

218219

220+
@pytest.mark.asyncio
221+
async def test_project_exists_surfaces_cloud_lookup_failures(config_home, config_manager):
222+
"""project_exists should surface lookup failures instead of pretending the project is missing."""
223+
config = config_manager.load_config()
224+
config.cloud_host = "https://cloud.example.test"
225+
config_manager.save_config(config)
226+
227+
async def api_request(**_kwargs):
228+
raise httpx.ConnectError("boom")
229+
230+
with pytest.raises(CloudUtilsError, match="Failed to fetch cloud projects"):
231+
await project_exists("alpha", api_request=api_request)
232+
233+
219234
@pytest.mark.asyncio
220235
async def test_make_api_request_prefers_api_key_over_oauth(config_home, config_manager):
221236
"""API key in config should be used without needing an OAuth token on disk."""

tests/cli/cloud/test_project_sync_command.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,39 @@ async def sync(self, external_id: str, force_full: bool = False):
7676
assert seen["force_full"] is False
7777

7878

79+
def test_cloud_bisync_fails_fast_when_sync_entry_disappears(monkeypatch, config_manager):
80+
"""Bisync should raise a runtime error when validated sync config vanishes before persistence."""
81+
project_sync_command = importlib.import_module("basic_memory.cli.commands.cloud.project_sync")
82+
83+
config = config_manager.load_config()
84+
config.projects.pop("research", None)
85+
config_manager.save_config(config)
86+
87+
monkeypatch.setattr(project_sync_command, "_require_cloud_credentials", lambda _config: None)
88+
monkeypatch.setattr(
89+
project_sync_command,
90+
"get_mount_info",
91+
lambda: _async_value(SimpleNamespace(bucket_name="tenant-bucket")),
92+
)
93+
monkeypatch.setattr(
94+
project_sync_command,
95+
"_get_cloud_project",
96+
lambda _name: _async_value(
97+
SimpleNamespace(name="research", external_id="external-project-id", path="research")
98+
),
99+
)
100+
monkeypatch.setattr(
101+
project_sync_command,
102+
"_get_sync_project",
103+
lambda _name, _config, _project_data: (SimpleNamespace(name="research"), "/tmp/research"),
104+
)
105+
monkeypatch.setattr(project_sync_command, "project_bisync", lambda *args, **kwargs: True)
106+
107+
result = runner.invoke(app, ["cloud", "bisync", "--name", "research"])
108+
109+
assert result.exit_code == 1, result.output
110+
assert "unexpectedly missing after validation" in result.output
111+
112+
79113
async def _async_value(value):
80114
return value

tests/cli/cloud/test_upload_command_routing.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from typer.testing import CliRunner
77

88
from basic_memory.cli.app import app
9+
from basic_memory.cli.commands.cloud.cloud_utils import CloudUtilsError
910
from basic_memory.config import ProjectMode
1011

1112
runner = CliRunner()
@@ -111,3 +112,32 @@ async def fake_upload_path(*args, **kwargs):
111112
assert seen["project_exists_workspace"] == "project-workspace"
112113
assert seen["control_plane_workspace"] == "project-workspace"
113114
assert seen["base_url"] == "https://cloud.example.test"
115+
116+
117+
def test_cloud_upload_exits_when_project_lookup_fails(monkeypatch, tmp_path):
118+
"""Upload command should fail fast when cloud project lookup cannot reach the API."""
119+
import basic_memory.cli.commands.cloud.upload_command as upload_command
120+
121+
upload_dir = tmp_path / "upload"
122+
upload_dir.mkdir()
123+
(upload_dir / "note.md").write_text("hello", encoding="utf-8")
124+
125+
async def fake_project_exists(_project_name: str, workspace: str | None = None) -> bool:
126+
raise CloudUtilsError("lookup failed")
127+
128+
monkeypatch.setattr(upload_command, "project_exists", fake_project_exists)
129+
130+
result = runner.invoke(
131+
app,
132+
[
133+
"cloud",
134+
"upload",
135+
str(upload_dir),
136+
"--project",
137+
"routing-test",
138+
"--no-sync",
139+
],
140+
)
141+
142+
assert result.exit_code == 1, result.output
143+
assert "Failed to check cloud project 'routing-test'" in result.output

0 commit comments

Comments
 (0)