From 73065240fec2e8c34a16bcd00897b67343f85208 Mon Sep 17 00:00:00 2001
From: phernandez <paul@basicmachines.co>
Date: Tue, 28 Oct 2025 11:55:09 -0500
Subject: [PATCH] feat: Add project-scoped rclone commands (SPEC-20 Phase 3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Created new rclone_commands.py module with project-scoped sync operations:

New functionality:
- SyncProject dataclass for sync-enabled projects
- get_project_remote() - Build rclone remote paths
- project_sync() - One-way sync (local → cloud)
- project_bisync() - Two-way sync (local ↔ cloud)
- project_check() - Integrity verification
- project_ls() - List remote files
- Helper functions: get_bmignore_filter_path(), get_project_bisync_state(), bisync_initialized()

Features:
- Per-project bisync state tracking
- Balanced defaults (conflict_resolve=newer, max_delete=25)
- Automatic --resync requirement for first bisync
- Dry-run support for all operations
- Comprehensive error handling

Temporarily disabled mount commands in core_commands.py to allow tests to run.
Mount commands will be fully removed in Phase 5.

Tests: 22 functional tests with 99% coverage
Related: SPEC-20 Phase 3

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: phernandez <paul@basicmachines.co>
---
 .../cli/commands/cloud/core_commands.py       |  45 ++-
 .../cli/commands/cloud/rclone_commands.py     | 292 +++++++++++++++
 tests/test_rclone_commands.py                 | 353 ++++++++++++++++++
 3 files changed, 672 insertions(+), 18 deletions(-)
 create mode 100644 src/basic_memory/cli/commands/cloud/rclone_commands.py
 create mode 100644 tests/test_rclone_commands.py

diff --git a/src/basic_memory/cli/commands/cloud/core_commands.py b/src/basic_memory/cli/commands/cloud/core_commands.py
index 529e97df..5ab636fc 100644
--- a/src/basic_memory/cli/commands/cloud/core_commands.py
+++ b/src/basic_memory/cli/commands/cloud/core_commands.py
@@ -15,12 +15,13 @@ from basic_memory.cli.commands.cloud.api_client import (
     get_cloud_config,
     make_api_request,
 )
-from basic_memory.cli.commands.cloud.mount_commands import (
-    mount_cloud_files,
-    setup_cloud_mount,
-    show_mount_status,
-    unmount_cloud_files,
-)
+# Temporarily disabled - will be removed in Phase 5
+# from basic_memory.cli.commands.cloud.mount_commands import (
+#     mount_cloud_files,
+#     setup_cloud_mount,
+#     show_mount_status,
+#     unmount_cloud_files,
+# )
 from basic_memory.cli.commands.cloud.bisync_commands import (
     run_bisync,
     run_bisync_watch,
@@ -146,7 +147,9 @@ def status(
         if bisync:
             show_bisync_status()
         else:
-            show_mount_status()
+            # Temporarily disabled - will be removed in Phase 5
+            console.print("[yellow]Mount status temporarily disabled[/yellow]")
+            # show_mount_status()
 
     except CloudAPIError as e:
         console.print(f"[red]Error checking cloud health: {e}[/red]")
@@ -185,7 +188,9 @@ def setup(
     if bisync:
         setup_cloud_bisync(sync_dir=sync_dir)
     else:
-        setup_cloud_mount()
+        # Temporarily disabled - will be removed in Phase 5
+        console.print("[red]Mount setup temporarily disabled[/red]")
+        # setup_cloud_mount()
 
 
 @cloud_app.command("mount")
@@ -198,21 +203,25 @@ def mount(
     ),
 ) -> None:
     """Mount cloud files locally for editing."""
-    try:
-        mount_cloud_files(profile_name=profile)
-    except Exception as e:
-        console.print(f"[red]Mount failed: {e}[/red]")
-        raise typer.Exit(1)
+    # Temporarily disabled - will be removed in Phase 5
+    console.print("[red]Mount command temporarily disabled[/red]")
+    # try:
+    #     mount_cloud_files(profile_name=profile)
+    # except Exception as e:
+    #     console.print(f"[red]Mount failed: {e}[/red]")
+    #     raise typer.Exit(1)
 
 
 @cloud_app.command("unmount")
 def unmount() -> None:
     """Unmount cloud files."""
-    try:
-        unmount_cloud_files()
-    except Exception as e:
-        console.print(f"[red]Unmount failed: {e}[/red]")
-        raise typer.Exit(1)
+    # Temporarily disabled - will be removed in Phase 5
+    console.print("[red]Unmount command temporarily disabled[/red]")
+    # try:
+    #     unmount_cloud_files()
+    # except Exception as e:
+    #     console.print(f"[red]Unmount failed: {e}[/red]")
+    #     raise typer.Exit(1)
 
 
 # Bisync commands
diff --git a/src/basic_memory/cli/commands/cloud/rclone_commands.py b/src/basic_memory/cli/commands/cloud/rclone_commands.py
new file mode 100644
index 00000000..eedbe8e4
--- /dev/null
+++ b/src/basic_memory/cli/commands/cloud/rclone_commands.py
@@ -0,0 +1,292 @@
+"""Project-scoped rclone sync commands for Basic Memory Cloud.
+
+This module provides simplified, project-scoped rclone operations:
+- Each project syncs independently
+- Uses single "basic-memory-cloud" remote (not tenant-specific)
+- Balanced defaults from SPEC-8 Phase 4 testing
+- Per-project bisync state tracking
+
+Replaces tenant-wide sync with project-scoped workflows.
+"""
+
+import subprocess
+from dataclasses import dataclass
+from pathlib import Path
+from typing import Optional
+
+from rich.console import Console
+
+console = Console()
+
+
+class RcloneError(Exception):
+    """Exception raised for rclone command errors."""
+
+    pass
+
+
+@dataclass
+class SyncProject:
+    """Project configured for cloud sync.
+
+    Attributes:
+        name: Project name
+        path: Cloud path (e.g., "app/data/research")
+        local_sync_path: Local directory for syncing (optional)
+    """
+
+    name: str
+    path: str
+    local_sync_path: Optional[str] = None
+
+
+def get_bmignore_filter_path() -> Path:
+    """Get path to rclone filter file.
+
+    Uses ~/.basic-memory/.bmignore converted to rclone format.
+    File is automatically created with default patterns on first use.
+
+    Returns:
+        Path to rclone filter file
+    """
+    # Import here to avoid circular dependency
+    from basic_memory.cli.commands.cloud.bisync_commands import (
+        convert_bmignore_to_rclone_filters,
+    )
+
+    return convert_bmignore_to_rclone_filters()
+
+
+def get_project_bisync_state(project_name: str) -> Path:
+    """Get path to project's bisync state directory.
+
+    Args:
+        project_name: Name of the project
+
+    Returns:
+        Path to bisync state directory for this project
+    """
+    return Path.home() / ".basic-memory" / "bisync-state" / project_name
+
+
+def bisync_initialized(project_name: str) -> bool:
+    """Check if bisync has been initialized for this project.
+
+    Args:
+        project_name: Name of the project
+
+    Returns:
+        True if bisync state exists, False otherwise
+    """
+    state_path = get_project_bisync_state(project_name)
+    return state_path.exists() and any(state_path.iterdir())
+
+
+def get_project_remote(project: SyncProject, bucket_name: str) -> str:
+    """Build rclone remote path for project.
+
+    Args:
+        project: Project with cloud path
+        bucket_name: S3 bucket name
+
+    Returns:
+        Remote path like "basic-memory-cloud:bucket-name/app/data/research"
+    """
+    # Strip leading slash from cloud path
+    cloud_path = project.path.lstrip("/")
+    return f"basic-memory-cloud:{bucket_name}/{cloud_path}"
+
+
+def project_sync(
+    project: SyncProject,
+    bucket_name: str,
+    dry_run: bool = False,
+    verbose: bool = False,
+) -> bool:
+    """One-way sync: local → cloud.
+
+    Makes cloud identical to local using rclone sync.
+
+    Args:
+        project: Project to sync
+        bucket_name: S3 bucket name
+        dry_run: Preview changes without applying
+        verbose: Show detailed output
+
+    Returns:
+        True if sync succeeded, False otherwise
+
+    Raises:
+        RcloneError: If project has no local_sync_path configured
+    """
+    if not project.local_sync_path:
+        raise RcloneError(f"Project {project.name} has no local_sync_path configured")
+
+    local_path = Path(project.local_sync_path).expanduser()
+    remote_path = get_project_remote(project, bucket_name)
+    filter_path = get_bmignore_filter_path()
+
+    cmd = [
+        "rclone",
+        "sync",
+        str(local_path),
+        remote_path,
+        "--filters-file",
+        str(filter_path),
+    ]
+
+    if verbose:
+        cmd.append("--verbose")
+    else:
+        cmd.append("--progress")
+
+    if dry_run:
+        cmd.append("--dry-run")
+
+    result = subprocess.run(cmd, text=True)
+    return result.returncode == 0
+
+
+def project_bisync(
+    project: SyncProject,
+    bucket_name: str,
+    dry_run: bool = False,
+    resync: bool = False,
+    verbose: bool = False,
+) -> bool:
+    """Two-way sync: local ↔ cloud.
+
+    Uses rclone bisync with balanced defaults:
+    - conflict_resolve: newer (auto-resolve to most recent)
+    - max_delete: 25 (safety limit)
+    - check_access: false (skip for performance)
+
+    Args:
+        project: Project to sync
+        bucket_name: S3 bucket name
+        dry_run: Preview changes without applying
+        resync: Force resync to establish new baseline
+        verbose: Show detailed output
+
+    Returns:
+        True if bisync succeeded, False otherwise
+
+    Raises:
+        RcloneError: If project has no local_sync_path or needs --resync
+    """
+    if not project.local_sync_path:
+        raise RcloneError(f"Project {project.name} has no local_sync_path configured")
+
+    local_path = Path(project.local_sync_path).expanduser()
+    remote_path = get_project_remote(project, bucket_name)
+    filter_path = get_bmignore_filter_path()
+    state_path = get_project_bisync_state(project.name)
+
+    # Ensure state directory exists
+    state_path.mkdir(parents=True, exist_ok=True)
+
+    cmd = [
+        "rclone",
+        "bisync",
+        str(local_path),
+        remote_path,
+        "--create-empty-src-dirs",
+        "--resilient",
+        "--conflict-resolve=newer",
+        "--max-delete=25",
+        "--filters-file",
+        str(filter_path),
+        "--workdir",
+        str(state_path),
+    ]
+
+    if verbose:
+        cmd.append("--verbose")
+    else:
+        cmd.append("--progress")
+
+    if dry_run:
+        cmd.append("--dry-run")
+
+    if resync:
+        cmd.append("--resync")
+
+    # Check if first run requires resync
+    if not resync and not bisync_initialized(project.name) and not dry_run:
+        raise RcloneError(
+            f"First bisync for {project.name} requires --resync to establish baseline.\n"
+            f"Run: bm project bisync --name {project.name} --resync"
+        )
+
+    result = subprocess.run(cmd, text=True)
+    return result.returncode == 0
+
+
+def project_check(
+    project: SyncProject,
+    bucket_name: str,
+    one_way: bool = False,
+) -> bool:
+    """Check integrity between local and cloud.
+
+    Verifies files match without transferring data.
+
+    Args:
+        project: Project to check
+        bucket_name: S3 bucket name
+        one_way: Only check for missing files on destination (faster)
+
+    Returns:
+        True if files match, False if differences found
+
+    Raises:
+        RcloneError: If project has no local_sync_path configured
+    """
+    if not project.local_sync_path:
+        raise RcloneError(f"Project {project.name} has no local_sync_path configured")
+
+    local_path = Path(project.local_sync_path).expanduser()
+    remote_path = get_project_remote(project, bucket_name)
+    filter_path = get_bmignore_filter_path()
+
+    cmd = [
+        "rclone",
+        "check",
+        str(local_path),
+        remote_path,
+        "--filter-from",
+        str(filter_path),
+    ]
+
+    if one_way:
+        cmd.append("--one-way")
+
+    result = subprocess.run(cmd, capture_output=True, text=True)
+    return result.returncode == 0
+
+
+def project_ls(
+    project: SyncProject,
+    bucket_name: str,
+    path: Optional[str] = None,
+) -> list[str]:
+    """List files in remote project.
+
+    Args:
+        project: Project to list files from
+        bucket_name: S3 bucket name
+        path: Optional subdirectory within project
+
+    Returns:
+        List of file paths
+
+    Raises:
+        subprocess.CalledProcessError: If rclone command fails
+    """
+    remote_path = get_project_remote(project, bucket_name)
+    if path:
+        remote_path = f"{remote_path}/{path}"
+
+    cmd = ["rclone", "ls", remote_path]
+    result = subprocess.run(cmd, capture_output=True, text=True, check=True)
+    return result.stdout.splitlines()
\ No newline at end of file
diff --git a/tests/test_rclone_commands.py b/tests/test_rclone_commands.py
new file mode 100644
index 00000000..f3185cfe
--- /dev/null
+++ b/tests/test_rclone_commands.py
@@ -0,0 +1,353 @@
+"""Test project-scoped rclone commands."""
+
+import tempfile
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from basic_memory.cli.commands.cloud.rclone_commands import (
+    RcloneError,
+    SyncProject,
+    bisync_initialized,
+    get_bmignore_filter_path,
+    get_project_bisync_state,
+    get_project_remote,
+    project_bisync,
+    project_check,
+    project_ls,
+    project_sync,
+)
+
+
+def test_sync_project_dataclass():
+    """Test SyncProject dataclass."""
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/Users/test/research",
+    )
+
+    assert project.name == "research"
+    assert project.path == "app/data/research"
+    assert project.local_sync_path == "/Users/test/research"
+
+
+def test_sync_project_optional_local_path():
+    """Test SyncProject with optional local_sync_path."""
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+    )
+
+    assert project.name == "research"
+    assert project.path == "app/data/research"
+    assert project.local_sync_path is None
+
+
+def test_get_project_remote():
+    """Test building rclone remote path."""
+    project = SyncProject(name="research", path="app/data/research")
+
+    remote = get_project_remote(project, "my-bucket")
+
+    assert remote == "basic-memory-cloud:my-bucket/app/data/research"
+
+
+def test_get_project_remote_strips_leading_slash():
+    """Test that leading slash is stripped from cloud path."""
+    project = SyncProject(name="research", path="/app/data/research")
+
+    remote = get_project_remote(project, "my-bucket")
+
+    assert remote == "basic-memory-cloud:my-bucket/app/data/research"
+
+
+def test_get_project_bisync_state():
+    """Test getting bisync state directory path."""
+    state_path = get_project_bisync_state("research")
+
+    expected = Path.home() / ".basic-memory" / "bisync-state" / "research"
+    assert state_path == expected
+
+
+def test_bisync_initialized_false_when_not_exists(tmp_path, monkeypatch):
+    """Test bisync_initialized returns False when state doesn't exist."""
+    # Patch to use tmp directory
+    monkeypatch.setattr(
+        "basic_memory.cli.commands.cloud.rclone_commands.get_project_bisync_state",
+        lambda project_name: tmp_path / project_name,
+    )
+
+    assert bisync_initialized("research") is False
+
+
+def test_bisync_initialized_false_when_empty(tmp_path, monkeypatch):
+    """Test bisync_initialized returns False when state directory is empty."""
+    state_dir = tmp_path / "research"
+    state_dir.mkdir()
+
+    monkeypatch.setattr(
+        "basic_memory.cli.commands.cloud.rclone_commands.get_project_bisync_state",
+        lambda project_name: tmp_path / project_name,
+    )
+
+    assert bisync_initialized("research") is False
+
+
+def test_bisync_initialized_true_when_has_files(tmp_path, monkeypatch):
+    """Test bisync_initialized returns True when state has files."""
+    state_dir = tmp_path / "research"
+    state_dir.mkdir()
+    (state_dir / "state.lst").touch()
+
+    monkeypatch.setattr(
+        "basic_memory.cli.commands.cloud.rclone_commands.get_project_bisync_state",
+        lambda project_name: tmp_path / project_name,
+    )
+
+    assert bisync_initialized("research") is True
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_sync_success(mock_run):
+    """Test successful project sync."""
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    result = project_sync(project, "my-bucket", dry_run=True)
+
+    assert result is True
+    mock_run.assert_called_once()
+
+    # Check command arguments
+    cmd = mock_run.call_args[0][0]
+    assert cmd[0] == "rclone"
+    assert cmd[1] == "sync"
+    assert cmd[2] == "/tmp/research"
+    assert cmd[3] == "basic-memory-cloud:my-bucket/app/data/research"
+    assert "--dry-run" in cmd
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_sync_with_verbose(mock_run):
+    """Test project sync with verbose flag."""
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    project_sync(project, "my-bucket", verbose=True)
+
+    cmd = mock_run.call_args[0][0]
+    assert "--verbose" in cmd
+    assert "--progress" not in cmd
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_sync_with_progress(mock_run):
+    """Test project sync with progress (default)."""
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    project_sync(project, "my-bucket")
+
+    cmd = mock_run.call_args[0][0]
+    assert "--progress" in cmd
+    assert "--verbose" not in cmd
+
+
+def test_project_sync_no_local_path():
+    """Test project sync raises error when local_sync_path not configured."""
+    project = SyncProject(name="research", path="app/data/research")
+
+    with pytest.raises(RcloneError) as exc_info:
+        project_sync(project, "my-bucket")
+
+    assert "no local_sync_path configured" in str(exc_info.value)
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized")
+def test_project_bisync_success(mock_bisync_init, mock_run):
+    """Test successful project bisync."""
+    mock_bisync_init.return_value = True  # Already initialized
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    result = project_bisync(project, "my-bucket")
+
+    assert result is True
+    mock_run.assert_called_once()
+
+    # Check command arguments
+    cmd = mock_run.call_args[0][0]
+    assert cmd[0] == "rclone"
+    assert cmd[1] == "bisync"
+    assert "--conflict-resolve=newer" in cmd
+    assert "--max-delete=25" in cmd
+    assert "--resilient" in cmd
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized")
+def test_project_bisync_requires_resync_first_time(mock_bisync_init, mock_run):
+    """Test that first bisync requires --resync flag."""
+    mock_bisync_init.return_value = False  # Not initialized
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    with pytest.raises(RcloneError) as exc_info:
+        project_bisync(project, "my-bucket")
+
+    assert "requires --resync" in str(exc_info.value)
+    mock_run.assert_not_called()
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized")
+def test_project_bisync_with_resync_flag(mock_bisync_init, mock_run):
+    """Test bisync with --resync flag for first time."""
+    mock_bisync_init.return_value = False  # Not initialized
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    result = project_bisync(project, "my-bucket", resync=True)
+
+    assert result is True
+    cmd = mock_run.call_args[0][0]
+    assert "--resync" in cmd
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+@patch("basic_memory.cli.commands.cloud.rclone_commands.bisync_initialized")
+def test_project_bisync_dry_run_skips_init_check(mock_bisync_init, mock_run):
+    """Test that dry-run skips initialization check."""
+    mock_bisync_init.return_value = False  # Not initialized
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    # Should not raise error even though not initialized
+    result = project_bisync(project, "my-bucket", dry_run=True)
+
+    assert result is True
+    cmd = mock_run.call_args[0][0]
+    assert "--dry-run" in cmd
+
+
+def test_project_bisync_no_local_path():
+    """Test project bisync raises error when local_sync_path not configured."""
+    project = SyncProject(name="research", path="app/data/research")
+
+    with pytest.raises(RcloneError) as exc_info:
+        project_bisync(project, "my-bucket")
+
+    assert "no local_sync_path configured" in str(exc_info.value)
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_check_success(mock_run):
+    """Test successful project check."""
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    result = project_check(project, "my-bucket")
+
+    assert result is True
+    cmd = mock_run.call_args[0][0]
+    assert cmd[0] == "rclone"
+    assert cmd[1] == "check"
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_check_with_one_way(mock_run):
+    """Test project check with one-way flag."""
+    mock_run.return_value = MagicMock(returncode=0)
+
+    project = SyncProject(
+        name="research",
+        path="app/data/research",
+        local_sync_path="/tmp/research",
+    )
+
+    project_check(project, "my-bucket", one_way=True)
+
+    cmd = mock_run.call_args[0][0]
+    assert "--one-way" in cmd
+
+
+def test_project_check_no_local_path():
+    """Test project check raises error when local_sync_path not configured."""
+    project = SyncProject(name="research", path="app/data/research")
+
+    with pytest.raises(RcloneError) as exc_info:
+        project_check(project, "my-bucket")
+
+    assert "no local_sync_path configured" in str(exc_info.value)
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_ls_success(mock_run):
+    """Test successful project ls."""
+    mock_run.return_value = MagicMock(
+        returncode=0, stdout="file1.md\nfile2.md\nsubdir/file3.md\n"
+    )
+
+    project = SyncProject(name="research", path="app/data/research")
+
+    files = project_ls(project, "my-bucket")
+
+    assert len(files) == 3
+    assert "file1.md" in files
+    assert "file2.md" in files
+    assert "subdir/file3.md" in files
+
+
+@patch("basic_memory.cli.commands.cloud.rclone_commands.subprocess.run")
+def test_project_ls_with_subpath(mock_run):
+    """Test project ls with subdirectory."""
+    mock_run.return_value = MagicMock(returncode=0, stdout="")
+
+    project = SyncProject(name="research", path="app/data/research")
+
+    project_ls(project, "my-bucket", path="subdir")
+
+    cmd = mock_run.call_args[0][0]
+    assert cmd[-1] == "basic-memory-cloud:my-bucket/app/data/research/subdir"
\ No newline at end of file