Skip to content

Commit a7937d4

Browse files
committed
fix(cli): compare by checksum on overwrite push/pull to match conflict detection
Second code-review finding (PLAUSIBLE): project_diff detects conflicts with `rclone check` (content/hash), but project_copy used `rclone copy` with its default size+modtime comparison. On an overwrite strategy (keep-cloud on pull / keep-local on push), copy could skip a file the diff had flagged as a conflict when sizes matched and the destination was not older — silently ignoring the user's explicit choice. Add `--checksum` to the overwrite copy so the transfer decision uses the same content basis as detection. New-only mode is unaffected: `--ignore-existing` skips by existence, so the comparison basis is irrelevant there. project_sync (the Personal-only mirror) is untouched and keeps its default comparison. Signed-off-by: phernandez <paul@basicmachines.co>
1 parent 9dbe313 commit a7937d4

2 files changed

Lines changed: 15 additions & 2 deletions

File tree

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,14 @@ def project_copy(
416416
filter_path = filter_path or get_bmignore_filter_path()
417417

418418
source, dest = (remote_path, local_path) if direction == "pull" else (local_path, remote_path)
419-
extra_flags = () if overwrite else ("--ignore-existing",)
419+
# Overwrite mode compares by checksum so the transfer decision matches
420+
# project_diff's content-based conflict detection (rclone check). Without
421+
# --checksum, copy's default size+modtime comparison could skip a file the
422+
# diff flagged as a conflict (same size, destination not older) — silently
423+
# ignoring the user's explicit keep-cloud/keep-local choice. New-only mode
424+
# uses --ignore-existing, which skips by existence so the comparison basis
425+
# does not matter.
426+
extra_flags = ("--checksum",) if overwrite else ("--ignore-existing",)
420427

421428
cmd = _build_transfer_cmd(
422429
"copy",

tests/test_rclone_commands.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,8 @@ def test_project_copy_pull_new_only_adds_ignore_existing(tmp_path):
662662
assert Path(cmd[3]) == Path("/tmp/research")
663663
assert "--ignore-existing" in cmd
664664
assert "--local-no-preallocate" in cmd
665+
# new-only skips by existence, so no content comparison is needed
666+
assert "--checksum" not in cmd
665667

666668

667669
def test_project_copy_pull_overwrite_omits_ignore_existing(tmp_path):
@@ -681,6 +683,8 @@ def test_project_copy_pull_overwrite_omits_ignore_existing(tmp_path):
681683

682684
cmd, _ = runner.calls[0]
683685
assert "--ignore-existing" not in cmd
686+
# overwrite compares by content so it matches hash-based conflict detection
687+
assert "--checksum" in cmd
684688

685689

686690
def test_project_copy_push_uses_local_as_source(tmp_path):
@@ -771,11 +775,13 @@ def test_project_transfer_keep_cloud_on_pull_overwrites_local(tmp_path):
771775
filter_path=filter_path,
772776
)
773777

774-
# keep-cloud + pull → cloud (source) overwrites local (dest): no --ignore-existing
778+
# keep-cloud + pull → cloud (source) overwrites local (dest): no --ignore-existing,
779+
# and --checksum so the overwrite decision matches the content-based conflict.
775780
assert len(runner.calls) == 1
776781
cmd, _ = runner.calls[0]
777782
assert cmd[:2] == ["rclone", "copy"]
778783
assert "--ignore-existing" not in cmd
784+
assert "--checksum" in cmd
779785

780786

781787
def test_project_transfer_keep_local_on_push_overwrites_cloud(tmp_path):

0 commit comments

Comments
 (0)