fix(incremental): add stop_at to find_repo_root / find_project_root (#241)

code_review_graph/incremental.py

@@ -71,35 +71,65 @@
 ]
 
 
-def find_repo_root(start: Path | None = None) -> Optional[Path]:
-    """Walk up from start to find the nearest .git directory."""
+def find_repo_root(
+    start: Path | None = None,
+    stop_at: Path | None = None,
+) -> Optional[Path]:
+    """Walk up from ``start`` to find the nearest ``.git`` directory.
+
+    Args:
+        start: Starting directory.  Defaults to ``Path.cwd()``.
+        stop_at: Optional boundary — if provided, the walk examines
+            ``stop_at`` for a ``.git`` directory and then stops without
+            crossing above it.  Useful for tests that create a synthetic
+            repo under ``tmp_path`` (so the walk does not accidentally
+            climb into a developer's home-directory dotfiles repo) and
+            for any production caller that wants to bound the ancestor
+            walk — e.g. multi-repo orchestrators, CI containers with
+            bind-mounted volumes, embedded sandboxes.  See #241.
+
+    Returns:
+        The first ancestor containing ``.git``, or ``None`` if no
+        ancestor up to and including ``stop_at`` (when set) or the
+        filesystem root (when ``stop_at is None``) contains one.
+    """
     current = start or Path.cwd()
     while current != current.parent:
         if (current / ".git").exists():
             return current
+        if stop_at is not None and current == stop_at:
+            return None
         current = current.parent
     if (current / ".git").exists():
         return current
     return None
 
 
-def find_project_root(start: Path | None = None) -> Path:
+def find_project_root(
+    start: Path | None = None,
+    stop_at: Path | None = None,
+) -> Path:
     """Find the project root.
 
     Resolution order (highest precedence first):
 
     1. ``CRG_REPO_ROOT`` environment variable — explicit override for
        anyone scripting the CLI from outside the repo (CI jobs, daemons,
        multi-repo orchestrators). See: #155
-    2. Git repository root via :func:`find_repo_root` from ``start``.
+    2. Git repository root via :func:`find_repo_root` from ``start``,
+       honoring ``stop_at`` if provided.
     3. ``start`` itself (or cwd if no start given).
+
+    ``stop_at`` is forwarded to :func:`find_repo_root` so callers that
+    want to bound the ancestor walk (typically tests; see #241) can do so
+    without having to call ``find_repo_root`` directly.
     """
     env_override = os.environ.get("CRG_REPO_ROOT", "").strip()
     if env_override:
         p = Path(env_override).expanduser().resolve()
         if p.exists():
             return p
-    root = find_repo_root(start)
+    root = find_repo_root(start, stop_at=stop_at)
     if root:
         return root
     return start or Path.cwd()

tests/test_incremental.py

@@ -35,20 +35,83 @@ def test_finds_parent_git_dir(self, tmp_path):
         assert find_repo_root(sub) == tmp_path
 
     def test_returns_none_without_git(self, tmp_path):
+        """No .git between ``sub`` and ``tmp_path`` -> None.
+
+        Bounded with ``stop_at=tmp_path`` so the walk does not climb into
+        ancestors outside the test sandbox.  On Windows in particular,
+        ``tmp_path`` lives under ``C:/Users/<user>/AppData/Local/Temp/...``
+        and if the user has ``git init`` anywhere under their home (dotfiles,
+        chezmoi, etc.) the unbounded walk would find that ancestor .git and
+        the test would fail for reasons unrelated to the product.  See #241.
+        """
         sub = tmp_path / "no_git"
         sub.mkdir()
-        assert find_repo_root(sub) is None
+        assert find_repo_root(sub, stop_at=tmp_path) is None
+
+    def test_stop_at_prevents_escape_to_outer_git(self, tmp_path):
+        """Positive regression test for #241: ``stop_at`` must halt the
+        walk even when an ancestor *does* contain ``.git``.
+
+        Without ``stop_at`` the walk correctly finds the outer .git; with
+        ``stop_at=inner`` the walk is bounded and returns None.
+        """
+        outer = tmp_path / "outer"
+        outer.mkdir()
+        (outer / ".git").mkdir()
+        inner = outer / "inner"
+        inner.mkdir()
+
+        # Unbounded walk finds the ancestor .git (existing behavior).
+        assert find_repo_root(inner) == outer
+
+        # Bounded walk stops at ``inner`` and never climbs to ``outer``.
+        assert find_repo_root(inner, stop_at=inner) is None
+
+    def test_stop_at_finds_git_at_boundary(self, tmp_path):
+        """stop_at does not suppress a .git that lives *at* the boundary."""
+        boundary = tmp_path / "boundary"
+        boundary.mkdir()
+        (boundary / ".git").mkdir()
+        inner = boundary / "inner"
+        inner.mkdir()
+
+        # The walk examines ``boundary`` and finds the .git before stopping.
+        assert find_repo_root(inner, stop_at=boundary) == boundary
 
 
 class TestFindProjectRoot:
     def test_returns_git_root(self, tmp_path):
         (tmp_path / ".git").mkdir()
         assert find_project_root(tmp_path) == tmp_path
 
-    def test_falls_back_to_start(self, tmp_path):
+    def test_falls_back_to_start(self, tmp_path, monkeypatch):
+        """With no .git and no env override, find_project_root returns ``sub``.
+
+        Bounded with ``stop_at=tmp_path`` to prevent the ancestor walk from
+        escaping the test sandbox (see #241), and ``CRG_REPO_ROOT`` is
+        cleared so a developer env var cannot shadow the test expectation.
+        """
+        monkeypatch.delenv("CRG_REPO_ROOT", raising=False)
         sub = tmp_path / "no_git"
         sub.mkdir()
-        assert find_project_root(sub) == sub
+        assert find_project_root(sub, stop_at=tmp_path) == sub
+
+    def test_stop_at_forwarded_to_find_repo_root(self, tmp_path, monkeypatch):
+        """Positive regression test for #241: find_project_root must forward
+        stop_at to find_repo_root, not silently drop it."""
+        monkeypatch.delenv("CRG_REPO_ROOT", raising=False)
+        outer = tmp_path / "outer"
+        outer.mkdir()
+        (outer / ".git").mkdir()
+        inner = outer / "inner"
+        inner.mkdir()
+
+        # Without stop_at, find_project_root climbs to outer (existing behavior).
+        assert find_project_root(inner) == outer
+
+        # With stop_at=inner, the walk is bounded and find_project_root falls
+        # back to its third resolution rule (the start path itself).
+        assert find_project_root(inner, stop_at=inner) == inner
 
 
 class TestGetDbPath: