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
61 changes: 32 additions & 29 deletions src/basic_memory/cli/commands/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -805,15 +805,16 @@ def _abort_after_project_created(
step: str,
remedy: str,
error: Exception,
retry_add_after_repair: bool = False,
) -> NoReturn:
"""Report a failure that happened after the project row became durable.

Two things are always true here and stated once: the project was created, and
re-running `bm project add` will refuse with "already exists". What must vary
is the remedy — it has to name a command that both fits the project's mode and
can actually repair the step that failed. A single fixed remedy pointed cloud
projects at `bm project index`, which the local-only reindex path rejects
outright (#1440 review).
The default contract states that the project was created and a blind re-run
will refuse with "already exists". When ``retry_add_after_repair`` is true,
the failed step prevented local registration, so repairing it makes re-running
the command safe: the existing adoption path finishes registration instead of
creating a second project. In either case, the remedy must fit the project's
mode and repair the step that actually failed (#1440 review).

The project is deliberately not rolled back: the row is the user's, and
deleting it to tidy up an error message would discard what they asked for.
Expand All @@ -823,7 +824,10 @@ def _abort_after_project_created(
# missing. Anything else still needs its message shown.
detail = "" if isinstance(error, typer.Exit) else f": {error}"
console.print(f"[yellow]Project '{name}' was created, but {step} failed{detail}[/yellow]")
console.print(f"Do not re-run 'bm project add' — '{name}' already exists. {remedy}")
if retry_add_after_repair:
console.print(f"Fix the failed step before re-running 'bm project add'. {remedy}")
else:
console.print(f"Do not re-run 'bm project add' — '{name}' already exists. {remedy}")
raise typer.Exit(1)


Expand Down Expand Up @@ -1016,6 +1020,27 @@ async def _add_project():
error=e,
)
else:
if local_sync_path:
try:
Path(local_sync_path).mkdir(parents=True, exist_ok=True)
except Exception as e:
# The routing entry must not name a path that failed validation:
# config loading creates every absolute project path, so saving
# this value would make every later CLI command fail (#1441).
_abort_after_project_created(
name,
step=f"creating the local sync directory {local_sync_path}",
remedy=(
f"Create it yourself (or fix its permissions), then run this same "
f"command again: "
f"[green]{_add_command_hint(name, local_sync_path, resolved_workspace_id)}[/green]. "
f"It will adopt the project that already exists rather than create a "
f"second one."
),
error=e,
retry_add_after_repair=True,
)

# Trigger: local config needs enough metadata to route future commands back to cloud.
# Why: explicit workspace selection and local sync state should persist across CLI sessions.
# Outcome: cloud-backed projects keep cloud mode, workspace_id, and optional local sync path.
Expand Down Expand Up @@ -1065,28 +1090,6 @@ async def _add_project():

# Save local sync path to config if in cloud mode
if local_sync_path:
try:
# Create local directory if it doesn't exist
local_dir = Path(local_sync_path)
local_dir.mkdir(parents=True, exist_ok=True)
except Exception as e:
# Cloud project and routing are both saved; only the local folder
# is missing, so recovery is filesystem-side then a resync.
# `bisync` is Personal-workspace-only -- `_require_personal_workspace`
# rejects it outright for a Team workspace, and the workspace type is
# not knowable here. `pull`/`push` are additive and Team-safe, and
# work on Personal too, so they are the remedy for both (#1440
# review). Same wording `bm cloud sync-setup` already prints.
_abort_after_project_created(
name,
step=f"creating the local sync directory {local_sync_path}",
remedy=(
f"Create it yourself (or fix its permissions), then fetch with "
f"[green]{command_hint('bm', 'cloud', 'pull', '--name', name)}[/green]."
),
error=e,
)

console.print(f"\n[green]Local sync path configured: {local_sync_path}[/green]")
# Lead with the Team-safe additive commands (they work on any
# workspace); the bisync mirror is Personal-only, so it is an aside
Expand Down
14 changes: 7 additions & 7 deletions tests/cli/test_project_add_post_creation_failure.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ def _run(coro: Any) -> Any:
assert resolved == []


def test_a_failed_local_sync_mkdir_gets_a_sync_remedy(tmp_path, monkeypatch, cloud_add):
"""A directory that cannot be created is repaired on the filesystem, then resynced."""
def test_a_failed_local_sync_mkdir_gets_an_add_retry_remedy(tmp_path, monkeypatch, cloud_add):
"""A directory failure is repaired before add adopts and configures the project."""
# A real mkdir failure: the parent is a file, so creating a child raises.
blocker = tmp_path / "blocker"
blocker.write_text("not a directory")
Expand All @@ -342,12 +342,12 @@ def test_a_failed_local_sync_mkdir_gets_a_sync_remedy(tmp_path, monkeypatch, clo
assert "added successfully" in flat(result.output)
assert "was created, but creating the local sync directory" in flat(result.output)
assert "bm project index" not in flat(result.output)
# `bisync` is Personal-only; `pull` works on Team workspaces too, so it is
# the remedy for both. The full mode x step matrix lives in
# tests/cli/test_project_add_remedy_matrix.py.
assert "bm cloud pull --name research" in flat(result.output)
remedy = _printed_remedy_command(result.output)
assert remedy[:3] == ["project", "add", "research"]
assert "--local-path" in remedy
assert "bm cloud pull" not in flat(result.output)
assert "bm cloud bisync" not in flat(result.output)
assert "Do not re-run 'bm project add'" in flat(result.output)
assert "Fix the failed step before re-running 'bm project add'" in flat(result.output)


def test_the_remedy_survives_a_project_name_with_a_space(tmp_path, monkeypatch, cloud_add):
Expand Down
5 changes: 4 additions & 1 deletion tests/cli/test_project_add_remedy_matrix.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,10 @@ def test_the_printed_remedy_actually_works(

assert result.exit_code == 1
assert f"but {step}" in flat(result.output)
assert "Do not re-run 'bm project add'" in flat(result.output)
if step == SYNC_DIR:
assert "Fix the failed step before re-running 'bm project add'" in flat(result.output)
else:
assert "Do not re-run 'bm project add'" in flat(result.output)

remedy = _printed_remedy_command(result.output)

Expand Down
27 changes: 27 additions & 0 deletions tests/cli/test_project_add_with_local_path.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,33 @@ def test_project_add_local_path_creates_nested_directories(
assert nested_path.is_dir()


def test_project_add_invalid_local_path_keeps_config_loadable(
runner, mock_config, mock_api_client, tmp_path
):
"""A mkdir failure must not persist a path that bricks later CLI commands."""
blocker = tmp_path / "blocker"
blocker.write_text("not a directory")
invalid_path = blocker / "sync"

result = runner.invoke(
app,
["project", "add", "test-project", "--cloud", "--local-path", str(invalid_path)],
)

assert result.exit_code == 1
config_data = json.loads(mock_config.read_text())
assert "test-project" not in config_data["projects"]

from basic_memory import config as config_module

config_module._CONFIG_CACHE = None
config_module._CONFIG_MTIME = None
config_module._CONFIG_SIZE = None

follow_up = runner.invoke(app, ["config", "list"])
assert follow_up.exit_code == 0, follow_up.stdout


def test_project_add_cloud_visibility_passes_payload(runner, mock_config, mock_api_client):
"""Cloud project creation should forward visibility to the API payload."""
result = runner.invoke(
Expand Down
Loading