fix(disk-cleanup): protect every per-profile user tree and trim tests to two invariants

Extends the salvaged #112860 entry (`workspace`) to the whole class: `plans` and `home` are bootstrapped alongside `workspace` by `hermes_cli/profiles.py::_PROFILE_DIRS` and listed as user data in `profile_distribution.py::USER_OWNED_EXCLUDE`, so a `test_*`/`tmp_*` file inside any of them is a user's file, never scratch. Trims the contributor's five tests to two invariants: the end-to-end post_tool_call -> on_session_end path (workspace file survives, a root-level `tmp_scratch.py` control is still removed) and the empty-dir sweep leaving a directory under `workspace/` alone. Documents the protected trees in the plugin README and the built-in-plugins page. Dropped: test_workspace_project_tree_never_tracked, test_quick_keeps_workspace_test_file, test_root_level_test_file_still_auto_deleted (folded into the E2E test as its control). Part of #112859.
2026-09-16 09:56:36 -07:00
parent 3b6b0e24e4
commit 0631710ae4
5 changed files with 41 additions and 80 deletions
--- a/contributors/emails/kevin@meinon.de
+++ b/contributors/emails/kevin@meinon.de
@@ -0,0 +1 @@
+KevinMeinon
--- a/plugins/disk-cleanup/README.md
+++ b/plugins/disk-cleanup/README.md
@@ -46,6 +46,9 @@ Deletion rules (same as the original PR):
 - The state directory `$HERMES_HOME/disk-cleanup/` is itself excluded
 - `$HERMES_HOME/logs/`, `memories/`, `sessions/`, `skills/`, `plugins/`,
  and config files are never tracked
+- User project trees (`workspace/`, `projects/`, `plans/`, `home/`, `patches/`,
+  `skins/`, `themes/`, `contributors/`, `profiles/`, `backups/`) are never
+  tracked or swept, even for files named `test_*`/`tmp_*`
 - Backup/restore is scoped to `tracked.json` — the plugin never touches
  agent logs
 - Atomic writes: `.tmp` → backup → rename
--- a/plugins/disk-cleanup/disk_cleanup.py
+++ b/plugins/disk-cleanup/disk_cleanup.py
@@ -85,7 +85,9 @@ _EMPTY_DIR_PROTECTED_TOP_LEVEL = frozenset({
    "logs", "memories", "sessions", "cron", "cronjobs",
    "cache", "skills", "plugins", "disk-cleanup", "optional-skills",
    "hermes-agent", "backups", "profiles", ".worktrees",
-    "patches", "projects", "skins", "themes", "contributors", "workspace"})
+    "patches", "projects", "skins", "themes", "contributors",
+    # Per-profile user trees bootstrapped by ``profiles.py::_PROFILE_DIRS`` (#112859).
+    "workspace", "plans", "home"})

 _EMPTY_DIR_SWEEP_PRUNE_DIRS = frozenset({
    ".git", "node_modules", "venv", ".venv", "site-packages", "__pycache__"})
@@ -98,11 +100,10 @@ _NEVER_TRACK_TOP_LEVEL = frozenset({
    "auth.json", "hermes-agent",
    # User-authored project trees — never sweep empty directories inside these (#75403).
    # User-authored and project trees — never auto-delete files inside these just because they happen to be
-    # named test_* or tmp_* (#75403, also #32164, #37721). ``workspace`` is the agent/user project tree
-    # bootstrapped in every profile (``profiles.py::_PROFILE_DIRS``) and holds durable artifacts
-    # (``workspace/meetings/`` auth + registry state, project checkouts), so its contents are user data.
+    # named test_* or tmp_* (#75403, also #32164, #37721). ``workspace``, ``plans`` and ``home`` are the
+    # per-profile user trees bootstrapped by ``profiles.py::_PROFILE_DIRS`` (#112859).
    "patches", "projects", "skins", "themes", "contributors",
-    "profiles", "backups", "optional-skills", "workspace"})
+    "profiles", "backups", "optional-skills", "workspace", "plans", "home"})

@functools.lru_cache(maxsize=8)  # keyed by home: a multiplexed process serves several profiles
 def _protected_cron_paths(home: Path) -> frozenset:
