From 1fc338a4ec7befc5f9b24039cbe3df58666eb4e9 Mon Sep 17 00:00:00 2001
From: Frowtek <frowte3k@gmail.com>
Date: Wed, 22 Jul 2026 20:12:58 +0300
Subject: [PATCH] fix(checkpoints): an empty surviving mount point is not
 evidence of deletion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses @egilewski's review: the parent-directory check still deleted
checkpoint history for the most common unmount layout.

Detaching storage removes the parent outright in some layouts
(`/Volumes/Ext/proj` on macOS, `/media/<user>/<label>/proj`), which the first
commit handles. But in the classic static layout — `/mnt/volume/proj`, an
fstab entry, a container bind-mount — unmounting removes the contents and
leaves the mount point behind as an empty directory. `parent.is_dir()` is then
true, the project is absent, and the startup sweep deletes its ref, index and
metadata: exactly the case this PR set out to protect.

Reproduced against the real predicate before this commit:

    mount root vanished (macOS)   -> False   ok
    empty surviving mount point   -> True    <-- history deleted
    really deleted (siblings)     -> True    ok

An empty parent carries no information: it looks identical whether the volume
was detached or the project was deleted. So require the parent to actually say
something — it holds some other entry (we observed a populated directory that
does not contain the project), or it is itself a live mount point (the volume
is attached right now and demonstrably does not hold the project).

The cost is that a project deleted out of an otherwise-empty parent is no
longer reclaimed by the orphan rule. It is not leaked: the retention rule
reads `last_touch` rather than probing the filesystem and still collects it,
so reclamation is deferred, not lost. That is the right direction for a
predicate whose false positive destroys a user's restore points unattended.

`_dir_has_any_entry` stops at the first entry via `os.scandir` instead of
materializing a listing, since a project root can hold a large tree.

tests/tools/test_checkpoint_manager.py: `test_surviving_empty_mountpoint_
keeps_its_checkpoints` pins the reviewed case, and `test_empty_parent_project_
is_still_reclaimed_by_retention` pins the deferral above so the safety valve
cannot silently regress into a leak. Both fail on the previous commit. The
real-orphan control now seeds a sibling so it exercises a populated parent
rather than the ambiguous empty one. 80 passed in the checkpoint suite; the 2
remaining failures (`TestGitEnvIsolation`, `TestClearFunctions`) fail
identically on clean main.
---
 tests/tools/test_checkpoint_manager.py | 66 +++++++++++++++++++++++++-
 tools/checkpoint_manager.py            | 47 +++++++++++++++---
 2 files changed, 105 insertions(+), 8 deletions(-)

diff --git a/tests/tools/test_checkpoint_manager.py b/tests/tools/test_checkpoint_manager.py
index 6a0e3f6cd4c3..2223cc636912 100644
--- a/tests/tools/test_checkpoint_manager.py
+++ b/tests/tools/test_checkpoint_manager.py
@@ -1211,14 +1211,78 @@ class TestOrphanPruneRequiresObservableDeletion:
             "merely unmounted"
         )
 
+    def test_surviving_empty_mountpoint_keeps_its_checkpoints(
+        self, tmp_path, checkpoint_base, monkeypatch,
+    ):
+        """The mount point outlives the volume as an empty dir → keep history.
+
+        The classic static layout (``/mnt/volume/proj``, an fstab entry, a
+        container bind-mount) does not remove the mount point on unmount: the
+        project disappears but ``/mnt/volume`` stays behind as an empty
+        directory. The parent being present is therefore not evidence on its
+        own — an empty parent looks exactly the same whether the volume was
+        detached or the project was deleted.
+        """
+        mount_point = tmp_path / "mnt" / "volume"
+        work_dir = mount_point / "proj"
+        work_dir.mkdir(parents=True)
+        (work_dir / "main.py").write_text("print('x')\n")
+        self._project_with_history(work_dir, checkpoint_base, monkeypatch)
+
+        # Unmount: contents go, the mount point itself remains.
+        shutil.rmtree(work_dir)
+        assert mount_point.is_dir() and not any(mount_point.iterdir())
+
+        prune_checkpoints(
+            retention_days=0, delete_orphans=True, checkpoint_base=checkpoint_base,
+        )
+
+        assert self._history_survives(checkpoint_base, work_dir), (
+            "checkpoint history was deleted for a project whose mount point "
+            "merely survived the unmount as an empty directory"
+        )
+
+    def test_empty_parent_project_is_still_reclaimed_by_retention(
+        self, tmp_path, checkpoint_base, monkeypatch,
+    ):
+        """Skipping an ambiguous parent defers reclamation, it does not lose it.
+
+        A project deleted out of an otherwise-empty parent is indistinguishable
+        from an unmounted volume, so orphan pruning leaves it alone — but the
+        retention rule, which reads ``last_touch`` instead of probing the
+        filesystem, still reclaims it.
+        """
+        parent = tmp_path / "solo"
+        work_dir = parent / "proj"
+        work_dir.mkdir(parents=True)
+        (work_dir / "main.py").write_text("print('x')\n")
+        self._project_with_history(work_dir, checkpoint_base, monkeypatch)
+
+        shutil.rmtree(work_dir)
+        assert parent.is_dir() and not any(parent.iterdir())
+
+        store = _store_path(checkpoint_base)
+        meta_path = _project_meta_path(store, _project_hash(str(work_dir)))
+        meta = json.loads(meta_path.read_text())
+        meta["last_touch"] = time.time() - 60 * 86400
+        meta_path.write_text(json.dumps(meta))
+
+        result = prune_checkpoints(
+            retention_days=30, delete_orphans=True, checkpoint_base=checkpoint_base,
+        )
+
+        assert result["deleted_stale"] >= 1
+        assert not self._history_survives(checkpoint_base, work_dir)
+
     def test_genuinely_deleted_project_is_still_pruned(
         self, tmp_path, checkpoint_base, monkeypatch,
     ):
