From 2334fc5e85d1a3fc045b8a014edd675b3c3ac86d Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 18 Apr 2026 21:38:55 +0000 Subject: [PATCH 1/2] Add tests and docs for worktree reattach prune fix (#4394) - tests/recipes/test_worktree_reattach_prune_4394.py: 13 tests (5 YAML static + 8 live git) - docs/recipes/step-04-worktree-reattach-prune.md: full retcon doc - worktrees/README.md: cross-reference to prune doc Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../step-04-worktree-reattach-prune.md | 138 +++++++ .../test_worktree_reattach_prune_4394.py | 345 ++++++++++++++++++ worktrees/README.md | 9 + 3 files changed, 492 insertions(+) create mode 100644 docs/recipes/step-04-worktree-reattach-prune.md create mode 100644 tests/recipes/test_worktree_reattach_prune_4394.py diff --git a/docs/recipes/step-04-worktree-reattach-prune.md b/docs/recipes/step-04-worktree-reattach-prune.md new file mode 100644 index 0000000000..0831f23a52 --- /dev/null +++ b/docs/recipes/step-04-worktree-reattach-prune.md @@ -0,0 +1,138 @@ +# step-04-setup-worktree: Re-Prune After Orphan Directory Cleanup + +`step-04-setup-worktree` creates an isolated git worktree for each workflow +run. When reattaching to an existing branch whose worktree directory was +deleted out-of-band (e.g., by `rm -rf` or a cleanup script), the step now +runs `git worktree prune` immediately after removing the orphaned directory +to clear stale registrations before calling `git worktree add`. + +**Added in:** PR #4394 (merged 2026-04-18) +**Affects:** `amplifier-bundle/recipes/default-workflow.yaml` + +--- + +## Quick Start + +No configuration required. The fix is transparent — `step-04-setup-worktree` +handles stale worktree registrations automatically during any reattach path. + +```bash +# Run the default workflow — step-04 handles stale registrations transparently +amplihack recipe run default-workflow -c task_description="Fix issue #1234" \ + -c repo_path="$(pwd)" +``` + +If a prior worktree directory was removed without `git worktree remove`, the +step logs: + +``` +INFO: Removing orphaned worktree directory '/path/to/worktrees/fix-issue-1234' +``` + +and then prunes the stale `.git/worktrees/` registration before re-creating +the worktree. + +--- + +## Problem + +`step-04-setup-worktree` uses a three-state idempotency guard (see below) to +handle re-runs. Two of those states involve reattaching to a branch when the +worktree directory is missing: + +- **State 2** — Branch exists, worktree missing → `git worktree add` +- **State 3** — New branch, but orphan directory present → `rm -rf` + `git worktree add -b` + +Both states deleted the orphaned directory with `rm -rf` but did **not** +re-run `git worktree prune` before calling `git worktree add`. If git's +internal `.git/worktrees/` registration still existed (because the +initial prune at the top of the step ran before the directory was removed, or +because a user/script deleted the directory out-of-band), the subsequent +`worktree add` would fail with: + +``` +fatal: '' is a missing but already registered worktree; +use 'add -f' to override, or 'prune' or 'remove' to clear +``` + +This made workflow re-runs fragile — any interrupted run that left a stale +worktree directory would block all subsequent attempts. + +--- + +## Solution + +An explicit `git worktree prune >&2` call is inserted immediately after +every `rm -rf "${WORKTREE_PATH}"` in the reattach branches. This clears +the stale `.git/worktrees/` registration so the following `worktree add` +succeeds without `--force`. + +### Prune Points in step-04 + +The step now prunes at four points: + +| # | When | Purpose | +| --- | -------------------------------------------------------- | ----------------------------------------------------- | +| 1 | Top of step (pre-existing) | Clear any stale refs before detection | +| 2 | After wrong-base-branch cleanup (pre-existing, PR #4254) | Clear ref after `worktree remove --force` | +| 3 | **After orphan cleanup in State 2** (new, PR #4394) | Clear ref after `rm -rf` of missing worktree dir | +| 4 | **After orphan cleanup in State 3** (new, PR #4394) | Clear ref after `rm -rf` of orphan dir for new branch | + +--- + +## Three-State Idempotency Guard + +`step-04-setup-worktree` detects three possible states on each run: + +``` +input: BRANCH_NAME + WORKTREE_PATH + │ + ▼ +┌─────────────────────────────────────────────────────────┐ +│ State 1 — Branch exists + worktree exists │ +│ → Reuse silently, output created=false │ +│ (Also checks for wrong base branch — PR #4254) │ +├─────────────────────────────────────────────────────────┤ +│ State 2 — Branch exists + worktree missing │ +│ → rm -rf orphan dir (if present) │ +│ → git worktree prune ← NEW (PR #4394) │ +│ → git worktree add │ +├─────────────────────────────────────────────────────────┤ +│ State 3 — Branch missing (new branch) │ +│ → rm -rf orphan dir (if present) │ +│ → git worktree prune ← NEW (PR #4394) │ +│ → git worktree add -b │ +└─────────────────────────────────────────────────────────┘ +``` + +--- + +## Reproduction & Verification + +To reproduce the original failure: + +```bash +# 1. Create a worktree normally +git worktree add ./worktrees/test-branch -b test-branch main + +# 2. Delete the directory (simulating cleanup script or interrupted run) +rm -rf ./worktrees/test-branch + +# 3. Without the fix, this fails: +git worktree add ./worktrees/test-branch test-branch +# fatal: './worktrees/test-branch' is a missing but already registered worktree + +# 4. With the fix, step-04 runs `git worktree prune` first, so add succeeds +git worktree prune +git worktree add ./worktrees/test-branch test-branch +# ✓ success +``` + +--- + +## Related Documentation + +- [step-03 Idempotency Guards](step-03-idempotency.md) — Issue-creation deduplication +- [Recent Fixes — March 2026](RECENT_FIXES_MARCH_2026.md) — Worktree execution and hook isolation fixes +- [Power Steering Worktree Troubleshooting](../power-steering-worktree-troubleshooting.md) — General worktree debugging +- [Worktree Support](../worktree-support.md) — Feature overview diff --git a/tests/recipes/test_worktree_reattach_prune_4394.py b/tests/recipes/test_worktree_reattach_prune_4394.py new file mode 100644 index 0000000000..53e719a74e --- /dev/null +++ b/tests/recipes/test_worktree_reattach_prune_4394.py @@ -0,0 +1,345 @@ +#!/usr/bin/env python3 +"""Regression test for issue #4394: re-prune after rm -rf of orphan worktree dirs. + +When a worktree directory is deleted out-of-band (rm -rf, cleanup script, +interrupted run), git's internal .git/worktrees/ registration survives. +A subsequent `git worktree add` for the same path fails with: + + fatal: '' is a missing but already registered worktree; + use 'add -f' to override, or 'prune' or 'remove' to clear + +The fix inserts `git worktree prune` immediately after every `rm -rf` +of orphan directories in step-04-setup-worktree, for both State 2 +(branch exists, worktree missing) and State 3 (new branch, orphan dir). + +Tests include: + - Static YAML analysis (prune-after-rm-rf pattern present in both states) + - Live git scenarios (prune enables successful re-add after rm -rf) + - Edge cases (no stale registration, concurrent worktree operations) + +Run: + python3 -m pytest tests/recipes/test_worktree_reattach_prune_4394.py -v +""" + +import os +import re +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[2] +_DEFAULT_RECIPE = _REPO_ROOT / "amplifier-bundle" / "recipes" / "default-workflow.yaml" + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _git(cmd: str, cwd: str) -> subprocess.CompletedProcess: + """Run a git command in a temp repo with deterministic author config.""" + env = { + **os.environ, + "GIT_AUTHOR_NAME": "test", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "test", + "GIT_COMMITTER_EMAIL": "t@t", + } + return subprocess.run( + cmd, + shell=True, + cwd=cwd, + capture_output=True, + text=True, + env=env, + ) + + +def _git_ok(cmd: str, cwd: str) -> str: + """Run a git command, assert success, return stdout.""" + r = _git(cmd, cwd) + if r.returncode != 0: + raise RuntimeError(f"Command failed: {cmd}\nstderr: {r.stderr}") + return r.stdout + + +# =========================================================================== +# Part 1: Static YAML Analysis +# =========================================================================== + + +class TestYAMLPruneAfterRmRf(unittest.TestCase): + """Verify default-workflow.yaml contains prune calls after rm -rf in both states.""" + + @classmethod + def setUpClass(cls): + if not _DEFAULT_RECIPE.exists(): + raise unittest.SkipTest("default-workflow.yaml not found") + cls.yaml_text = _DEFAULT_RECIPE.read_text(encoding="utf-8") + + def test_state2_has_prune_after_rm_rf(self): + """State 2 (branch exists, worktree missing): prune must follow rm -rf.""" + # The State 2 block contains 'Removing orphaned worktree directory' + # followed by rm -rf and then git worktree prune. + # Use a relaxed pattern: rm -rf ... followed by worktree prune within ~5 lines. + state2_pattern = re.compile( + r'rm -rf "\$\{WORKTREE_PATH\}".*?git worktree prune', + re.DOTALL, + ) + matches = list(state2_pattern.finditer(self.yaml_text)) + self.assertGreaterEqual( + len(matches), + 2, + "Expected at least 2 'rm -rf → prune' sequences (State 2 and State 3)", + ) + + def test_state3_has_prune_after_rm_rf(self): + """State 3 (new branch, orphan dir): prune must follow rm -rf.""" + # State 3 marker is "Creating new branch and worktree." (no "from correct base") + # The exact string distinguishes it from the wrong-base recreation path. + state3_marker = 'Creating new branch and worktree."' + idx = self.yaml_text.find(state3_marker) + self.assertNotEqual(idx, -1, "State 3 marker not found in YAML") + + state3_block = self.yaml_text[idx : idx + 500] + self.assertIn("rm -rf", state3_block, "State 3 must rm -rf orphan dir") + self.assertIn( + "git worktree prune", + state3_block, + "State 3 must prune after rm -rf", + ) + + def test_prune_comment_documents_reason(self): + """Each post-rm-rf prune should have an explanatory comment.""" + self.assertIn( + "Re-prune in case the path is still registered", + self.yaml_text, + "Post-rm-rf prune calls should be documented with a comment", + ) + + def test_issue_4394_fix_does_not_use_force_flag(self): + """The fix uses prune, NOT --force flag on worktree add.""" + # Within the rm-rf + prune blocks, worktree add should not use -f/--force + # (except the wrong-base-branch removal which uses `remove --force`) + state2_start = self.yaml_text.find("Branch '${BRANCH_NAME}' exists but worktree is missing") + state3_end = self.yaml_text.find("CREATED=true", state2_start + 100) + if state2_start != -1 and state3_end != -1: + block = self.yaml_text[state2_start:state3_end] + # git worktree add -f or --force should NOT appear + force_pattern = re.compile(r"git worktree add\s+.*(-f|--force)") + match = force_pattern.search(block) + self.assertIsNone( + match, + "Fix should use 'prune' not '--force' for worktree add", + ) + + def test_total_prune_count(self): + """step-04 should have at least 4 prune points (doc says exactly 4).""" + count = self.yaml_text.count("git worktree prune") + self.assertGreaterEqual( + count, + 4, + f"Expected >= 4 prune calls in step-04, found {count}", + ) + + +# =========================================================================== +# Part 2: Live Git Scenarios +# =========================================================================== + + +class TestLivePruneAfterOrphanCleanup(unittest.TestCase): + """Live git tests: verify prune enables worktree add after rm -rf.""" + + def setUp(self): + self.tmpdir = tempfile.mkdtemp(prefix="test_4394_") + self.repo = os.path.join(self.tmpdir, "repo") + os.makedirs(self.repo) + _git_ok("git init -b main", self.repo) + _git_ok("git commit --allow-empty -m 'initial'", self.repo) + + def tearDown(self): + try: + _git("git worktree prune", self.repo) + except Exception: + pass + shutil.rmtree(self.tmpdir, ignore_errors=True) + + # --- State 2: branch exists, worktree removed by rm -rf --- + + def test_state2_rm_rf_then_prune_allows_readd(self): + """State 2: After rm -rf of worktree dir, prune enables worktree add.""" + wt_path = os.path.join(self.tmpdir, "wt-state2") + branch = "feat/state2-prune" + + # Create worktree normally + _git_ok(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + self.assertTrue(os.path.isdir(wt_path)) + + # Simulate out-of-band deletion (cleanup script, interrupted run) + shutil.rmtree(wt_path) + self.assertFalse(os.path.isdir(wt_path)) + + # Without prune, worktree add should fail + r = _git(f"git worktree add {wt_path} {branch}", self.repo) + self.assertNotEqual( + r.returncode, + 0, + "git worktree add should fail when stale registration exists", + ) + self.assertIn("already registered", r.stderr) + + # Apply the fix: prune then add + _git_ok("git worktree prune", self.repo) + _git_ok(f"git worktree add {wt_path} {branch}", self.repo) + self.assertTrue(os.path.isdir(wt_path), "Worktree should exist after prune + add") + + def test_state2_rm_rf_without_prune_fails(self): + """Confirm the bug: rm -rf alone leaves stale registration that blocks add.""" + wt_path = os.path.join(self.tmpdir, "wt-noproune") + branch = "feat/no-prune" + + _git_ok(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + shutil.rmtree(wt_path) + + r = _git(f"git worktree add {wt_path} {branch}", self.repo) + self.assertNotEqual(r.returncode, 0, "Without prune, add must fail") + self.assertIn("already registered", r.stderr) + + # --- State 3: new branch, orphan directory present --- + + def test_state3_orphan_dir_prune_allows_new_branch(self): + """State 3: Stale path registration blocks new branch worktree add.""" + wt_path = os.path.join(self.tmpdir, "wt-state3") + branch = "feat/state3-new" + + # Create a worktree at wt_path, then rm -rf to leave stale registration + _git_ok(f"git worktree add {wt_path} -b stale-reg HEAD", self.repo) + shutil.rmtree(wt_path) + + # Without prune, the stale registration blocks worktree add at same path + r = _git(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + self.assertNotEqual(r.returncode, 0, "Stale registration should block new branch add") + + # Clean up the branch that may have been partially created by the failed attempt + _git("git branch -D " + branch, self.repo) + + # Apply fix: prune then add -b + _git_ok("git worktree prune", self.repo) + _git_ok("git branch -D stale-reg", self.repo) + _git_ok(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + self.assertTrue(os.path.isdir(wt_path)) + + # Verify it's the correct new branch + tip = _git_ok(f"git rev-parse {branch}", self.repo).strip() + head = _git_ok("git rev-parse HEAD", self.repo).strip() + self.assertEqual(tip, head, "New branch should be at HEAD") + + # --- Edge cases --- + + def test_prune_is_idempotent_no_stale_refs(self): + """Prune when there are no stale refs is a no-op (shouldn't error).""" + _git_ok("git worktree prune", self.repo) + # Second prune also fine + _git_ok("git worktree prune", self.repo) + + def test_prune_does_not_affect_healthy_worktrees(self): + """Prune must not remove registrations for worktrees that still exist.""" + wt1 = os.path.join(self.tmpdir, "wt-healthy") + _git_ok(f"git worktree add {wt1} -b feat/healthy HEAD", self.repo) + + # Create and rm -rf a second worktree (stale) + wt2 = os.path.join(self.tmpdir, "wt-stale") + _git_ok(f"git worktree add {wt2} -b feat/stale HEAD", self.repo) + shutil.rmtree(wt2) + + # Prune should clear wt2's registration but keep wt1 + _git_ok("git worktree prune", self.repo) + + # wt1 should still be listed + listing = _git_ok("git worktree list", self.repo) + self.assertIn("wt-healthy", listing, "Healthy worktree must survive prune") + self.assertNotIn("wt-stale", listing, "Stale worktree should be pruned") + + def test_double_rm_rf_then_single_prune(self): + """Multiple orphaned paths are all cleared by a single prune.""" + wt_a = os.path.join(self.tmpdir, "wt-a") + wt_b = os.path.join(self.tmpdir, "wt-b") + _git_ok(f"git worktree add {wt_a} -b feat/a HEAD", self.repo) + _git_ok(f"git worktree add {wt_b} -b feat/b HEAD", self.repo) + + shutil.rmtree(wt_a) + shutil.rmtree(wt_b) + + _git_ok("git worktree prune", self.repo) + + # Both should be re-addable + _git_ok(f"git worktree add {wt_a} feat/a", self.repo) + _git_ok(f"git worktree add {wt_b} feat/b", self.repo) + self.assertTrue(os.path.isdir(wt_a)) + self.assertTrue(os.path.isdir(wt_b)) + + def test_state2_full_sequence_matches_yaml(self): + """Simulate the exact sequence from default-workflow.yaml State 2. + + 1. Check if dir exists → yes → rm -rf + 2. git worktree prune + 3. git worktree add + """ + wt_path = os.path.join(self.tmpdir, "wt-yaml-s2") + branch = "feat/yaml-s2" + + _git_ok(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + shutil.rmtree(wt_path) + + # Recreate orphan dir (simulating partial cleanup or crashed run) + os.makedirs(wt_path) + + # Step sequence from YAML: + if os.path.isdir(wt_path): + shutil.rmtree(wt_path) + _git_ok("git worktree prune", self.repo) + _git_ok(f"git worktree add {wt_path} {branch}", self.repo) + + self.assertTrue(os.path.isdir(wt_path)) + # Verify it's a valid git worktree + self.assertTrue( + os.path.isfile(os.path.join(wt_path, ".git")), + "Worktree should have .git file (not directory)", + ) + + def test_state3_full_sequence_matches_yaml(self): + """Simulate the exact sequence from default-workflow.yaml State 3. + + 1. Check if dir exists → yes → rm -rf + 2. git worktree prune + 3. git worktree add -b + """ + wt_path = os.path.join(self.tmpdir, "wt-yaml-s3") + branch = "feat/yaml-s3" + + # Create orphan dir (not a real worktree, just leftover directory) + os.makedirs(wt_path) + + # Also create a stale registration by adding and rm-rfing a different branch + _git_ok(f"git worktree add {wt_path}-tmp -b tmp-reg HEAD", self.repo) + # Properly remove worktree first so branch can be deleted + _git_ok(f"git worktree remove --force {wt_path}-tmp", self.repo) + _git_ok("git branch -D tmp-reg", self.repo) + + # Step sequence from YAML: + if os.path.isdir(wt_path): + shutil.rmtree(wt_path) + _git_ok("git worktree prune", self.repo) + _git_ok(f"git worktree add {wt_path} -b {branch} HEAD", self.repo) + + self.assertTrue(os.path.isdir(wt_path)) + tip = _git_ok(f"git rev-parse {branch}", self.repo).strip() + head = _git_ok("git rev-parse HEAD", self.repo).strip() + self.assertEqual(tip, head) + + +if __name__ == "__main__": + unittest.main() diff --git a/worktrees/README.md b/worktrees/README.md index 12573b0d0c..8f48d164ac 100644 --- a/worktrees/README.md +++ b/worktrees/README.md @@ -13,6 +13,15 @@ Each worktree is a separate working directory: - ./worktrees/feat-issue-123-description/ - ./worktrees/fix-issue-456-bug-name/ +## Automatic Stale Registration Handling + +The default workflow's `step-04-setup-worktree` automatically runs +`git worktree prune` after removing orphaned worktree directories. This +prevents `fatal: already registered worktree` errors when reattaching to +a branch whose directory was deleted out-of-band. See +[docs/recipes/step-04-worktree-reattach-prune.md](../docs/recipes/step-04-worktree-reattach-prune.md) +for details. + ## Cleanup Remove completed worktrees with: From d4f4fe4ffcc1820e4b45e8fcd238230263a38161 Mon Sep 17 00:00:00 2001 From: Copilot Date: Sat, 18 Apr 2026 21:39:56 +0000 Subject: [PATCH 2/2] wip: checkpoint after implementation (steps 7-8) Automatic checkpoint to preserve work in progress. Tests and implementation saved before refactoring phase. --- .../test_worktree_reattach_prune_4394.py | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/tests/recipes/test_worktree_reattach_prune_4394.py b/tests/recipes/test_worktree_reattach_prune_4394.py index 53e719a74e..e653ade5f2 100644 --- a/tests/recipes/test_worktree_reattach_prune_4394.py +++ b/tests/recipes/test_worktree_reattach_prune_4394.py @@ -32,6 +32,15 @@ _REPO_ROOT = Path(__file__).resolve().parents[2] _DEFAULT_RECIPE = _REPO_ROOT / "amplifier-bundle" / "recipes" / "default-workflow.yaml" +# Cached env dict — avoids copying os.environ on every subprocess call +_GIT_ENV = { + **os.environ, + "GIT_AUTHOR_NAME": "test", + "GIT_AUTHOR_EMAIL": "t@t", + "GIT_COMMITTER_NAME": "test", + "GIT_COMMITTER_EMAIL": "t@t", +} + # --------------------------------------------------------------------------- # Helpers @@ -40,20 +49,13 @@ def _git(cmd: str, cwd: str) -> subprocess.CompletedProcess: """Run a git command in a temp repo with deterministic author config.""" - env = { - **os.environ, - "GIT_AUTHOR_NAME": "test", - "GIT_AUTHOR_EMAIL": "t@t", - "GIT_COMMITTER_NAME": "test", - "GIT_COMMITTER_EMAIL": "t@t", - } return subprocess.run( cmd, shell=True, cwd=cwd, capture_output=True, text=True, - env=env, + env=_GIT_ENV, ) @@ -153,19 +155,20 @@ def test_total_prune_count(self): class TestLivePruneAfterOrphanCleanup(unittest.TestCase): """Live git tests: verify prune enables worktree add after rm -rf.""" - def setUp(self): - self.tmpdir = tempfile.mkdtemp(prefix="test_4394_") - self.repo = os.path.join(self.tmpdir, "repo") - os.makedirs(self.repo) - _git_ok("git init -b main", self.repo) - _git_ok("git commit --allow-empty -m 'initial'", self.repo) + @classmethod + def setUpClass(cls): + cls.tmpdir = tempfile.mkdtemp(prefix="test_4394_") + cls.repo = os.path.join(cls.tmpdir, "repo") + os.makedirs(cls.repo) + _git_ok("git init -b main", cls.repo) + _git_ok("git commit --allow-empty -m 'initial'", cls.repo) + + @classmethod + def tearDownClass(cls): + shutil.rmtree(cls.tmpdir, ignore_errors=True) def tearDown(self): - try: - _git("git worktree prune", self.repo) - except Exception: - pass - shutil.rmtree(self.tmpdir, ignore_errors=True) + _git("git worktree prune", self.repo) # --- State 2: branch exists, worktree removed by rm -rf --- @@ -323,12 +326,6 @@ def test_state3_full_sequence_matches_yaml(self): # Create orphan dir (not a real worktree, just leftover directory) os.makedirs(wt_path) - # Also create a stale registration by adding and rm-rfing a different branch - _git_ok(f"git worktree add {wt_path}-tmp -b tmp-reg HEAD", self.repo) - # Properly remove worktree first so branch can be deleted - _git_ok(f"git worktree remove --force {wt_path}-tmp", self.repo) - _git_ok("git branch -D tmp-reg", self.repo) - # Step sequence from YAML: if os.path.isdir(wt_path): shutil.rmtree(wt_path)