fix(updater): route every git step through app_dir for separate-mount installs (#1715)

  Native installs that follow the systemd template
    WorkingDirectory=/opt/bambuddy
    Environment="DATA_DIR=/srv/bambuddy/data"
  (or any layout where DATA_DIR is not a subdirectory of the install)
  could not apply in-app updates. Every git subprocess in _perform_update
  used cwd=settings.base_dir and safe.directory={base_dir}. On standard
  installs (DATA_DIR=INSTALL_PATH/data) this happened to work by accident
  because git walks up from a subdirectory of the repo to find .git; on
  separate-mount layouts the walk has nowhere to go and every call
  returns "fatal: not a git repository." safe.directory was also wrong
  even on the standard install -- it must equal the repo root git
  discovers, not the data dir.

  Resolve app_dir = settings.app_dir at the top of _perform_update and
  route all four git subprocesses (remote get-url, remote set-url, fetch,
  reset --hard) and the embedded safe.directory through it. Rename the
  base_dir parameter on _origin_points_at_repo to app_dir so the
  signature documents the contract.

backend/app/api/routes/updates.py

@@ -181,12 +181,16 @@ def _parse_github_remote(url: str) -> tuple[str, str] | None:
     return (parts[0], parts[1])
 
 
-async def _origin_points_at_repo(git_path: str, git_config: list[str], base_dir, expected_repo: str) -> bool:
+async def _origin_points_at_repo(git_path: str, git_config: list[str], app_dir, expected_repo: str) -> bool:
     """Return True iff the working tree's `origin` already resolves to
     `<owner>/<repo>` matching `expected_repo` (e.g. "maziggy/bambuddy"),
     regardless of whether it's the SSH or HTTPS form. Used to skip the
     `git remote set-url origin https://...` rewrite when the developer's
-    SSH origin is already correct — see `_perform_update` for context."""
+    SSH origin is already correct — see `_perform_update` for context.
+
+    ``app_dir`` is the working tree (where ``.git`` lives), not the data
+    dir — see #1715 for the separate-mount layout that proved why this
+    must NOT be ``base_dir``."""
     try:
         process = await asyncio.create_subprocess_exec(
             git_path,
@@ -194,7 +198,7 @@ async def _origin_points_at_repo(git_path: str, git_config: list[str], base_dir,
             "remote",
             "get-url",
             "origin",
-            cwd=str(base_dir),
+            cwd=str(app_dir),
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
@@ -555,7 +559,16 @@ async def _perform_update(target_ref: str):
     global _update_status
 
     try:
-        base_dir = settings.base_dir
+        # Every git step runs against the working tree (app_dir), NOT base_dir.
+        # On a standard install with DATA_DIR=INSTALL_PATH/data, git happens
+        # to walk up from a subdirectory of the repo to find .git so cwd=base_dir
+        # used to silently work — but only by accident. On a native install with
+        # DATA_DIR mounted at an unrelated path (e.g. /srv/bambuddy/data while
+        # the install is /opt/bambuddy — see #1715), git can't walk up and every
+        # operation fails with "not a git repository". safe.directory has the
+        # same requirement: it must equal the repo root git discovers, not the
+        # data dir, or every call returns "fatal: detected dubious ownership."
+        app_dir = settings.app_dir
 
         # Find git executable (may not be in PATH when running as systemd service)
         git_path = _find_executable("git")
@@ -570,8 +583,9 @@ async def _perform_update(target_ref: str):
 
         logger.info("Using git at: %s", git_path)
 
-        # Git config to avoid safe.directory issues
-        git_config = ["-c", f"safe.directory={base_dir}"]
+        # Git config to avoid safe.directory issues — must point at the working
+        # tree (where .git lives), see app_dir comment above.
+        git_config = ["-c", f"safe.directory={app_dir}"]
 
         _update_status = {
             "status": "downloading",
@@ -593,7 +607,7 @@ async def _perform_update(target_ref: str):
         # correct repo are preserved; only missing / wrong / corrupted
         # origins get reset to HTTPS.
         https_url = f"https://github.com/{GITHUB_REPO}.git"
-        if not await _origin_points_at_repo(git_path, git_config, base_dir, GITHUB_REPO):
+        if not await _origin_points_at_repo(git_path, git_config, app_dir, GITHUB_REPO):
             process = await asyncio.create_subprocess_exec(
                 git_path,
                 *git_config,
@@ -601,7 +615,7 @@ async def _perform_update(target_ref: str):
                 "set-url",
                 "origin",
                 https_url,
-                cwd=str(base_dir),
+                cwd=str(app_dir),
                 stdout=asyncio.subprocess.PIPE,
                 stderr=asyncio.subprocess.PIPE,
             )
@@ -635,7 +649,7 @@ async def _perform_update(target_ref: str):
             "--tags",
             "--force",
             "origin",
-            cwd=str(base_dir),
+            cwd=str(app_dir),
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
@@ -671,7 +685,7 @@ async def _perform_update(target_ref: str):
             "reset",
             "--hard",
             target_ref,
-            cwd=str(base_dir),
+            cwd=str(app_dir),
             stdout=asyncio.subprocess.PIPE,
             stderr=asyncio.subprocess.PIPE,
         )
@@ -696,12 +710,9 @@ async def _perform_update(target_ref: str):
         }
 
         # Install Python dependencies — must run from the source-code directory
-        # (where requirements.txt lives), not the data dir. On native installs
-        # systemd sets DATA_DIR=INSTALL_PATH/data, so `base_dir` is the data dir,
-        # not the working tree. `git reset` above worked from base_dir because
-        # git walks up looking for .git, but `pip install -r requirements.txt`
-        # needs the file in cwd literally.
-        app_dir = settings.app_dir
+        # (where requirements.txt lives). app_dir is already resolved at the top
+        # of this function; see the comment there for why every step uses it
+        # instead of base_dir.
         process = await asyncio.create_subprocess_exec(
             sys.executable,
             "-m",

backend/tests/integration/test_updates_api.py

@@ -569,3 +569,71 @@ class TestUpdatesAPI:
         # at the captured cwd. If this fails the cwd is wrong even if it isn't
         # base_dir — useful diagnostic if someone refactors path handling.
         assert (Path(pip_cwd) / "requirements.txt").exists()
+
+    @pytest.mark.asyncio
+    async def test_perform_update_runs_git_in_app_dir_when_data_dir_on_separate_mount(self, tmp_path):
+        """Regression for #1715: when DATA_DIR is on a path separate from the
+        install (e.g. WorkingDirectory=/opt/bambuddy + DATA_DIR=/srv/bambuddy/data),
+        ``base_dir`` and the repo working tree are on different mounts. Pre-fix,
+        every git subprocess (`remote get-url`, `remote set-url`, `fetch`,
+        `reset --hard`) used ``cwd=base_dir`` — and git could no longer walk up
+        to find ``.git`` because the data dir is not a subdir of the repo.
+        Every update failed with "not a git repository". The fix routes every
+        git step (and the embedded ``safe.directory`` config) through
+        ``app_dir`` instead. This test pins the cwd of all four git steps so a
+        future refactor that re-introduces ``base_dir`` for any of them surfaces
+        loudly here instead of silently re-breaking native installs."""
+        from backend.app.api.routes import updates as updates_module
+
+        # Separate-mount layout: app_dir and data_dir are SIBLINGS, not parent/
+        # child. base_dir is not under app_dir, so git cannot walk up.
+        app_dir = tmp_path / "opt" / "bambuddy"
+        data_dir = tmp_path / "srv" / "bambuddy" / "data"
+        app_dir.mkdir(parents=True)
+        data_dir.mkdir(parents=True)
+        (app_dir / "requirements.txt").write_text("fastapi\n")
+
+        calls: list[dict] = []
+
+        async def fake_create_subprocess_exec(*args, **kwargs):
+            calls.append({"args": args, "cwd": kwargs.get("cwd")})
+            proc = MagicMock()
+            if "get-url" in args and "origin" in args:
+                proc.communicate = AsyncMock(return_value=(b"git@github.com:maziggy/bambuddy.git\n", b""))
+            else:
+                proc.communicate = AsyncMock(return_value=(b"", b""))
+            proc.returncode = 0
+            return proc
+
+        with (
+            patch.object(updates_module.settings, "base_dir", data_dir),
+            patch.object(updates_module.settings, "app_dir", app_dir),
+            patch.object(updates_module, "_find_executable", return_value="/usr/bin/git"),
+            patch.object(
+                updates_module.asyncio,
+                "create_subprocess_exec",
+                side_effect=fake_create_subprocess_exec,
+            ),
+        ):
+            await updates_module._perform_update("v0.2.4b1")
+
+        # Every git subprocess must run in app_dir (the working tree). A
+        # regression to base_dir would silently break #1715-class installs.
+        git_calls = [c for c in calls if c["args"] and c["args"][0] == "/usr/bin/git"]
+        assert git_calls, "no git subprocess was invoked; setup is wrong"
+        wrong_cwd = [c for c in git_calls if c["cwd"] != str(app_dir)]
+        assert not wrong_cwd, (
+            "git subprocess ran with cwd != app_dir; #1715 would resurface. "
+            f"Offending calls: {[(c['args'][1:5], c['cwd']) for c in wrong_cwd]}"
+        )
+
+        # ``safe.directory`` must equal app_dir (the repo root git discovers),
+        # not the data dir — otherwise git refuses with "dubious ownership"
+        # even when the cwd is technically correct.
+        safe_dir_configs = [
+            arg for c in git_calls for arg in c["args"] if isinstance(arg, str) and arg.startswith("safe.directory=")
+        ]
+        assert safe_dir_configs, "safe.directory config was never set on git calls"
+        assert all(s == f"safe.directory={app_dir}" for s in safe_dir_configs), (
+            f"safe.directory must point at app_dir ({app_dir}); got {safe_dir_configs}"
+        )