-        """Control: parent present, project gone → a real orphan, still pruned."""
+        """Control: a populated parent without the project → a real orphan."""
         parent = tmp_path / "projects"
         work_dir = parent / "proj"
         work_dir.mkdir(parents=True)
         (work_dir / "main.py").write_text("print('x')\n")
+        (parent / "other-project").mkdir()
         self._project_with_history(work_dir, checkpoint_base, monkeypatch)
 
         shutil.rmtree(work_dir)
diff --git a/tools/checkpoint_manager.py b/tools/checkpoint_manager.py
index 6aec5e2e6d8d..e35f1282fcb3 100644
--- a/tools/checkpoint_manager.py
+++ b/tools/checkpoint_manager.py
@@ -1304,12 +1304,25 @@ def _workdir_is_observably_gone(workdir: str) -> bool:
     "deleted" throws away the user's restore points over a transient mount
     state, unattended, at startup.
 
-    Require corroboration: the parent directory must be present, so the
-    absence of the project inside it is something we actually observed. When
-    the parent is missing too, the volume is not there and we know nothing —
-    leave the entry alone. Genuinely abandoned projects are still reclaimed by
-    the retention/stale rule, which runs off ``last_touch`` rather than a
-    filesystem probe.
+    Require corroboration, in two steps.
+
+    First, the parent directory must be present, so the absence of the project
+    inside it is something we actually observed. When the parent is missing
+    too, the volume is not there and we know nothing.
+
+    Second, the present parent must actually carry information. Detaching
+    storage removes the parent outright in some layouts (``/Volumes/Ext/proj``
+    on macOS, ``/media/<user>/<label>/proj``), but in the classic static
+    layout — ``/mnt/volume/proj``, a container bind-mount, an fstab entry —
+    unmounting leaves the mount point behind as an *empty* directory. An empty
+    parent is therefore the signature of a detached volume just as much as of
+    a deleted project, so it corroborates nothing. Prune only when the parent
+    holds something else (we observed a populated directory that does not
+    contain the project) or is itself a live mount point (the volume is
+    demonstrably attached and the project is demonstrably not on it).
+
+    Genuinely abandoned projects are still reclaimed by the retention/stale
+    rule, which runs off ``last_touch`` rather than a filesystem probe.
     """
     if not workdir:
         return False
@@ -1322,12 +1335,32 @@ def _workdir_is_observably_gone(workdir: str) -> bool:
         # to corroborate against.
         if parent == path:
             return False
-        return parent.is_dir()
+        if not parent.is_dir():
+            return False
+        if _dir_has_any_entry(parent):
+            return True
+        # Empty parent: only evidence if that directory is a mount point, i.e.
+        # the volume is attached right now and simply does not hold the
+        # project. An empty plain directory is an unmounted mount point as
+        # readily as an emptied project root.
+        return os.path.ismount(parent)
     except OSError:
         # Probe failed (permission, I/O error) — not evidence of deletion.
         return False
 
 
+def _dir_has_any_entry(directory: Path) -> bool:
+    """True when ``directory`` contains at least one entry.
+
+    Stops after the first entry rather than materializing the listing; a
+    project root can hold a large tree.
+    """
+    with os.scandir(directory) as entries:
+        for _ in entries:
+            return True
+    return False
+
+
 def prune_checkpoints(
     retention_days: int = 7,
     delete_orphans: bool = True,