From ecd5d11b3286d8d35baffcffb4b13244b4136815 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 4 May 2026 23:08:13 -0400 Subject: [PATCH] fix(worktree): disable destructive orphaned-worktree removal by default --- get-shit-done/bin/lib/core.cjs | 47 +++++-------------------- tests/prune-orphaned-worktrees.test.cjs | 23 ++++++------ 2 files changed, 19 insertions(+), 51 deletions(-) diff --git a/get-shit-done/bin/lib/core.cjs b/get-shit-done/bin/lib/core.cjs index a51b80f09..3eb338078 100644 --- a/get-shit-done/bin/lib/core.cjs +++ b/get-shit-done/bin/lib/core.cjs @@ -792,19 +792,13 @@ function parseWorktreePorcelain(porcelain) { } /** - * Remove linked git worktrees whose branch has already been merged into the - * current HEAD of the main worktree. Also runs `git worktree prune` to clear - * any stale references left by manually-deleted worktree directories. + * Clear stale worktree metadata references via `git worktree prune`. * - * Safe guards: - * - Never removes the main worktree (first entry in --porcelain output). - * - Never removes the worktree at process.cwd(). - * - Never removes a worktree whose branch has unmerged commits. - * - Skips detached-HEAD worktrees (no branch name). + * Destructive linked-worktree removal is disabled by default for safety. * * @param {string} repoRoot - absolute path to the main (or any) worktree of * the repository; used as `cwd` for git commands. - * @returns {string[]} list of worktree paths that were removed + * @returns {string[]} list of worktree paths that were removed (always empty) */ function pruneOrphanedWorktrees(repoRoot) { const pruned = []; @@ -821,37 +815,14 @@ function pruneOrphanedWorktrees(repoRoot) { return pruned; } - // 2. First entry is the main worktree — never touch it - const mainWorktreePath = worktrees[0].path; - - // 3. Check each non-main worktree - for (let i = 1; i < worktrees.length; i++) { - const { path: wtPath, branch } = worktrees[i]; - - // Never remove the worktree for the current process directory - if (wtPath === cwd || cwd.startsWith(wtPath + path.sep)) continue; - - // Check if the branch is fully merged into HEAD (main) - // git merge-base --is-ancestor HEAD exits 0 when merged - const ancestorCheck = execGit(repoRoot, [ - 'merge-base', '--is-ancestor', branch, 'HEAD', - ]); - - if (ancestorCheck.exitCode !== 0) { - // Not yet merged — leave it alone - continue; - } - - // Remove the worktree and delete the branch - const removeResult = execGit(repoRoot, ['worktree', 'remove', '--force', wtPath]); - if (removeResult.exitCode === 0) { - execGit(repoRoot, ['branch', '-D', branch]); - pruned.push(wtPath); - } - } + // Destructive removal of linked worktrees is intentionally disabled. + // Keep metadata cleanup only (git worktree prune), which clears stale refs + // for manually-deleted directories without removing active sibling worktrees. + void cwd; + void worktrees; } catch { /* never crash the caller */ } - // 4. Always run prune to clear stale references (e.g. manually-deleted dirs) + // Always run prune to clear stale references (e.g. manually-deleted dirs) execGit(repoRoot, ['worktree', 'prune']); return pruned; diff --git a/tests/prune-orphaned-worktrees.test.cjs b/tests/prune-orphaned-worktrees.test.cjs index 6f84edc1b..18756e8f4 100644 --- a/tests/prune-orphaned-worktrees.test.cjs +++ b/tests/prune-orphaned-worktrees.test.cjs @@ -49,8 +49,8 @@ describe('pruneOrphanedWorktrees', () => { cleanup(tmpBase); }); - // Test 1: removes a worktree whose branch is merged into main - test('removes a worktree whose branch is merged into main', () => { + // Test 1: keeps a merged worktree (destructive removal disabled by default) + test('keeps a worktree whose branch is merged into main', () => { const repoDir = path.join(tmpBase, 'repo'); const worktreeDir = path.join(tmpBase, 'wt-merged'); @@ -72,17 +72,17 @@ describe('pruneOrphanedWorktrees', () => { const pruneOrphanedWorktrees = getPruneOrphanedWorktrees(); pruneOrphanedWorktrees(repoDir); - // Assert: worktree directory no longer exists + // Assert: worktree directory still exists assert.ok( - !fs.existsSync(worktreeDir), - 'worktree directory should have been removed but still exists: ' + worktreeDir + fs.existsSync(worktreeDir), + 'merged worktree should not be removed by default: ' + worktreeDir ); - // Assert: git worktree list no longer shows it + // Assert: git worktree list still shows it const listOut = execSync('git worktree list', { cwd: repoDir, encoding: 'utf8' }); assert.ok( - !listOut.includes(worktreeDir), - 'git worktree list still references removed worktree:\n' + listOut + listOut.includes(worktreeDir), + 'git worktree list should still reference merged worktree:\n' + listOut ); }); @@ -132,11 +132,8 @@ describe('pruneOrphanedWorktrees', () => { const pruneOrphanedWorktrees = getPruneOrphanedWorktrees(); const pruned = pruneOrphanedWorktrees(repoDir); - // process.cwd() must not appear in pruned paths - assert.ok( - !pruned.includes(process.cwd()), - 'process.cwd() should never be pruned, but found in: ' + JSON.stringify(pruned) - ); + // No destructive removals are performed by default + assert.deepStrictEqual(pruned, []); // The main worktree (repoDir) itself must still exist assert.ok(