From 740d2cac89ecccb11f4948a8d3ea3e328085b2cf Mon Sep 17 00:00:00 2001 From: Joris Wouter Jonkers Date: Thu, 10 Sep 2026 13:57:25 +0200 Subject: [PATCH] fix(sidecar): adopt a non-empty non-git vault dir instead of failing to clone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On first boot the shared vault PVC is not empty: the Basic Memory app initialises the vault tree into the PVC root before the git-sync sidecar starts (they share one RWO volume, and Basic Memory owns the tree). The sidecar's attach() used clone_from, which refuses a non-empty target with "destination path '...' already exists and is not an empty directory", crash-looping the pod. Change attach() so that when the vault dir exists but is not a git repo, it `git init` (which adopts an existing directory), adds the canonical origin, fetches the branch and checks it out — mirroring a clone without requiring an empty target, and without discarding anything already on the volume. This is the correct behaviour for "Basic Memory owns the tree, the sidecar is its git backstop", and matches the AC that a reschedule (fresh mount) must not lose the notes. New unit test reproduces the production shape (a `secrets/` subdir already in the vault root, as the vault-agent overlay or Basic Memory's own init leaves) and asserts the sidecar attaches, keeps the existing dir, and has the origin. 13 tests pass (83% coverage), ruff + mypy clean. --- .../src/basic_memory_git_sync/sync.py | 45 ++++++++++++++----- basic-memory-git-sync/tests/unit/test_sync.py | 21 +++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/basic-memory-git-sync/src/basic_memory_git_sync/sync.py b/basic-memory-git-sync/src/basic_memory_git_sync/sync.py index b966a77..0b7dace 100644 --- a/basic-memory-git-sync/src/basic_memory_git_sync/sync.py +++ b/basic-memory-git-sync/src/basic_memory_git_sync/sync.py @@ -83,20 +83,45 @@ def __init__( # -- lifecycle -- def attach(self) -> None: - """Attach to the existing checkout, or clone it on first boot.""" + """Attach to the existing checkout, or initialise + fetch on first boot. + + On a fresh PVC the directory is usually empty, so we clone. But the + shared vault PVC can also be created non-empty-but-not-a-repo — e.g. + when another container (or the vault-agent secrets mount) has already + touched the mount root — in which case ``clone_from`` would fail with + "destination path exists and is not an empty directory". We treat + "exists, not a git repo" as a repo to initialise: ``git init`` can + adopt an existing directory, then we add the origin and fetch the + branch. This makes first boot robust to the shared-PVC layout without + discarding anything already on the volume. + """ if self._vault_dir.exists() and (self._vault_dir / ".git").exists(): self._repo = Repo(self._vault_dir) self._log.info("backstop.attached", dir=str(self._vault_dir)) self._repo.remotes.origin.fetch() - else: - self._vault_dir.parent.mkdir(parents=True, exist_ok=True) - self._repo = Repo.clone_from( - self._clone_url, - self._vault_dir, - branch=self._branch, - env=self._git_env(), - ) - self._log.info("backstop.cloned", dir=str(self._vault_dir)) + return + + self._vault_dir.parent.mkdir(parents=True, exist_ok=True) + if self._vault_dir.exists() and not (self._vault_dir / ".git").exists(): + # Adopt the existing (non-git) directory instead of clone_from, + # which refuses a non-empty target. init + remote + fetch mirrors + # a clone without requiring an empty target. + repo = Repo.init(self._vault_dir) + repo.create_remote("origin", self._clone_url) + with repo.git.custom_environment(**self._git_env()): + repo.git.fetch("origin", self._branch) + repo.git.checkout("-B", self._branch, f"origin/{self._branch}") + self._repo = repo + self._log.info("backstop.init_adopted", dir=str(self._vault_dir)) + return + + self._repo = Repo.clone_from( + self._clone_url, + self._vault_dir, + branch=self._branch, + env=self._git_env(), + ) + self._log.info("backstop.cloned", dir=str(self._vault_dir)) def close(self) -> None: self._repo = None diff --git a/basic-memory-git-sync/tests/unit/test_sync.py b/basic-memory-git-sync/tests/unit/test_sync.py index 0aecb71..9350692 100644 --- a/basic-memory-git-sync/tests/unit/test_sync.py +++ b/basic-memory-git-sync/tests/unit/test_sync.py @@ -68,6 +68,27 @@ def test_attach_reuses_existing_checkout(tmp_path: Path, remote: Path) -> None: b.attach() # no error +def test_attach_adopts_existing_non_git_dir(tmp_path: Path, remote: Path) -> None: + """First boot on a shared PVC root that is non-empty but not a git repo. + + Reproduces the production failure: another container (or the vault-agent + secrets mount) has already written into the vault PVC root, so + ``clone_from`` would refuse per "destination path exists and is not an + empty directory". attach() must init + fetch into the adopted dir instead + of failing, without discarding what is already on the volume. + """ + v = tmp_path / "vault" + v.mkdir(parents=True) + (v / "secrets").mkdir() # e.g. the vault-agent secrets overlay + b = VaultGitBackstop(clone_url=str(remote), vault_dir=v, push=False) + b.attach() + assert (v / ".git").exists() + assert (v / "secrets").exists() # not discarded + # The remote branch was fetched + checked out; the sidecar can now push writes. + assert b._repo.active_branch.name == "main" + assert b._repo.remotes.origin.url == str(remote) + + def test_poll_commits_nothing_when_clean(backstop: VaultGitBackstop) -> None: result = backstop.poll_once() assert result.committed is False