--- a/tests/plugins/test_disk_cleanup_plugin.py
+++ b/tests/plugins/test_disk_cleanup_plugin.py
@@ -140,67 +140,45 @@ class TestGuessCategory:
        p.write_text("x")
        assert dg.guess_category(p) is None

-    def test_workspace_project_tree_never_tracked(self, _isolate_env):
-        """``workspace/`` is a user project tree: nothing inside is disposable.

-        Regression for the 2026-09-16 data loss: a pytest file written into
-        ``workspace/<project>/tests/test_parse.py`` was classified "test" on write and
-        unlinked by ``quick()`` at session end, so the next ``git add -A`` in that
-        project committed the deletion.
-        """
-        dg = _load_lib()
-        tests_dir = _isolate_env / "workspace" / "proj" / "tests"
-        tests_dir.mkdir(parents=True)
-        for name in ("test_parse.py", "tmp_probe.py", "thing.test.js"):
-            p = tests_dir / name
-            p.write_text("x")
-            assert dg.guess_category(p) is None, f"{name} must not be classified disposable"
-        artifact = _isolate_env / "workspace" / "proj" / ".artifacts" / "test_role_gate.py"
-        artifact.parent.mkdir(parents=True)
-        artifact.write_text("x")
-        assert dg.guess_category(artifact) is None
+class TestProfileUserTreesNeverCleaned:
+    """``workspace/`` (and the other per-profile user trees) hold project files, so
+    a ``test_*``/``tmp_*`` name inside them is never a disposable scratch file.

-
-class TestWorkspaceProjectTreeGuard:
-    """``workspace/`` must be in both protected top-level sets.
-
-    Without the entry the plugin is a silent data-loss path: ``post_tool_call``
-    tracks ``workspace/<project>/tests/test_*.py`` as "test",
-    ``_is_auto_delete("test", age)`` accepts it at any age, and ``on_session_end``
-    unlinks it. Every profile gets a ``workspace/`` directory
-    (``profiles.py::_PROFILE_DIRS``) and bundled plugins keep durable state there
-    (``google_meet`` writes ``workspace/meetings/``).
+    Regression for the data loss where ``workspace/<project>/tests/test_parse.py`` was
+    classified "test" on write and unlinked by ``quick()`` at session end.
    """

-    def test_quick_keeps_workspace_test_file(self, _isolate_env):
-        """A stale tracked.json "test" entry for a workspace file must be dropped,
-        not honoured (the guess_category re-validation path in quick())."""
+    def test_session_end_hook_leaves_workspace_files_alone(self, _isolate_env):
+        """End-to-end: write_file into a project tree, then session end. A scratch file at
+        the HERMES_HOME root is the control: it is still tracked and removed."""
+        pi = _load_plugin_init()
        dg = _load_lib()
-        p = _isolate_env / "workspace" / "proj" / "tests" / "test_parse.py"
-        p.parent.mkdir(parents=True)
-        p.write_text("def test_x(): pass\n")
-
-        tracked_file = _isolate_env / "disk-cleanup" / "tracked.json"
-        tracked_file.parent.mkdir(parents=True, exist_ok=True)
-        tracked_file.write_text(json.dumps([{
-            "path": str(p.resolve()),
-            "category": "test",
-            "timestamp": "2025-01-01T00:00:00+00:00",  # old, but "test" ignores age
-            "size": 21,
-        }]))
-
-        summary = dg.quick()
-        assert summary["deleted"] == 0, "workspace project file must never be auto-deleted"
-        assert p.exists()
-        assert json.loads(tracked_file.read_text()) == [], "stale entry must be dropped"
+        keep = _isolate_env / "workspace" / "proj" / "tests" / "test_parse.py"
+        keep.parent.mkdir(parents=True)
+        keep.write_text("x")
+        scratch = _isolate_env / "tmp_scratch.py"
+        scratch.write_text("x")
+        assert dg.guess_category(keep) is None
+        assert dg.guess_category(scratch) == "test"
+        for p in (keep, scratch):
+            pi._on_post_tool_call(
+                tool_name="write_file",
+                args={"path": str(p), "content": "x"},
+                result="OK",
+                task_id="t_ws", session_id="s_ws",
+            )
+        pi._on_session_end(session_id="s_ws", completed=True, interrupted=False)
+        assert keep.exists(), "session-end cleanup must not touch workspace project files"
+        assert not scratch.exists(), "root-level scratch files are still cleaned up"

    def test_empty_dir_sweep_skips_workspace(self, _isolate_env):
-        """Empty dirs inside a project tree are meaningful (``data/``, ``.artifacts/``,
-        chrome profile dirs) and must survive the empty-dir sweep."""
+        """Empty dirs inside a project tree are meaningful (``data/``, ``.artifacts/``)
+        and must survive the empty-dir sweep; unprotected empty top levels are still swept."""
        dg = _load_lib()
        keep = _isolate_env / "workspace" / "watch-battery" / "data"
        keep.mkdir(parents=True)
-        sweepable = _isolate_env / "pairing"  # unprotected top level, empty
+        sweepable = _isolate_env / "pairing"
        sweepable.mkdir()

        dg._sweep_empty_dirs(_isolate_env)
@@ -208,28 +186,6 @@ class TestWorkspaceProjectTreeGuard:
        assert keep.exists(), "empty dir inside workspace/ must survive the sweep"
        assert not sweepable.exists(), "unprotected empty dirs are still swept"

-    def test_root_level_test_file_still_auto_deleted(self, _isolate_env):
-        """The guard must not disable cleanup of genuine ephemeral test files."""
-        dg = _load_lib()
-        p = _isolate_env / "test_ephemeral.py"
-        p.write_text("x")
-        assert dg.guess_category(p) == "test"
-
-    def test_session_end_hook_leaves_workspace_files_alone(self, _isolate_env):
-        """End-to-end: write_file into a project tree, then session end."""
-        pi = _load_plugin_init()
-        p = _isolate_env / "workspace" / "proj" / "tests" / "test_parse.py"
-        p.parent.mkdir(parents=True)
-        p.write_text("x")
-        pi._on_post_tool_call(
-            tool_name="write_file",
-            args={"path": str(p), "content": "x"},
-            result="OK",
-            task_id="t_ws", session_id="s_ws",
-        )
-        pi._on_session_end(session_id="s_ws", completed=True, interrupted=False)
-        assert p.exists(), "session-end cleanup must not touch workspace project files"
-

 class TestStaleCronEntryMigration:
    """Regression tests for #37721 — stale cron-output entries in tracked.json."""
