From b22c36d3bf3e994f5148ad181510abe08f92f570 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 26 Sep 2026 18:43:31 +0530 Subject: [PATCH] test(disk-cleanup): fold late-commit and enclosing-repo cases into one test Keep the stack at two invariant tests: the tracked-file test now also covers a file committed after first classification in the same process and a HERMES_HOME nested in an enclosing repo; the separate stale-cache test is redundant now that there is no cache. Co-authored-by: David Crandall --- tests/plugins/test_disk_cleanup_plugin.py | 68 ++++++----------------- 1 file changed, 16 insertions(+), 52 deletions(-) diff --git a/tests/plugins/test_disk_cleanup_plugin.py b/tests/plugins/test_disk_cleanup_plugin.py index 2966867537..4a49bd0371 100644 --- a/tests/plugins/test_disk_cleanup_plugin.py +++ b/tests/plugins/test_disk_cleanup_plugin.py @@ -277,7 +277,7 @@ class TestGitWorktreeFilesNeverCleaned: assert not scratch.exists() assert result["deleted"] == 1 - def test_tracked_file_in_hermes_home_checkout_is_never_disposable(self, _isolate_env): + def test_tracked_file_in_hermes_home_checkout_is_never_disposable(self, _isolate_env, monkeypatch): """HERMES_HOME itself is a git checkout: a file git TRACKS is Git-owned, so a ``test_*``/``tmp_*`` name must not classify it as disposable. @@ -310,6 +310,21 @@ class TestGitWorktreeFilesNeverCleaned: assert tracked.exists(), "a git-tracked test file must never be auto-deleted" assert result["deleted"] == 0 + # Committed AFTER first classification in the same process: seen immediately. + assert dg.guess_category(scratch) == "test" + subprocess.run(["git", "-C", str(_isolate_env), "add", "test_untracked.py"], check=True) + assert dg.guess_category(scratch) is None + + # HERMES_HOME nested in an enclosing repo (a ~/.git dotfiles repo) that tracks it. + outer = _isolate_env.parent / "outer" + (outer / ".hermes" / "scripts").mkdir(parents=True) + subprocess.run(["git", "init", "-q", str(outer)], check=True) + nested = outer / ".hermes" / "scripts" / "test_x.py" + nested.write_text("x") + subprocess.run(["git", "-C", str(outer), "add", "."], check=True) + monkeypatch.setenv("HERMES_HOME", str(outer / ".hermes")) + assert dg.guess_category(nested) is None + def test_untracked_scratch_beside_tracked_files_is_still_cleaned(self, _isolate_env): """Control for the checkout case: the guard protects what git TRACKS, not the whole home. An untracked ``test_*`` scratch file in the same repo is still disposable, so @@ -329,57 +344,6 @@ class TestGitWorktreeFilesNeverCleaned: assert not scratch.exists() assert result["deleted"] == 1 - def test_file_staged_after_first_classification_is_never_deleted(self, _isolate_env): - """The cached ``git ls-files`` set must not outlive the index it was read from. - - Reviewer's repro on the guard added by this PR: ``guess_category()`` runs on - every post-tool-call, so a long-lived process (the gateway) caches the ls-files - set while ``test_scratch.py`` is still untracked. The same process then stages - and commits that file (e.g. an auto-snapshot) and a later ``quick()`` - re-validates the stored "test" entry through ``guess_category()`` — the stale - cache still reports the file as untracked, so ``_delete_item()`` unlinks a file - git now owns (``git status`` shows ``D test_scratch.py``). - - No ``cache_clear()`` anywhere in this test: the guard has to notice the index - change on its own, exactly as it must inside the live gateway process. - """ - import subprocess - - dg = _load_lib() - subprocess.run(["git", "init", "-q", str(_isolate_env)], check=True) - scratch = _isolate_env / "test_scratch.py" - scratch.write_text("x") - control = _isolate_env / "tmp_control.log" - control.write_text("x") - - # 1. First classification happens while the file is still untracked — this is - # what fills the per-process index cache. - assert dg._inside_git_worktree(scratch) is False - assert dg.guess_category(scratch) == "test" - assert dg.guess_category(control) == "test" - dg.save_tracked([{"path": str(scratch), "category": "test", - "timestamp": datetime.now(timezone.utc).isoformat(), "size": 1}, - {"path": str(control), "category": "test", - "timestamp": datetime.now(timezone.utc).isoformat(), "size": 1}]) - - # 2. Git takes ownership of the file without the plugin being told. - subprocess.run(["git", "-C", str(_isolate_env), "add", "test_scratch.py"], check=True) - subprocess.run(["git", "-C", str(_isolate_env), - "-c", "user.email=test@example.com", "-c", "user.name=Test", - "commit", "-q", "-m", "own it"], check=True) - - # The guard must read the new index, not the one it cached in step 1. - assert dg._inside_git_worktree(scratch) is True - assert dg.guess_category(scratch) is None - - # 3. quick() re-validates: the file git now owns survives, and the never-tracked - # control in the same repo is still cleaned (not a blanket stop on cleanup). - result = dg.quick() - assert scratch.exists(), "a file git now owns must never be auto-deleted" - assert not control.exists(), "untracked scratch beside it is still disposable" - assert result["deleted"] == 1 - assert dg.load_tracked() == [], "both entries are resolved, neither is kept" - class TestStaleCronEntryMigration: """Regression tests for #37721 — stale cron-output entries in tracked.json."""