fix(memory): keep sample import backups unique per run (#7418)

* fix(memory): keep sample import backups unique per run

* docs(memory): clarify backup retention and align script format

---------

Co-authored-by: Kiko0301 <191240073+Kiko0301@users.noreply.github.com>

README.md

@@ -768,7 +768,7 @@ such a checkout, use `bash ./scripts/<name>.sh ...`.
    uv run python ../scripts/load_memory_sample.py --all-users
    ```
 
-   Bulk mode supports SQLite/PostgreSQL user registries, creates timestamped backups under `.deer-flow/memory-sample-backups/`, and rejects the non-persistent `database.backend: memory` mode. See [backend/docs/MEMORY_SETTINGS_REVIEW.md](backend/docs/MEMORY_SETTINGS_REVIEW.md) for the complete review flow.
+   Bulk mode supports SQLite/PostgreSQL user registries, creates a distinct timestamped backup directory for every import under `.deer-flow/memory-sample-backups/`, and rejects the non-persistent `database.backend: memory` mode. See [backend/docs/MEMORY_SETTINGS_REVIEW.md](backend/docs/MEMORY_SETTINGS_REVIEW.md) for the complete review flow.
 
 Local services always use their internal ports (`8001`, `3000`, and `2026`).
 The root `.env` variable `PORT` configures only the published Docker ingress;

backend/docs/MEMORY_SETTINGS_REVIEW.md

@@ -46,11 +46,11 @@ uv run python ../scripts/load_memory_sample.py --all-users
 This command:
 
 - supports SQLite and PostgreSQL registered-user databases;
-- creates timestamped backups under `.deer-flow/memory-sample-backups/` before replacing memory;
+- creates a unique timestamped backup directory under `.deer-flow/memory-sample-backups/` before replacing memory, even when imports start in the same second;
 - imports through the configured memory storage provider; and
 - rejects `database.backend: memory`, which has no persistent user registry.
 
-Bulk loading replaces every registered user's memory. Do not run it against an environment containing memory you need to preserve. Pass `--no-backup` only when losing the existing memory is acceptable.
+Bulk loading replaces every registered user's memory. Do not run it against an environment containing memory you need to preserve. Backups accumulate until removed manually. On POSIX, each new backup directory is private to the creating OS user (mode 0700); restoring as another user may require adjusting permissions. Pass `--no-backup` only when losing the existing memory is acceptable.
 
 ## Minimal Manual Test
 
@@ -79,6 +79,6 @@ Bulk loading replaces every registered user's memory. Do not run it against an e
 
 - Sample fixture: `backend/docs/memory-settings-sample.json`
 - Per-user file-storage target: `backend/.deer-flow/users/{user_id}/memory.json`
-- Bulk backups: `backend/.deer-flow/memory-sample-backups/{timestamp}/{user_id}.json`
+- Bulk backups: `backend/.deer-flow/memory-sample-backups/{timestamp}-{unique-suffix}/{user_id}.json`
 
-For an explicit one-file copy, pass `--target PATH` to the loader. There is no implicit target because authenticated sessions use different per-user paths.
+For an explicit one-file copy, pass `--target PATH` to the loader. Its sidecar backup is `{target.name}.bak-{timestamp}-{unique-suffix}` beside the target; repeated imports never reuse a backup path. There is no implicit target because authenticated sessions use different per-user paths.

backend/tests/test_load_memory_sample.py

@@ -2,6 +2,7 @@ from __future__ import annotations
 
 import importlib.util
 import json
+from datetime import datetime
 from pathlib import Path
 from types import SimpleNamespace
 
@@ -113,6 +114,41 @@ def test_load_sample_for_users_rejects_unpersisted_sample(tmp_path):
         )
 
 
+def test_target_backups_keep_both_copies_when_imports_share_a_second(monkeypatch, tmp_path):
+    source = tmp_path / "sample.json"
+    target = tmp_path / "memory.json"
+    source.write_text(json.dumps({"facts": [{"id": "sample"}]}), encoding="utf-8")
+    target.write_text(json.dumps({"facts": [{"id": "original"}]}), encoding="utf-8")
+    monkeypatch.setattr(loader, "datetime", SimpleNamespace(now=lambda: datetime(2026, 10, 24, 12, 0, 0)))
+
+    args = ["--source", str(source), "--target", str(target)]
+    assert loader.main(args) == 0
+    first_backup = next(tmp_path.glob("memory.json.bak-*"))
+    assert json.loads(first_backup.read_text(encoding="utf-8"))["facts"][0]["id"] == "original"
+
+    assert loader.main(args) == 0
+    backups = list(tmp_path.glob("memory.json.bak-*"))
+    assert len(backups) == 2
+    assert json.loads(first_backup.read_text(encoding="utf-8"))["facts"][0]["id"] == "original"
+    second_backup = next(path for path in backups if path != first_backup)
+    assert json.loads(second_backup.read_text(encoding="utf-8"))["facts"][0]["id"] == "sample"
+
+
+def test_target_backup_removes_reserved_file_if_copy_fails(monkeypatch, tmp_path):
+    target = tmp_path / "memory.json"
+    target.write_text("original", encoding="utf-8")
+
+    def fail_copy(_source, _destination):
+        raise OSError("copy failed")
+
+    monkeypatch.setattr(loader.shutil, "copy2", fail_copy)
+    with pytest.raises(OSError, match="copy failed"):
+        loader.create_target_backup(target)
+
+    assert list(tmp_path.glob("memory.json.bak-*")) == []
+    assert target.read_text(encoding="utf-8") == "original"
+
+
 def test_load_sample_for_all_users_uses_runtime_config_resolution(monkeypatch, tmp_path):
     import asyncio
 
@@ -147,14 +183,16 @@ def test_load_sample_for_all_users_uses_configured_memory_manager(monkeypatch, t
     manager_factory_calls = 0
     loaded = []
     imported = []
+    current = {user_id: {"facts": [{"id": f"old-{user_id}"}]} for user_id in ("u1", "u2")}
 
     class FakeMemoryManager:
         def get_memory(self, *, user_id):
             loaded.append(user_id)
-            return {"facts": [{"id": f"old-{user_id}"}]}
+            return current[user_id]
 
         def import_memory(self, sample, *, user_id):
             imported.append((user_id, sample))
+            current[user_id] = sample
             return sample
 
     manager = FakeMemoryManager()
@@ -188,6 +226,7 @@ def test_load_sample_for_all_users_uses_configured_memory_manager(monkeypatch, t
     monkeypatch.setattr(persistence_engine, "init_engine_from_config", init_engine_from_config)
     monkeypatch.setattr(persistence_engine, "get_session_factory", object)
     monkeypatch.setattr(persistence_engine, "close_engine", close_engine)
+    monkeypatch.setattr(loader, "datetime", SimpleNamespace(now=lambda: datetime(2026, 10, 24, 12, 0, 0)))
 
     sample = {"facts": [{"id": "sample"}]}
     count, backup_root = asyncio.run(
@@ -202,6 +241,16 @@ def test_load_sample_for_all_users_uses_configured_memory_manager(monkeypatch, t
     assert json.loads((backup_root / "u1.json").read_text())["facts"][0]["id"] == "old-u1"
     assert json.loads((backup_root / "u2.json").read_text())["facts"][0]["id"] == "old-u2"
 
+    second_count, second_backup_root = asyncio.run(
+        loader.load_sample_for_all_users(SCRIPT_PATH.parents[1], sample, no_backup=False),
+    )
+    assert second_count == 2
+    assert second_backup_root is not None and second_backup_root != backup_root
+    assert json.loads((backup_root / "u1.json").read_text())["facts"][0]["id"] == "old-u1"
+    assert json.loads((backup_root / "u2.json").read_text())["facts"][0]["id"] == "old-u2"
+    assert json.loads((second_backup_root / "u1.json").read_text())["facts"][0]["id"] == "sample"
+    assert json.loads((second_backup_root / "u2.json").read_text())["facts"][0]["id"] == "sample"
+
 
 def test_require_persistent_database_rejects_memory():
     with pytest.raises(SystemExit, match="sqlite or postgres"):

scripts/load_memory_sample.py

@@ -7,8 +7,10 @@ import argparse
 import asyncio
 import copy
 import json
+import os
 import shutil
 import sys
+import tempfile
 from collections.abc import Callable
 from datetime import datetime
 from pathlib import Path
@@ -61,6 +63,28 @@ def require_persistent_database(backend: str) -> None:
         raise SystemExit("--all-users requires database.backend sqlite or postgres")
 
 
+def create_target_backup(target: Path) -> Path:
+    """Reserve a unique sidecar before copying so earlier backups survive."""
+    timestamp = datetime.now().strftime("%Y%m%d-%H%M%S")
+    fd, name = tempfile.mkstemp(prefix=f"{target.name}.bak-{timestamp}-", dir=target.parent)
+    os.close(fd)
+    backup = Path(name)
+    try:
+        shutil.copy2(target, backup)
+    except BaseException:
+        backup.unlink(missing_ok=True)
+        raise
+    return backup
+
+
+def reserve_bulk_backup_root(base_dir: Path) -> Path:
+    """Create a unique backup directory before any user memory is replaced."""
+    parent = base_dir / "memory-sample-backups"
+    parent.mkdir(parents=True, exist_ok=True)
+    timestamp = datetime.now().strftime("%Y%m%d-%H%M%S")
+    return Path(tempfile.mkdtemp(prefix=f"{timestamp}-", dir=parent))
+
+
 def load_sample_for_users(
     sample: dict[str, Any],
     user_ids: list[str],
@@ -135,7 +159,7 @@ async def load_sample_for_all_users(
         user_ids = await SQLiteUserRepository(session_factory).list_user_ids()
         backup_root = None
         if user_ids and not no_backup:
-            backup_root = get_paths().base_dir / "memory-sample-backups" / datetime.now().strftime("%Y%m%d-%H%M%S")
+            backup_root = await asyncio.to_thread(reserve_bulk_backup_root, get_paths().base_dir)
 
         count = await asyncio.to_thread(
             load_sample_for_users,
@@ -178,9 +202,7 @@ def main(argv: list[str] | None = None) -> int:
 
     backup_path: Path | None = None
     if target.exists() and not args.no_backup:
-        timestamp = datetime.now().strftime("%Y%m%d-%H%M%S")
-        backup_path = target.with_name(f"{target.name}.bak-{timestamp}")
-        shutil.copy2(target, backup_path)
+        backup_path = create_target_backup(target)
 
     shutil.copy2(source, target)