@@ -508,4 +464,4 @@ class TestBundledDiscovery:
        mgr = pmod.PluginManager()
        mgr.discover_and_load()
        assert "memory" not in mgr._plugins
-        assert "context_engine" not in mgr._plugins
+        assert "context_engine" not in mgr._plugins
--- a/website/docs/user-guide/features/built-in-plugins.md
+++ b/website/docs/user-guide/features/built-in-plugins.md
@@ -111,7 +111,7 @@ Auto-tracks and removes ephemeral files created during sessions — test scripts
 | `tracked.json.bak` | Atomic-write backup of the above |
 | `cleanup.log` | Append-only audit trail of every track / skip / reject / delete |

-**Safety** — cleanup only ever touches paths under `HERMES_HOME` or `/tmp/hermes-*`. Windows mounts (`/mnt/c/...`) are rejected. Well-known top-level state dirs (`logs/`, `memories/`, `sessions/`, `cron/`, `cache/`, `skills/`, `plugins/`, `disk-cleanup/` itself) are never removed even when empty — a fresh install does not get gutted on first session end.
+**Safety** — cleanup only ever touches paths under `HERMES_HOME` or `/tmp/hermes-*`. Windows mounts (`/mnt/c/...`) are rejected. Well-known top-level state dirs (`logs/`, `memories/`, `sessions/`, `cron/`, `cache/`, `skills/`, `plugins/`, `disk-cleanup/` itself) are never removed even when empty — a fresh install does not get gutted on first session end. User project trees (`workspace/`, `projects/`, `plans/`, `home/`) are never tracked or swept at all: a `test_*.py` or `tmp_*` file inside your project is source code, not scratch.

 **Enabling:** `hermes plugins enable disk-cleanup` (or check the box in `hermes plugins`).