feat(cli): add Team-safe cloud push/pull and gate sync to Personal workspaces (#917)

Adds additive, git-style `bm cloud push`/`pull` that are safe on shared Team workspaces (never delete on the destination; conflicts abort by default with `--on-conflict {fail|keep-local|keep-cloud|keep-both}`), and gates the destructive `bm cloud sync`/`bisync` mirrors to Personal workspaces. Closes #858. Longer-term Team-safe reconciler tracked in #862; workspace-scoped mount info (Codex P1) tracked as a follow-up.
This commit is contained in:
Paul Hernandez
2026-06-08 14:08:49 -05:00
committed by GitHub
parent a8d034b940
commit 9b53d7863f
6 changed files with 1294 additions and 80 deletions
@@ -7,6 +7,7 @@ they are cloud-specific operations.
import os
from datetime import datetime
from enum import Enum
import typer
from rich.console import Console
@@ -16,10 +17,14 @@ from basic_memory.cli.commands.cloud.bisync_commands import get_mount_info
from basic_memory.cli.commands.cloud.rclone_commands import (
RcloneError,
SyncProject,
TransferDirection,
TransferPlan,
get_project_bisync_state,
project_bisync,
project_check,
project_diff,
project_sync,
project_transfer,
)
from basic_memory.cli.commands.command_utils import run_with_cleanup
from basic_memory.cli.commands.routing import force_routing
@@ -35,9 +40,34 @@ console = Console()
TEAM_WORKSPACE_BISYNC_UNSUPPORTED = (
"The bisync operation is only supported on Personal workspaces.\n"
"Use `bm cloud sync --name {name}` instead."
"Use `bm cloud pull --name {name}` / `bm cloud push --name {name}` instead."
)
TEAM_WORKSPACE_SYNC_UNSUPPORTED = (
"The sync operation mirrors local onto the shared bucket and can delete a "
"teammate's files, so it is only supported on Personal workspaces.\n"
"Use `bm cloud pull --name {name}` (fetch) / `bm cloud push --name {name}` "
"(additive upload) instead."
)
class ConflictStrategy(str, Enum):
"""How push/pull resolves files that differ on both sides.
Default is ``fail``: surface the conflicts and abort before transferring,
leaving the user to re-run with an explicit resolution — like git refusing
to clobber local changes.
This is the Typer-facing enum; the engine in ``rclone_commands`` accepts the
same values as a ``ConflictStrategy`` Literal. ``_run_directional_transfer``
bridges the two by passing ``on_conflict.value``. Keep the values in sync.
"""
fail = "fail"
keep_local = "keep-local"
keep_cloud = "keep-cloud"
keep_both = "keep-both"
# --- Shared helpers ---
@@ -92,8 +122,18 @@ async def _get_workspace_for_project(name: str, config: BasicMemoryConfig) -> Wo
)
def _require_personal_workspace(name: str, config: BasicMemoryConfig) -> WorkspaceInfo:
"""Exit before bisync work when the target workspace is not personal."""
def _require_personal_workspace(
name: str,
config: BasicMemoryConfig,
*,
unsupported_message: str = TEAM_WORKSPACE_BISYNC_UNSUPPORTED,
) -> WorkspaceInfo:
"""Exit before mirror work when the target workspace is not personal.
Used to gate the destructive mirror operations (`sync`, `bisync`) to
Personal workspaces. ``unsupported_message`` lets each command point Team
users at the right Team-safe alternative.
"""
try:
workspace = run_with_cleanup(_get_workspace_for_project(name, config))
except Exception as exc:
@@ -101,7 +141,7 @@ def _require_personal_workspace(name: str, config: BasicMemoryConfig) -> Workspa
raise typer.Exit(1)
if workspace.workspace_type != "personal":
console.print(f"[red]{TEAM_WORKSPACE_BISYNC_UNSUPPORTED.format(name=name)}[/red]")
console.print(f"[red]{unsupported_message.format(name=name)}[/red]")
raise typer.Exit(1)
return workspace
@@ -150,7 +190,11 @@ def sync_project_command(
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without syncing"),
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
) -> None:
"""One-way sync: local -> cloud (make cloud identical to local).
"""One-way mirror: local -> cloud (make cloud identical to local).
Personal workspaces only. This deletes cloud files not present locally, so
on Team workspaces use `bm cloud push` (additive upload) / `bm cloud pull`
(fetch) instead.
Example:
bm cloud sync --name research
@@ -158,6 +202,7 @@ def sync_project_command(
"""
config = ConfigManager().config
_require_cloud_credentials(config)
_require_personal_workspace(name, config, unsupported_message=TEAM_WORKSPACE_SYNC_UNSUPPORTED)
try:
# Get tenant info for bucket name
@@ -191,6 +236,172 @@ def sync_project_command(
raise typer.Exit(1)
def _print_conflict_abort(name: str, direction: TransferDirection, plan: TransferPlan) -> None:
"""Explain a conflict abort and how to resolve it (git-pull style)."""
console.print(
f"[red]{direction.capitalize()} aborted: {len(plan.conflicts)} file(s) differ between "
f"local and cloud.[/red]"
)
for path in plan.conflicts:
console.print(f" [yellow]*[/yellow] {path}")
console.print("\nRe-run with one of:")
console.print(" [dim]--on-conflict keep-cloud[/dim] take the cloud version")
console.print(" [dim]--on-conflict keep-local[/dim] keep your local version")
console.print(
" [dim]--on-conflict keep-both[/dim] keep both (writes <name>.conflict-<date>)"
)
def _run_directional_transfer(
name: str,
direction: TransferDirection,
*,
on_conflict: ConflictStrategy,
dry_run: bool,
verbose: bool,
) -> None:
"""Shared orchestration for `bm cloud push` / `bm cloud pull`.
Detects conflicts first, then aborts (the default) or applies the chosen
resolution. Uses additive `rclone copy`, so it never deletes on the
destination — safe for Team workspaces and therefore not gated.
"""
config = ConfigManager().config
_require_cloud_credentials(config)
try:
# Get tenant info for bucket name
tenant_info = run_with_cleanup(get_mount_info())
bucket_name = tenant_info.bucket_name
# Get project info
with force_routing(cloud=True):
project_data = run_with_cleanup(_get_cloud_project(name))
if not project_data:
console.print(f"[red]Error: Project '{name}' not found[/red]")
raise typer.Exit(1)
sync_project, _ = _get_sync_project(name, config, project_data)
# --- Detect before transferring ---
plan = project_diff(sync_project, bucket_name, direction)
# Trigger: rclone could not read/hash some files.
# Why: comparing is the whole basis for a safe transfer — never guess.
# Outcome: abort before moving any bytes.
if plan.errors:
console.print(
f"[red]{direction.capitalize()} aborted: rclone could not compare "
f"{len(plan.errors)} file(s)[/red]"
)
for path in plan.errors:
console.print(f" [red]![/red] {path}")
raise typer.Exit(1)
# Trigger: files differ on both sides and the user chose no resolution.
# Why: "no surprises" — never silently pick a winner.
# Outcome: list the conflicts and exit, like git refusing to clobber.
if plan.conflicts and on_conflict is ConflictStrategy.fail:
_print_conflict_abort(name, direction, plan)
raise typer.Exit(1)
# --- Transfer ---
arrow = "cloud -> local" if direction == "pull" else "local -> cloud"
console.print(f"[blue]{direction.capitalize()} {name} ({arrow})...[/blue]")
conflict_suffix = datetime.now().strftime("%Y%m%d-%H%M%S")
success = project_transfer(
sync_project,
bucket_name,
direction,
plan,
strategy=on_conflict.value,
conflict_suffix=conflict_suffix,
dry_run=dry_run,
verbose=verbose,
)
if not success:
console.print(f"[red]{name} {direction} failed[/red]")
raise typer.Exit(1)
console.print(f"[green]{name} {direction} completed successfully[/green]")
# Without a sync baseline (see #862) we cannot tell an intentional delete
# from a file the other side simply never had, so deletions never sync.
if plan.dest_only:
kept_on = "local" if direction == "pull" else "cloud"
console.print(
f"[dim]{len(plan.dest_only)} file(s) exist only on {kept_on} and were left "
"untouched (deletions are not propagated).[/dim]"
)
except RcloneError as e:
console.print(f"[red]{direction.capitalize()} error: {e}[/red]")
raise typer.Exit(1)
except typer.Exit:
# Already-handled exits (not found, conflicts, errors) propagate cleanly.
raise
except Exception as e:
console.print(f"[red]Error: {e}[/red]")
raise typer.Exit(1)
@cloud_app.command("pull")
def pull_project_command(
name: str = typer.Option(..., "--name", "--project", help="Project name to pull"),
on_conflict: ConflictStrategy = typer.Option(
ConflictStrategy.fail,
"--on-conflict",
help="Resolve files that differ on both sides (default: fail and list them)",
),
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without pulling"),
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
) -> None:
"""Fetch cloud changes into local (cloud -> local), git-pull style.
Additive and Team-safe: downloads new/changed cloud files and never deletes
local files. A file that differs on both sides is a conflict; by default
pull aborts and lists them. Deletions are not propagated (see #862).
Examples:
bm cloud pull --name research
bm cloud pull --name research --dry-run
bm cloud pull --name research --on-conflict keep-cloud
"""
_run_directional_transfer(
name, "pull", on_conflict=on_conflict, dry_run=dry_run, verbose=verbose
)
@cloud_app.command("push")
def push_project_command(
name: str = typer.Option(..., "--name", "--project", help="Project name to push"),
on_conflict: ConflictStrategy = typer.Option(
ConflictStrategy.fail,
"--on-conflict",
help="Resolve files that differ on both sides (default: fail and list them)",
),
dry_run: bool = typer.Option(False, "--dry-run", help="Preview changes without pushing"),
verbose: bool = typer.Option(False, "--verbose", "-v", help="Show detailed output"),
) -> None:
"""Upload local changes to cloud (local -> cloud), additive and Team-safe.
Uploads new/changed local files and never deletes cloud files. A file that
differs on both sides is a conflict; by default push aborts and lists them
(like git rejecting a push when the remote is ahead — pull first). Deletions
are not propagated (see #862).
Examples:
bm cloud push --name research
bm cloud push --name research --dry-run
bm cloud push --name research --on-conflict keep-local
"""
_run_directional_transfer(
name, "push", on_conflict=on_conflict, dry_run=dry_run, verbose=verbose
)
@cloud_app.command("bisync")
def bisync_project_command(
name: str = typer.Option(..., "--name", "--project", help="Project name to bisync"),
@@ -395,9 +606,15 @@ def setup_project_sync(
console.print(f"[green]Sync configured for project '{name}'[/green]")
console.print(f"\nLocal sync path: {resolved_path}")
# Lead with the Team-safe additive commands (work on any workspace); the
# `sync`/`bisync` mirrors are Personal-workspace-only.
console.print("\nNext steps:")
console.print(f" 1. Preview: bm cloud sync --name {name} --dry-run")
console.print(f" 2. Sync: bm cloud sync --name {name}")
console.print(f" 1. Preview a pull: bm cloud pull --name {name} --dry-run")
console.print(f" 2. Fetch from cloud: bm cloud pull --name {name}")
console.print(f" 3. Upload local changes: bm cloud push --name {name}")
console.print(
f" Personal workspaces can also mirror with: bm cloud bisync --name {name} --resync"
)
except Exception as e:
console.print(f"[red]Error configuring sync: {str(e)}[/red]")
raise typer.Exit(1)
@@ -11,10 +11,10 @@ Replaces tenant-wide sync with project-scoped workflows.
import re
import subprocess
from dataclasses import dataclass
from dataclasses import dataclass, field
from functools import lru_cache
from pathlib import Path
from typing import Callable, Optional, Protocol
from pathlib import Path, PurePosixPath
from typing import Callable, Literal, Optional, Protocol
from loguru import logger
from rich.console import Console
@@ -42,6 +42,7 @@ TIGRIS_CONSISTENCY_HEADERS = [
class RunResult(Protocol):
returncode: int
stdout: str
stderr: str
RunFunc = Callable[..., RunResult]
@@ -184,6 +185,91 @@ def get_project_remote(project: SyncProject, bucket_name: str) -> str:
return f"basic-memory-cloud:{bucket_name}/{cloud_path}"
# --- Directional transfer primitives (push / pull) ---
#
# These power the Team-safe `bm cloud push` / `bm cloud pull` commands. Unlike
# the mirror operations (`sync`/`bisync`), they use `rclone copy` so they never
# delete on the destination, and conflicts are surfaced to the caller rather
# than silently resolved. See issue #858 for the full design rationale.
# push = local -> cloud, pull = cloud -> local.
TransferDirection = Literal["push", "pull"]
# How a directional transfer treats files that differ on both sides. "fail" is
# the safe default: the caller is expected to abort before any transfer runs.
ConflictStrategy = Literal["fail", "keep-local", "keep-cloud", "keep-both"]
@dataclass
class TransferPlan:
"""Classification of how local and cloud differ for a directional transfer.
Built from ``rclone check --combined``. Paths are relative to the project
root. ``conflicts`` are files present on both sides with differing content —
without a sync baseline (see #862) every divergence is a conflict, because
we cannot tell a teammate's edit from a stale local copy.
"""
new: list[str] = field(default_factory=list) # only on source → safe to bring over
conflicts: list[str] = field(default_factory=list) # differ on both sides
dest_only: list[str] = field(default_factory=list) # only on destination → left untouched
errors: list[str] = field(default_factory=list) # rclone could not read/hash
def _transfer_endpoints(project: SyncProject, bucket_name: str) -> tuple[str, str]:
"""Return (local_path, remote_path) strings for a project's transfer.
Raises:
RcloneError: If the 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 = str(Path(project.local_sync_path).expanduser())
remote_path = get_project_remote(project, bucket_name)
return local_path, remote_path
def _build_transfer_cmd(
operation: str,
source: str,
dest: str,
*,
filter_path: Path,
dry_run: bool,
verbose: bool,
extra_flags: tuple[str, ...] = (),
) -> list[str]:
"""Build an rclone sync/copy command with the shared Basic Memory flags.
All directional transfers share the same tail: Tigris consistency headers,
the .bmignore filter, and --local-no-preallocate (a no-op when local is the
source, required when local is the destination on pull — see rclone#6801).
"""
cmd = [
"rclone",
operation,
source,
dest,
*TIGRIS_CONSISTENCY_HEADERS,
"--filter-from",
str(filter_path),
# Prevent NUL byte padding on virtual filesystems (e.g. Google Drive File Stream)
# See: rclone/rclone#6801
"--local-no-preallocate",
*extra_flags,
]
if verbose:
cmd.append("--verbose")
else:
cmd.append("--progress")
if dry_run:
cmd.append("--dry-run")
return cmd
def project_sync(
project: SyncProject,
bucket_name: str,
@@ -219,24 +305,199 @@ def project_sync(
remote_path = get_project_remote(project, bucket_name)
filter_path = filter_path or get_bmignore_filter_path()
cmd = [
"rclone",
cmd = _build_transfer_cmd(
"sync",
str(local_path),
remote_path,
filter_path=filter_path,
dry_run=dry_run,
verbose=verbose,
)
result = run(cmd, text=True)
return result.returncode == 0
def _parse_check_combined(output: str) -> TransferPlan:
"""Parse ``rclone check --combined`` output into a TransferPlan.
rclone emits one prefixed line per path (src is the transfer source):
``=`` identical, ``+`` only on src, ``-`` only on dst, ``*`` differ,
``!`` error reading/hashing. We ignore identical files.
"""
plan = TransferPlan()
for line in output.splitlines():
symbol, _, path = line.partition(" ")
path = path.strip()
if not path:
continue
if symbol == "+":
plan.new.append(path)
elif symbol == "*":
plan.conflicts.append(path)
elif symbol == "-":
plan.dest_only.append(path)
elif symbol == "!":
plan.errors.append(path)
# "=" (identical) is intentionally dropped.
return plan
def project_diff(
project: SyncProject,
bucket_name: str,
direction: TransferDirection,
*,
run: RunFunc = subprocess.run,
is_installed: IsInstalledFunc = is_rclone_installed,
filter_path: Path | None = None,
) -> TransferPlan:
"""Classify how local and cloud differ for a push/pull, without transferring.
Uses ``rclone check`` (content comparison) so the caller can surface
conflicts before any data moves. The source side depends on direction:
pull compares cloud→local, push compares local→cloud.
Raises:
RcloneError: If project has no local_sync_path configured or rclone not installed
"""
check_rclone_installed(is_installed=is_installed)
local_path, remote_path = _transfer_endpoints(project, bucket_name)
filter_path = filter_path or get_bmignore_filter_path()
# Source/dest order matters: rclone check reports "+" for files only on the
# source, which is what we want to bring over.
source, dest = (remote_path, local_path) if direction == "pull" else (local_path, remote_path)
cmd = [
"rclone",
"check",
source,
dest,
*TIGRIS_CONSISTENCY_HEADERS,
"--filter-from",
str(filter_path),
# Prevent NUL byte padding on virtual filesystems (e.g. Google Drive File Stream)
# See: rclone/rclone#6801
"--local-no-preallocate",
"--combined",
"-",
]
# rclone check exits non-zero when files differ — that's expected here, so we
# parse the combined listing rather than trusting the return code.
result = run(cmd, capture_output=True, text=True)
plan = _parse_check_combined(result.stdout)
# Trigger: non-zero exit AND the combined listing produced no entries at all.
# Why: a difference always yields +/-/*/! lines, so an empty listing on a
# non-zero exit means the check itself failed (auth, missing remote, network,
# bad filter) rather than finding zero differences. Without this guard the
# caller would see an empty plan, transfer nothing, and report success.
# Outcome: fail fast with rclone's stderr instead of a silent no-op.
if result.returncode != 0 and not (plan.new or plan.conflicts or plan.dest_only or plan.errors):
detail = result.stderr.strip() or f"rclone check exited with code {result.returncode}"
raise RcloneError(f"Failed to compare {project.name} with cloud: {detail}")
return plan
def project_copy(
project: SyncProject,
bucket_name: str,
direction: TransferDirection,
*,
overwrite: bool,
dry_run: bool = False,
verbose: bool = False,
run: RunFunc = subprocess.run,
is_installed: IsInstalledFunc = is_rclone_installed,
filter_path: Path | None = None,
) -> bool:
"""Additive transfer via ``rclone copy`` — never deletes on the destination.
Trigger: ``overwrite=False`` adds ``--ignore-existing`` so files already on
the destination are left as-is (used when the destination side wins a
conflict, and for the no-conflict fast path).
Why: keeps the loser's bytes intact unless the caller explicitly chose to
overwrite, matching the "no surprises" contract.
Raises:
RcloneError: If project has no local_sync_path configured or rclone not installed
"""
check_rclone_installed(is_installed=is_installed)
local_path, remote_path = _transfer_endpoints(project, bucket_name)
filter_path = filter_path or get_bmignore_filter_path()
source, dest = (remote_path, local_path) if direction == "pull" else (local_path, remote_path)
# Overwrite mode compares by checksum so the transfer decision matches
# project_diff's content-based conflict detection (rclone check). Without
# --checksum, copy's default size+modtime comparison could skip a file the
# diff flagged as a conflict (same size, destination not older) — silently
# ignoring the user's explicit keep-cloud/keep-local choice. New-only mode
# uses --ignore-existing, which skips by existence so the comparison basis
# does not matter.
extra_flags = ("--checksum",) if overwrite else ("--ignore-existing",)
cmd = _build_transfer_cmd(
"copy",
source,
dest,
filter_path=filter_path,
dry_run=dry_run,
verbose=verbose,
extra_flags=extra_flags,
)
result = run(cmd, text=True)
return result.returncode == 0
def _conflict_copy_name(rel_path: str, suffix: str) -> str:
"""Insert a ``.conflict-<suffix>`` marker before the extension of a rel path."""
p = PurePosixPath(rel_path)
return str(p.with_name(f"{p.stem}.conflict-{suffix}{p.suffix}"))
def project_copy_file(
project: SyncProject,
bucket_name: str,
direction: TransferDirection,
source_rel_path: str,
dest_rel_path: str,
*,
dry_run: bool = False,
verbose: bool = False,
run: RunFunc = subprocess.run,
is_installed: IsInstalledFunc = is_rclone_installed,
) -> bool:
"""Copy a single file from source to destination under a (possibly renamed) path.
Used for the ``keep-both`` strategy: the incoming version is written beside
the destination's own copy as ``name.conflict-<date>`` so nothing is lost.
Raises:
RcloneError: If project has no local_sync_path configured or rclone not installed
"""
check_rclone_installed(is_installed=is_installed)
local_path, remote_path = _transfer_endpoints(project, bucket_name)
source_root, dest_root = (
(remote_path, local_path) if direction == "pull" else (local_path, remote_path)
)
cmd = [
"rclone",
"copyto",
f"{source_root}/{source_rel_path}",
f"{dest_root}/{dest_rel_path}",
*TIGRIS_CONSISTENCY_HEADERS,
# Matches _build_transfer_cmd: on pull this writes the conflict copy to
# the local filesystem, where this prevents NUL byte padding on virtual
# filesystems (e.g. Google Drive File Stream). See rclone/rclone#6801.
"--local-no-preallocate",
]
if verbose:
cmd.append("--verbose")
else:
cmd.append("--progress")
if dry_run:
cmd.append("--dry-run")
@@ -244,6 +505,72 @@ def project_sync(
return result.returncode == 0
def _strategy_overwrites_dest(direction: TransferDirection, strategy: ConflictStrategy) -> bool:
"""True when the strategy lets the source side overwrite the destination.
The source side is cloud on pull, local on push. "keep-cloud" wins on pull,
"keep-local" wins on push; otherwise the destination is preserved.
"""
if strategy == "keep-cloud":
return direction == "pull"
if strategy == "keep-local":
return direction == "push"
return False # "fail" (no conflicts) and "keep-both" never overwrite existing dest files
def project_transfer(
project: SyncProject,
bucket_name: str,
direction: TransferDirection,
plan: TransferPlan,
*,
strategy: ConflictStrategy = "fail",
conflict_suffix: str = "",
dry_run: bool = False,
verbose: bool = False,
run: RunFunc = subprocess.run,
is_installed: IsInstalledFunc = is_rclone_installed,
filter_path: Path | None = None,
) -> bool:
"""Execute a directional transfer for the chosen conflict strategy.
Callers detect conflicts with ``project_diff`` first and abort when
``strategy == "fail"`` and conflicts exist; this function assumes that gate
has already passed and applies the resolution.
"""
# keep-both: preserve the destination's version and drop the incoming one
# beside it as a conflict copy, then do an additive (new-only) pass.
if strategy == "keep-both":
for rel_path in plan.conflicts:
dest_rel = _conflict_copy_name(rel_path, conflict_suffix)
copied = project_copy_file(
project,
bucket_name,
direction,
rel_path,
dest_rel,
dry_run=dry_run,
verbose=verbose,
run=run,
is_installed=is_installed,
)
if not copied:
return False
overwrite = _strategy_overwrites_dest(direction, strategy)
return project_copy(
project,
bucket_name,
direction,
overwrite=overwrite,
dry_run=dry_run,
verbose=verbose,
run=run,
is_installed=is_installed,
filter_path=filter_path,
)
def project_bisync(
project: SyncProject,
bucket_name: str,