mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix: worktree removal treats ignored files as disposable
Ignored files in a worktree (.env copies, caches, build and test output, launcher scripts) are never meant to be preserved: removing a worktree deletes them, the same as every other worktree tool. The keep-ignored-work rule kept 461 of 574 real trees forever, so it is removed with its regenerable-dir list and main-checkout byte comparison. Kept: keep-large-untracked, keep-unarchivable (embedded repos, including ones inside ignored dirs, whose commits may exist nowhere else; an over-budget ignored walk now reports keep-unarchivable), keep-worktree-refs, the submodule rule, the showUntrackedFiles=no and skip-worktree fixes, and the staged-edit recheck.
This commit is contained in:
1 parent
44f60d70bc
commit
06671b7075
9 files changed
+45
-105
No files matched your search
@@ -578,7 +578,7 @@ def _cleanup_worktree(info: dict[str, str] | None = None) -> None:
|
||||
return
|
||||
|
||||
from hermes_cli.worktree_ops import removal_blocked
|
||||
blocked = removal_blocked(wt_path) # an embedded repo, private ref or ignored file dies with it
|
||||
blocked = removal_blocked(wt_path) # an embedded repo or private ref dies with it
|
||||
if blocked:
|
||||
_cprint(f"\n\033[33m{_t('cli.worktree.kept_reason', path=wt_path, reason=blocked[1])}\033[0m")
|
||||
return
|
||||
|
||||
@@ -13,8 +13,8 @@ source (``.gitattributes``, ``$GIT_COMMON_DIR/info/attributes``, ``core.attribut
|
||||
clean/smudge/eol conversion can change them; a restore writes back exactly the archived bytes. Untracked files over a size cap are skipped and listed
|
||||
(so ``.git`` never balloons with build artefacts the ignore rules missed), and a tree with a
|
||||
skipped file is never removed; an untracked embedded repository cannot be captured at all and
|
||||
fails the snapshot, which keeps the tree. Ignored files are not captured: removal is allowed only
|
||||
when every one is reproducible (``worktree_retention``).
|
||||
fails the snapshot, which keeps the tree. Ignored files are not captured and are deleted with the
|
||||
tree; an embedded repository inside an ignored directory keeps it (``worktree_retention``).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -218,7 +218,7 @@ def _digest_index(wt: Path, env) -> str:
|
||||
def archive_preflight(wt_path, max_untracked_mb: float = 50) -> Optional[tuple[str, str]]:
|
||||
"""``(keep-action, reason)`` when archiving and removing *wt_path* would lose something
|
||||
(``worktree_retention.removal_blocker``: submodule, embedded repository, per-worktree ref,
|
||||
an untracked file over the size cap, a non-reproducible ignored file), else None. Read-only."""
|
||||
an untracked file over the size cap), else None. Read-only."""
|
||||
from hermes_cli.worktree_retention import KEEP_UNARCHIVABLE, removal_blocker
|
||||
try:
|
||||
wt = Path(wt_path)
|
||||
|
||||
@@ -1019,8 +1019,7 @@ _PRESERVE_REASONS = {"dirty": "uncommitted changes", "unpushed": "unpushed commi
|
||||
def removal_blocked(path: str) -> Optional[tuple[str, str]]:
|
||||
"""``(keep-action, reason)`` when deleting the clean tree at *path* would lose something no
|
||||
branch holds (``worktree_retention.removal_blocker``: a checked-out submodule, an embedded
|
||||
repository, a per-worktree ref, an ignored file that is not reproducible); unverifiable
|
||||
counts as blocked."""
|
||||
repository, a per-worktree ref); unverifiable counts as blocked."""
|
||||
from hermes_cli.worktree_retention import removal_blocker
|
||||
return removal_blocker(path)
|
||||
|
||||
|
||||
@@ -9,10 +9,11 @@ that is not reproducible is in git or in the archive. Not reproducible:
|
||||
worktree): its private commits live only on disk (``keep-unarchivable``);
|
||||
* a commit held only by a per-worktree ref (``refs/worktree/*``, ``refs/bisect/*``,
|
||||
``refs/rewritten/*``), which dies with the tree's admin dir (``keep-worktree-refs``);
|
||||
* an untracked file the snapshot skipped for size (``keep-large-untracked``);
|
||||
* an ignored file, unless it lies under a well-known regenerable directory
|
||||
(:data:`REGENERABLE_DIRS`) or is a byte-identical copy of the same path in the main checkout
|
||||
(what ``.worktreeinclude`` copies, e.g. ``.env``) (``keep-ignored-work``).
|
||||
* an untracked file the snapshot skipped for size (``keep-large-untracked``).
|
||||
|
||||
Ignored files (``.env`` copies, caches, build and test output) are disposable: they are not
|
||||
archived and are deleted with the tree. Only an embedded repository inside an ignored directory
|
||||
keeps it, because its commits may exist nowhere else.
|
||||
|
||||
:func:`hidden_index_edits` finds skip-worktree / assume-unchanged files whose bytes differ from
|
||||
the index: ``git status`` never reports them, so dirty checks must add them.
|
||||
@@ -20,38 +21,27 @@ the index: ``git status`` never reports them, so dirty checks must add them.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import filecmp
|
||||
import os
|
||||
import subprocess
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import Optional
|
||||
|
||||
# Ignored content under these directory names is regenerated by a build/install/test run.
|
||||
REGENERABLE_DIRS = frozenset({
|
||||
"node_modules", ".venv", "venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".ruff_cache",
|
||||
".tox", "dist", "build", "target", ".next", ".turbo", ".cache", "coverage", ".gradle",
|
||||
})
|
||||
# Dependency stores and caches: never descended, not even to look for an embedded repository.
|
||||
_OPAQUE_DIRS = frozenset({
|
||||
"node_modules", ".venv", "venv", "__pycache__", ".pytest_cache", ".mypy_cache", ".ruff_cache",
|
||||
".tox", ".next", ".turbo", ".cache", ".gradle",
|
||||
})
|
||||
_PER_WORKTREE_REFS = ("refs/worktree/", "refs/bisect/", "refs/rewritten/")
|
||||
_MAX_ENTRIES = 200_000 # per tree; past this the ignored content is unverifiable -> keep
|
||||
_MAX_ENTRIES = 200_000 # per tree; past this an embedded repository cannot be ruled out -> keep
|
||||
_NAMED = 5 # offenders named in a reason
|
||||
|
||||
KEEP_UNARCHIVABLE = "keep-unarchivable"
|
||||
KEEP_WORKTREE_REFS = "keep-worktree-refs"
|
||||
KEEP_LARGE_UNTRACKED = "keep-large-untracked"
|
||||
KEEP_IGNORED_WORK = "keep-ignored-work"
|
||||
KEEP_SUBMODULE_WORK = "keep-submodule-work"
|
||||
|
||||
|
||||
def _regenerable(rel: str) -> bool:
|
||||
return any(part in REGENERABLE_DIRS or part.endswith(".egg-info") for part in rel.split("/"))
|
||||
|
||||
|
||||
def _env(wt: Path) -> dict:
|
||||
from hermes_cli.worktree_archive import _env as archive_env
|
||||
return {**archive_env(wt), "GIT_OPTIONAL_LOCKS": "0"}
|
||||
@@ -62,38 +52,15 @@ def _git(args, wt: Path, env, timeout: float = 120) -> str:
|
||||
return _ok(args, wt, env, timeout=timeout)
|
||||
|
||||
|
||||
def main_checkout(wt: Path, env) -> Optional[Path]:
|
||||
"""The repository's main working tree (``<common-dir>/..``), or None for a bare repo."""
|
||||
common = Path(_git(["rev-parse", "--path-format=absolute", "--git-common-dir"], wt, env).strip())
|
||||
return common.parent if common.name == ".git" and common.parent.is_dir() else None
|
||||
|
||||
|
||||
def _same_as_main(path: Path, rel: str, main: Optional[Path]) -> bool:
|
||||
"""*path* is byte-identical to (or a symlink equal to / pointing at) *rel* in *main*."""
|
||||
if main is None:
|
||||
return False
|
||||
twin = main / rel
|
||||
try:
|
||||
if path.is_symlink():
|
||||
if twin.is_symlink() and os.readlink(path) == os.readlink(twin):
|
||||
return True
|
||||
return path.resolve() == twin.resolve() # a .worktreeinclude link back to the main checkout
|
||||
return twin.is_file() and not twin.is_symlink() and filecmp.cmp(path, twin, shallow=False)
|
||||
except OSError:
|
||||
return False
|
||||
|
||||
|
||||
@dataclass
|
||||
class _Scan:
|
||||
embedded: list = field(default_factory=list)
|
||||
ignored: list = field(default_factory=list)
|
||||
entries: int = 0
|
||||
truncated: bool = False
|
||||
|
||||
|
||||
def _walk(scan: _Scan, wt: Path, rel_dir: str, main: Optional[Path], *, files: bool) -> None:
|
||||
"""Below ignored *rel_dir*: record nested ``.git`` entries and (when *files*) every file
|
||||
that is neither regenerable nor identical to the main checkout. Symlinks are not followed."""
|
||||
def _walk(scan: _Scan, wt: Path, rel_dir: str) -> None:
|
||||
"""Below ignored *rel_dir*: record nested ``.git`` entries. Symlinks are not followed."""
|
||||
stack = [rel_dir]
|
||||
while stack and not scan.truncated:
|
||||
rel = stack.pop()
|
||||
@@ -110,11 +77,8 @@ def _walk(scan: _Scan, wt: Path, rel_dir: str, main: Optional[Path], *, files: b
|
||||
if entry.name == ".git":
|
||||
scan.embedded.append(rel)
|
||||
continue
|
||||
if entry.is_dir(follow_symlinks=False):
|
||||
if entry.name not in _OPAQUE_DIRS:
|
||||
stack.append(child)
|
||||
elif files and not _regenerable(child) and not _same_as_main(Path(entry.path), child, main):
|
||||
scan.ignored.append(child)
|
||||
if entry.is_dir(follow_symlinks=False) and entry.name not in _OPAQUE_DIRS:
|
||||
stack.append(child)
|
||||
|
||||
|
||||
def _ignored_entries(wt: Path, env) -> list[str]:
|
||||
@@ -133,7 +97,7 @@ def _ignored_entries(wt: Path, env) -> list[str]:
|
||||
|
||||
|
||||
def scan_tree(wt_path) -> _Scan:
|
||||
"""Embedded repositories and non-reproducible ignored files of *wt_path*. Read-only.
|
||||
"""Embedded repositories of *wt_path*, ignored or not. Read-only.
|
||||
|
||||
Untracked non-ignored repositories are reported by ``ls-files`` itself (``dir/``); ignored
|
||||
ones hide inside collapsed ignored directories, so those are walked (bounded; dependency
|
||||
@@ -145,19 +109,15 @@ def scan_tree(wt_path) -> _Scan:
|
||||
# Without --directory git descends untracked dirs and lists each embedded repository as "dir/".
|
||||
untracked = _git(["ls-files", "--others", "--exclude-standard", "-z"], wt, env)
|
||||
scan.embedded += [p.rstrip("/") for p in untracked.split("\0") if p.endswith("/")]
|
||||
main = main_checkout(wt, env)
|
||||
for rel in _ignored_entries(wt, env):
|
||||
rel_clean = rel.rstrip("/")
|
||||
path = wt / rel_clean
|
||||
if rel.endswith("/") and not path.is_symlink():
|
||||
if rel_clean.rsplit("/", 1)[-1] in _OPAQUE_DIRS:
|
||||
continue
|
||||
if (path / ".git").exists():
|
||||
scan.embedded.append(rel_clean)
|
||||
continue
|
||||
_walk(scan, wt, rel_clean, main, files=not _regenerable(rel_clean))
|
||||
elif not _regenerable(rel_clean) and not _same_as_main(path, rel_clean, main):
|
||||
scan.ignored.append(rel_clean)
|
||||
if not rel.endswith("/") or path.is_symlink() or rel_clean.rsplit("/", 1)[-1] in _OPAQUE_DIRS:
|
||||
continue
|
||||
if (path / ".git").exists():
|
||||
scan.embedded.append(rel_clean)
|
||||
continue
|
||||
_walk(scan, wt, rel_clean)
|
||||
if scan.truncated:
|
||||
break
|
||||
return scan
|
||||
@@ -245,15 +205,13 @@ def removal_blocker(wt_path, *, skipped: Optional[list] = None, gitlinks: bool =
|
||||
scan = scan_tree(wt_path)
|
||||
if scan.embedded:
|
||||
return KEEP_UNARCHIVABLE, f"embedded repository {scan.embedded[0]} cannot be archived"
|
||||
if scan.truncated:
|
||||
return KEEP_UNARCHIVABLE, f"over {_MAX_ENTRIES} ignored entries; cannot rule out an embedded repository"
|
||||
refs = private_worktree_refs(wt_path)
|
||||
if refs:
|
||||
return KEEP_WORKTREE_REFS, f"per-worktree ref {_named(refs)} holds commits no branch has"
|
||||
if skipped:
|
||||
return KEEP_LARGE_UNTRACKED, f"untracked file(s) over the archive size cap: {_named([s['path'] for s in skipped])}"
|
||||
if scan.truncated:
|
||||
return KEEP_IGNORED_WORK, f"over {_MAX_ENTRIES} ignored entries; cannot verify they are reproducible"
|
||||
if scan.ignored:
|
||||
return KEEP_IGNORED_WORK, f"ignored file(s) not reproducible: {_named(scan.ignored)}"
|
||||
except (ArchiveError, OSError, subprocess.SubprocessError) as exc:
|
||||
return KEEP_UNARCHIVABLE, f"could not verify the tree is safe to remove: {exc}"
|
||||
return None
|
||||
@@ -8,7 +8,7 @@ the tree. Never touched, at any age or budget: Kanban ``t_<hex>`` trees, trees w
|
||||
lock owner still runs, trees any live process sits in (cwd or open file), and trees with file or
|
||||
ref activity younger than their tier, and trees holding anything an archive cannot carry
|
||||
(``worktree_retention.removal_blocker``: a submodule, an embedded repository, a per-worktree ref,
|
||||
an untracked file over the size cap, an ignored file that is not reproducible). With
|
||||
an untracked file over the size cap). With
|
||||
``archive_dirty: false`` and no size budget the classic pruner runs unchanged.
|
||||
|
||||
``plan_sweep`` is read-only (``hermes worktree prune --dry-run`` prints it); ``execute_plan``
|
||||
@@ -357,8 +357,7 @@ def archive_then_remove(repo_root: str, path: str, *, reason: str, max_untracked
|
||||
tree is checked again: a process (other than *exclude_pids*) now working in it, any edit
|
||||
made while the snapshot ran, staged entries that differ from the captured index, or anything
|
||||
the archive does not hold (``worktree_retention.removal_blocker``: a file the snapshot skipped
|
||||
for size, an embedded repository, a per-worktree ref, a non-reproducible ignored file, a
|
||||
submodule) keep the tree (``removed`` False, *why_kept* says why; the archive stays). The branch
|
||||
for size, an embedded repository, a per-worktree ref, a submodule) keep the tree (``removed`` False, *why_kept* says why; the archive stays). The branch
|
||||
survives when it is protected or published on a remote (an open PR may still need it);
|
||||
otherwise the archive ref anchors its commits and the local branch is deleted.
|
||||
"""
|
||||
|
||||
@@ -293,13 +293,6 @@ def _case_ignored_repo(repo, wt):
|
||||
return {}, (wt / "build" / "tool", _private_commit(_mkdirs(wt / "build" / "tool")))
|
||||
|
||||
|
||||
def _case_ignored_files(repo, wt):
|
||||
(wt / "notes.md").write_text("dirty\n", encoding="utf-8")
|
||||
(wt / "prod.secret").write_text("API KEY WORK\n", encoding="utf-8")
|
||||
(wt / ".env").write_text("LOCAL ONLY=1\n", encoding="utf-8") # differs from the main checkout
|
||||
return {"prod.secret": b"API KEY WORK\n", ".env": b"LOCAL ONLY=1\n"}, None
|
||||
|
||||
|
||||
def _case_untracked_hidden(repo, wt):
|
||||
_git(["config", "status.showUntrackedFiles", "no"], repo)
|
||||
(wt / "new_work.py").write_text("UNTRACKED WORK\n", encoding="utf-8")
|
||||
@@ -329,27 +322,30 @@ def _mkdirs(path):
|
||||
|
||||
|
||||
@pytest.mark.parametrize("mode", ["archive", "classic"])
|
||||
@pytest.mark.parametrize("setup", [_case_large, _case_ignored_repo, _case_ignored_files, _case_untracked_hidden,
|
||||
@pytest.mark.parametrize("setup", [_case_large, _case_ignored_repo, _case_untracked_hidden,
|
||||
_case_skip_worktree, _case_worktree_ref], ids=lambda f: f.__name__[6:])
|
||||
def test_the_pruner_removes_a_tree_only_when_everything_irreproducible_is_archived(repo, mode, setup):
|
||||
"""R2: through the real pruner, every byte and commit that git cannot regenerate survives,
|
||||
in the tree or in its archive: an untracked file over the size cap, a gitignored embedded
|
||||
repository, ignored files that are not reproducible, untracked files hidden by
|
||||
``status.showUntrackedFiles=no``, skip-worktree / assume-unchanged edits, and commits held
|
||||
only by per-worktree refs. Regenerable ignored content (``node_modules``, ``__pycache__``) and
|
||||
``.worktreeinclude`` copies identical to the main checkout never hold a tree back."""
|
||||
repository, untracked files hidden by ``status.showUntrackedFiles=no``, skip-worktree /
|
||||
assume-unchanged edits, and commits held only by per-worktree refs. Ignored files are
|
||||
disposable: a dirty tree holding only those is archived and removed."""
|
||||
if mode == "classic":
|
||||
_set_worktrees_config("worktrees:\n archive_dirty: false\n")
|
||||
else:
|
||||
_set_worktrees_config("worktrees:\n archive_max_file_mb: 1\n")
|
||||
(repo / ".git" / "info").mkdir(exist_ok=True)
|
||||
(repo / ".git" / "info" / "exclude").write_text("build/\n*.secret\n.env\n__pycache__/\n", encoding="utf-8")
|
||||
(repo / ".git" / "info" / "exclude").write_text("build/\n*.secret\n.env\n__pycache__/\ntest_durations.json\n",
|
||||
encoding="utf-8")
|
||||
(repo / ".env").write_text("SHARED=1\n", encoding="utf-8")
|
||||
wt, regen = _tree(repo, "lane-case"), _tree(repo, "lane-regen", dirty=True)
|
||||
for tree in (wt, regen):
|
||||
shutil.copy2(repo / ".env", tree / ".env") # what .worktreeinclude copies
|
||||
(_mkdirs(tree / "node_modules" / "dep") / "index.js").write_text("x\n", encoding="utf-8")
|
||||
(_mkdirs(tree / "pkg" / "__pycache__") / "m.pyc").write_bytes(b"\0")
|
||||
(regen / ".env").write_text("LOCAL EDIT=1\n", encoding="utf-8") # ignored, differs from main
|
||||
(regen / "test_durations.json").write_text("{}\n", encoding="utf-8")
|
||||
(regen / "notes.secret").write_text("scratch\n", encoding="utf-8")
|
||||
precious, commit = setup(repo, wt)
|
||||
sizes = {rel: (wt / rel).read_bytes() for rel, data in precious.items() if data is None}
|
||||
precious = {**precious, **sizes}
|
||||
@@ -371,7 +367,8 @@ def test_the_pruner_removes_a_tree_only_when_everything_irreproducible_is_archiv
|
||||
if where == repo:
|
||||
assert wt.exists()
|
||||
if mode == "archive":
|
||||
assert not regen.exists(), "regenerable / main-identical ignored files must not hold a tree back"
|
||||
assert not regen.exists(), "ignored files are disposable and must not hold a tree back"
|
||||
assert any(ref.endswith("/lane-regen") for ref in archives), "the dirty tree is archived first"
|
||||
|
||||
|
||||
def test_prune_older_than_bounds_what_the_sweep_removes(repo, capsys):
|
||||
|
||||
@@ -194,7 +194,7 @@ def finalize_subagent_worktree(info: dict[str, str], *, prune: bool = True) -> d
|
||||
cwd = info.get("repo_root", "") or path
|
||||
try:
|
||||
from hermes_cli.worktree_ops import release_lsp_clients, removal_blocked
|
||||
blocked = removal_blocked(path) # an embedded repo / ignored file / ref dies with the tree
|
||||
blocked = removal_blocked(path) # an embedded repo / private ref dies with the tree
|
||||
if blocked:
|
||||
payload["note"] = f"Worktree kept at {path}: {blocked[1]}."
|
||||
return payload
|
||||
|
||||
@@ -124,10 +124,10 @@ Safety guarantees (all modes, any age):
|
||||
- **Nothing irreproducible is deleted unarchived**: a tree is kept when it holds
|
||||
an embedded repository (even a gitignored one), a commit only a per-worktree
|
||||
ref (`refs/worktree/*`, `refs/bisect/*`) reaches, an untracked file over the
|
||||
archive size cap, or an ignored file that is neither under a build/dependency
|
||||
directory (`node_modules`, `.venv`, `__pycache__`, `dist`, `build`, …) nor a
|
||||
byte-identical copy of the main checkout's file (`.worktreeinclude`'s `.env`).
|
||||
See [ignored files](./git-worktrees.md#stale-worktree-sweep-and-restore).
|
||||
archive size cap. Ignored files (`.env` copies, caches, build output) are not
|
||||
archived and are deleted with the tree; a nested git repository inside an
|
||||
ignored folder keeps it. See
|
||||
[ignored files](./git-worktrees.md#stale-worktree-sweep-and-restore).
|
||||
- Branch deletion is content-gated, not name-gated: any local branch whose
|
||||
commits are all on upstream is safe to delete; branches with unique work,
|
||||
checked-out branches, and `main`/`master`/`develop` are always kept.
|
||||
|
||||
@@ -217,23 +217,10 @@ repositories' `.worktrees/`:
|
||||
| An embedded repository anywhere inside, gitignored or not (e.g. `build/tool/.git`) | kept (`keep-unarchivable`) |
|
||||
| A commit held only by a per-worktree ref (`refs/worktree/*`, `refs/bisect/*`, `refs/rewritten/*`) | kept (`keep-worktree-refs`: those refs are deleted with the tree) |
|
||||
| An untracked file larger than `worktrees.archive_max_file_mb` | kept (`keep-large-untracked`) |
|
||||
| An ignored file that is not reproducible (see below) | kept (`keep-ignored-work`; `--dry-run` names the files) |
|
||||
|
||||
**Ignored files.** An archive does not capture files your ignore rules
|
||||
(`.gitignore`, `.git/info/exclude`, `core.excludesFile`) exclude, so a tree is
|
||||
removed only when every ignored file in it can be regenerated:
|
||||
|
||||
- it lies under a build or dependency directory: `node_modules`, `.venv`,
|
||||
`venv`, `__pycache__`, `.pytest_cache`, `.mypy_cache`, `.ruff_cache`, `.tox`,
|
||||
`dist`, `build`, `target`, `.next`, `.turbo`, `.cache`, `coverage`, `.gradle`,
|
||||
`*.egg-info`; or
|
||||
- it is a byte-identical copy of the same path in the main checkout (what
|
||||
`.worktreeinclude` copies, such as `.env`).
|
||||
|
||||
Any other ignored file (a local `.env` edit, `*.secret`, notes excluded in
|
||||
`info/exclude`, test output) keeps the tree, on every path: the sweep, exit
|
||||
**Archive** and subagent cleanup. Commit, move or delete the file and the next
|
||||
sweep reclaims the tree.
|
||||
**Ignored files.** Ignored files (`.env` copies, caches, build and test output)
|
||||
are not archived and are deleted with the tree. A nested git repository inside
|
||||
an ignored folder is the one exception: it keeps the tree.
|
||||
|
||||
The same checks run on clean merged trees, and every dirty check counts files
|
||||
that `status.showUntrackedFiles=no` would hide and skip-worktree /
|
||||
|
||||
Reference in new issue
Block a user