From f0cacd11e830e544df3cd9c710fa2348d314cfe9 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Sat, 3 Oct 2026 19:46:20 +0530 Subject: [PATCH] fix(worktree): retry the plain remove with --force for clean submodule trees Gate finding (Medium, 2c + quality convergent): plain `git worktree remove` always refuses a tree with a checked-out submodule ("working trees containing submodules cannot be moved or removed"). Once --force was dropped, clean merged trees like that got a reap verdict on every startup and every `hermes worktree prune`, and were never removed. Both paths still try the plain remove first. Only when its stderr names submodules and an immediate re-check finds the tree still clean (_worktree_is_dirty False on startup; _dirty_split == (False, []) in the attended path) do they retry with --force, under the same safe env. Adds a submodule row to the existing parametrized startup test. --- hermes_cli/worktree_gc.py | 4 ++++ hermes_cli/worktree_ops.py | 5 +++++ tests/hermes_cli/test_worktree_reclaim_revalidation.py | 10 ++++++++-- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hermes_cli/worktree_gc.py b/hermes_cli/worktree_gc.py index f7346307dd..d91febd343 100644 --- a/hermes_cli/worktree_gc.py +++ b/hermes_cli/worktree_gc.py @@ -330,6 +330,10 @@ def reclaim_worktrees( if record.untracked: remove_args.append("--force") # only after the audited scratch was archived remove_result = _git(remove_args, cwd=repo_root, timeout=30) + # Plain remove always refuses trees with submodules; --force only after a fresh clean check. + if (remove_result.returncode != 0 and not record.untracked + and "submodules" in remove_result.stderr and _dirty_split(record.path) == (False, [])): + remove_result = _git([*remove_args, "--force"], cwd=repo_root, timeout=30) if remove_result.returncode != 0: actions.append(f"failed to remove {record.name}: {remove_result.stderr.strip()}") continue diff --git a/hermes_cli/worktree_ops.py b/hermes_cli/worktree_ops.py index 0a8bc38f32..9c5d3933d9 100644 --- a/hermes_cli/worktree_ops.py +++ b/hermes_cli/worktree_ops.py @@ -967,6 +967,11 @@ def _reap_prune_verdicts(repo_root: str, verdicts: list, stale_work_cutoff: floa continue remove_result = _git(["worktree", "remove", str(entry)], repo_root, timeout=15, stdin=subprocess.DEVNULL, env=remove_env) + # Plain remove always refuses trees with submodules; --force only after a fresh clean check. + if (remove_result.returncode != 0 and "submodules" in remove_result.stderr + and not _worktree_is_dirty(str(entry), repo_root)): + remove_result = _git(["worktree", "remove", "--force", str(entry)], repo_root, + timeout=15, stdin=subprocess.DEVNULL, env=remove_env) if remove_result.returncode != 0: logger.debug("Failed to remove worktree %s: %s", entry.name, remove_result.stderr.strip()) continue diff --git a/tests/hermes_cli/test_worktree_reclaim_revalidation.py b/tests/hermes_cli/test_worktree_reclaim_revalidation.py index 82ef6b2246..bf95449ff3 100644 --- a/tests/hermes_cli/test_worktree_reclaim_revalidation.py +++ b/tests/hermes_cli/test_worktree_reclaim_revalidation.py @@ -52,7 +52,7 @@ def test_attended_reclaim_preserves_work_added_after_audit(repository, kind): assert _git(root, "rev-parse", "--verify", "scratch") -@pytest.mark.parametrize("kind", ["edit", "commit", "include-symlink"]) +@pytest.mark.parametrize("kind", ["edit", "commit", "include-symlink", "submodule"]) def test_startup_reclaim_preserves_work_added_after_classification(repository, kind): from hermes_cli.worktree_ops import _classify_prune_candidates, _reap_prune_verdicts @@ -62,6 +62,12 @@ def test_startup_reclaim_preserves_work_added_after_classification(repository, k (root / ".worktreeinclude").write_text("node_modules\n", encoding="utf-8") (root / ".gitignore").write_text("node_modules/\n.worktrees/\n", encoding="utf-8") os.symlink(root / "node_modules", tree / "node_modules") + if kind == "submodule": # plain `worktree remove` always refuses these; clean + merged still reclaims + _git(root.parent, "init", "-b", "main", "sub") + _git(root.parent / "sub", "-c", "user.name=t", "-c", "user.email=t@x", "commit", "--allow-empty", "-m", "s") + _git(tree, "-c", "protocol.file.allow=always", "submodule", "add", str(root.parent / "sub"), "sub") + _git(tree, "-c", "commit.gpgsign=false", "commit", "-m", "add submodule") + _git(root, "merge", "--ff-only", "scratch") verdicts = _classify_prune_candidates(str(root), [(tree, time.time() - 86400, False)]) assert verdicts[0][3] == "reap" added_work = kind in {"edit", "commit"} @@ -74,5 +80,5 @@ def test_startup_reclaim_preserves_work_added_after_classification(repository, k if added_work: assert (tree / "README.md").read_text(encoding="utf-8") == "new work after audit\n" assert _git(root, "rev-parse", "--verify", "scratch") - else: + elif kind == "include-symlink": assert (root / "node_modules").is_dir()