fix(worktree): resolve the main checkout from git, not --git-common-dir's parent
_include_symlink_paths took the main checkout as the parent of
--git-common-dir. For a submodule (common dir .git/modules/<name>) or a
--separate-git-dir repo that is not the checkout, so no .worktreeinclude
symlink matched and every include-only worktree was kept forever as
"dirty" — a regression against BASE, which removed it.
_worktree_is_dirty / _include_symlink_paths now take repo_root, and the
CLI exit cleanup, the startup pruner and kanban teardown pass the one
they already hold. When it is None, fall back to the first
`git worktree list --porcelain` entry resolved through
`rev-parse --show-toplevel` (a submodule lists its git dir there).
Also share the .worktreeinclude parsing between the copy step and the
symlink check (_worktreeinclude_entries), compare with rstrip('/') only,
drop the backslash rewrite, and drop the redundant inner try/except —
_worktree_is_dirty's outer handler already fails safe to dirty.
This commit is contained in:
2
cli.py
2
cli.py
@@ -588,7 +588,7 @@ def _cleanup_worktree(info: Dict[str, str] = None) -> None:
|
||||
return
|
||||
_active_worktree = None
|
||||
|
||||
if _worktree_is_dirty(wt_path, timeout=10):
|
||||
if _worktree_is_dirty(wt_path, repo_root, timeout=10):
|
||||
# Uncommitted work: say so, and don't hint at `remove --force`, which would destroy it.
|
||||
_cprint(f"\n\033[33m{_t('cli.worktree.uncommitted_keeping', path=wt_path)}\033[0m")
|
||||
return
|
||||
|
||||
@@ -255,7 +255,7 @@ def _cleanup_worktree_workspace(
|
||||
repo_root = common.parent
|
||||
if _path_key(wp.resolve(strict=False)) == _path_key(repo_root.resolve(strict=False)):
|
||||
return # never remove the main checkout
|
||||
if _worktree_is_dirty(str(wp)) or _worktree_has_unpushed_commits(str(wp)):
|
||||
if _worktree_is_dirty(str(wp), str(repo_root)) or _worktree_has_unpushed_commits(str(wp)):
|
||||
_kb._log.info(
|
||||
"Preserving worktree for task %s: dirty or unpushed work at %s",
|
||||
task_id, wp,
|
||||
|
||||
@@ -309,20 +309,32 @@ def _ensure_worktrees_gitignored(repo_root: str) -> None:
|
||||
logger.debug("Could not update .gitignore: %s", e)
|
||||
|
||||
|
||||
def _worktreeinclude_entries(repo_root) -> list:
|
||||
"""Non-blank, non-comment entries of *repo_root*/``.worktreeinclude`` (``[]`` when absent).
|
||||
|
||||
utf-8-sig, not the locale default: a cp1251/GBK locale would mojibake or raise on a UTF-8
|
||||
list, and a Notepad BOM would glue to the first entry.
|
||||
"""
|
||||
include_file = Path(repo_root) / ".worktreeinclude"
|
||||
if not include_file.is_file():
|
||||
return []
|
||||
entries = []
|
||||
for line in include_file.read_text(encoding="utf-8-sig", errors="replace").splitlines():
|
||||
entry = line.strip()
|
||||
if entry and not entry.startswith("#"):
|
||||
entries.append(entry)
|
||||
return entries
|
||||
|
||||
|
||||
def _copy_worktree_includes(repo_root: str, wt_path: Path) -> None:
|
||||
"""Copy/symlink the entries listed in ``.worktreeinclude`` (gitignored files the agent needs)."""
|
||||
include_file = Path(repo_root) / ".worktreeinclude"
|
||||
if not include_file.exists():
|
||||
return
|
||||
try:
|
||||
entries = _worktreeinclude_entries(repo_root)
|
||||
if not entries:
|
||||
return
|
||||
repo_root_resolved = Path(repo_root).resolve()
|
||||
wt_path_resolved = wt_path.resolve()
|
||||
# utf-8-sig, not the locale default: a cp1251/GBK locale would mojibake or raise
|
||||
# (swallowed below) on a UTF-8 list; a Notepad BOM would glue to the first entry.
|
||||
for line in include_file.read_text(encoding="utf-8-sig", errors="replace").splitlines():
|
||||
entry = line.strip()
|
||||
if not entry or entry.startswith("#"):
|
||||
continue
|
||||
for entry in entries:
|
||||
src, dst = Path(repo_root) / entry, wt_path / entry
|
||||
# Traversal/symlink-escape guard: both resolved endpoints must stay inside their roots.
|
||||
try:
|
||||
@@ -508,42 +520,49 @@ def _worktree_has_unpushed_commits(worktree_path: str, timeout: int = 10) -> boo
|
||||
return True
|
||||
|
||||
|
||||
def _include_symlink_paths(worktree_path: str, timeout: int = 10) -> set:
|
||||
def _main_checkout_root(worktree_path: str, timeout: int = 10) -> Optional[Path]:
|
||||
"""Main checkout of *worktree_path*'s repo: first ``worktree`` line of ``git worktree list``.
|
||||
|
||||
Not ``--git-common-dir``'s parent: that is wrong for submodules and ``--separate-git-dir``.
|
||||
For a submodule git lists its git dir (``.git/modules/<name>``) there, so resolve that entry
|
||||
through ``rev-parse --show-toplevel`` (honours ``core.worktree``).
|
||||
"""
|
||||
out = _git_out(["worktree", "list", "--porcelain"], worktree_path, timeout=timeout)
|
||||
for line in (out or "").splitlines():
|
||||
if line.startswith("worktree "):
|
||||
first = line[len("worktree "):]
|
||||
top = _git_out(["rev-parse", "--show-toplevel"], first, timeout=timeout)
|
||||
return Path(top or first)
|
||||
return None
|
||||
|
||||
|
||||
def _include_symlink_paths(worktree_path: str, repo_root=None, timeout: int = 10) -> set:
|
||||
"""Relative paths in *worktree_path* that are ``.worktreeinclude`` directory symlinks.
|
||||
|
||||
``_copy_worktree_includes`` symlinks included directories back to the main checkout. A
|
||||
trailing-slash gitignore pattern (``node_modules/``) never matches a symlink, so git reports
|
||||
each one as untracked. Only a symlink at a listed entry that resolves to that same entry in
|
||||
the main repo qualifies — anything else is real user state.
|
||||
``_copy_worktree_includes`` symlinks included directories back to the main checkout
|
||||
(*repo_root*; discovered from git when None). A trailing-slash gitignore pattern
|
||||
(``node_modules/``) never matches a symlink, so git reports each one as untracked. Only a
|
||||
symlink at a listed entry that resolves to that same entry in the main repo qualifies —
|
||||
anything else is real user state.
|
||||
"""
|
||||
common = _git_out(["rev-parse", "--path-format=absolute", "--git-common-dir"], worktree_path,
|
||||
timeout=timeout)
|
||||
if not common:
|
||||
return set()
|
||||
repo_root = Path(common).parent
|
||||
include_file = repo_root / ".worktreeinclude"
|
||||
if not include_file.is_file():
|
||||
root = Path(repo_root) if repo_root else _main_checkout_root(worktree_path, timeout=timeout)
|
||||
if root is None:
|
||||
return set()
|
||||
wt = Path(worktree_path)
|
||||
paths = set()
|
||||
for line in include_file.read_text(encoding="utf-8-sig", errors="replace").splitlines():
|
||||
entry = line.strip().replace("\\", "/").rstrip("/")
|
||||
if not entry or entry.startswith("#"):
|
||||
continue
|
||||
for entry in _worktreeinclude_entries(root):
|
||||
dst = wt / entry
|
||||
try:
|
||||
if dst.is_symlink() and dst.resolve() == (repo_root / entry).resolve():
|
||||
paths.add(entry)
|
||||
except (OSError, ValueError):
|
||||
continue
|
||||
if dst.is_symlink() and dst.resolve() == (root / entry).resolve():
|
||||
paths.add(entry.rstrip("/"))
|
||||
return paths
|
||||
|
||||
|
||||
def _worktree_is_dirty(worktree_path: str, timeout: int = 10) -> bool:
|
||||
def _worktree_is_dirty(worktree_path: str, repo_root=None, timeout: int = 10) -> bool:
|
||||
"""Whether a worktree has staged/unstaged/untracked changes. Fails SAFE toward True.
|
||||
|
||||
Untracked ``.worktreeinclude`` directory symlinks are ignored: they are our own scaffolding,
|
||||
and counting them would keep every worktree of such a repo forever.
|
||||
and counting them would keep every worktree of such a repo forever. Pass the main checkout
|
||||
as *repo_root* when the caller knows it.
|
||||
"""
|
||||
try:
|
||||
result = _git(["status", "--porcelain", "-z"], worktree_path, timeout=timeout)
|
||||
@@ -554,7 +573,7 @@ def _worktree_is_dirty(worktree_path: str, timeout: int = 10) -> bool:
|
||||
return False
|
||||
if any(not e.startswith("?? ") for e in entries):
|
||||
return True
|
||||
include_links = _include_symlink_paths(worktree_path, timeout=timeout)
|
||||
include_links = _include_symlink_paths(worktree_path, repo_root, timeout=timeout)
|
||||
return any(e[3:].rstrip("/") not in include_links for e in entries)
|
||||
except Exception:
|
||||
return True
|
||||
@@ -852,7 +871,7 @@ def _classify_prune_candidates(repo_root: str, candidates: list) -> list:
|
||||
def _classify(item):
|
||||
entry, mtime, force = item
|
||||
# Never delete real work regardless of age: only clean, merged/pushed trees are reaped.
|
||||
if _worktree_is_dirty(str(entry), timeout=5):
|
||||
if _worktree_is_dirty(str(entry), repo_root, timeout=5):
|
||||
return (entry, mtime, force, "dirty", None)
|
||||
keep_branch = False
|
||||
if _worktree_has_unpushed_commits(str(entry), timeout=5):
|
||||
|
||||
@@ -213,7 +213,7 @@ def test_tree_dirtied_between_check_and_removal_preserved(
|
||||
# must still refuse the removal.
|
||||
from hermes_cli import worktree_ops
|
||||
|
||||
monkeypatch.setattr(worktree_ops, "_worktree_is_dirty", lambda _p: False)
|
||||
monkeypatch.setattr(worktree_ops, "_worktree_is_dirty", lambda _p, *_a, **_k: False)
|
||||
kbw._cleanup_worktree_workspace("t_gggg7777", str(wt))
|
||||
assert wt.is_dir()
|
||||
assert (wt / "late-wip.txt").exists()
|
||||
|
||||
Reference in New Issue
Block a user