fix: detect and clean orphaned worktree directories invisible to git #223

Merged

forgejo_admin merged 2 commits from 222-bug-cleanup-worktrees-sh-misses-orphaned into main

2026-03-29 03:44:06 +00:00

forgejo_admin commented

2026-03-29 03:24:45 +00:00

Contributor

Summary

cleanup-worktrees.sh only scanned git worktree list --porcelain, completely missing orphaned directories where git lost the worktree registration but the directory persisted on disk. On pal-e-platform alone, 37 of 67 agent worktree directories were orphans invisible to the cleanup script. This PR adds a filesystem scan that detects and removes these orphans.

Changes

hooks/cleanup-worktrees.sh — Added orphaned worktree directory cleanup section (lines 100-193) that:
- Scans {repo}/.claude/worktrees/agent-* on the filesystem for each known repo
- Compares against git worktree list --porcelain to identify orphans
- Three cleanup strategies based on orphan state:
  - With .git file + known branch: Extracts branch from gitdir metadata, verifies merged into default branch, removes if merged OR older than 7 days
  - With .git file + unknown branch: Age-based removal (>7 days)
  - Without .git file (empty shells): Age-based removal (>7 days)
- Cleans up orphaned gitdir metadata alongside the worktree directory
- Calls git worktree prune per-repo after orphan cleanup
- Updated summary output to report orphan cleanup counts
- Improved /tmp clone cleanup to catch orphans without .git directories
hooks/forgejo-helper.sh — Updated remove_worktree_for_branch function (line 276+) to:
- After git-tracked worktree removal, scan filesystem for orphaned agent-* dirs matching the merged branch
- Extract branch from orphan's gitdir/HEAD metadata and compare
- Remove matching orphan dirs + their gitdir metadata
- Call git worktree prune after cleanup

Test Plan

Verified syntax with bash -n on both files
Dry-run tested orphan detection against real pal-e-platform data: correctly identifies 37 orphans (26 would be cleaned as >7 days old, 11 safely skipped as too new)
Verified all 30 git-tracked worktrees are correctly excluded from orphan detection
All orphans confirmed empty (no .git file, 12K each) matching the expected pattern

Review Checklist

Syntax validated with bash -n on both modified files
Orphan detection tested against real pal-e-platform data (37 orphans found, 30 tracked correctly excluded)
Safety: unmerged branches with known branch info are preserved
Safety: directories newer than 7 days are preserved regardless
No changes to hook registration or settings.json

None -- this is a bug fix to existing hook scripts.

Closes #222
Parent issue: forgejo_admin/pal-e-platform#243
Board: board-pal-e-platform, item 637

## Summary `cleanup-worktrees.sh` only scanned `git worktree list --porcelain`, completely missing orphaned directories where git lost the worktree registration but the directory persisted on disk. On pal-e-platform alone, 37 of 67 agent worktree directories were orphans invisible to the cleanup script. This PR adds a filesystem scan that detects and removes these orphans. ## Changes - `hooks/cleanup-worktrees.sh` — Added orphaned worktree directory cleanup section (lines 100-193) that: - Scans `{repo}/.claude/worktrees/agent-*` on the filesystem for each known repo - Compares against `git worktree list --porcelain` to identify orphans - Three cleanup strategies based on orphan state: - **With `.git` file + known branch**: Extracts branch from gitdir metadata, verifies merged into default branch, removes if merged OR older than 7 days - **With `.git` file + unknown branch**: Age-based removal (>7 days) - **Without `.git` file (empty shells)**: Age-based removal (>7 days) - Cleans up orphaned gitdir metadata alongside the worktree directory - Calls `git worktree prune` per-repo after orphan cleanup - Updated summary output to report orphan cleanup counts - Improved `/tmp` clone cleanup to catch orphans without `.git` directories - `hooks/forgejo-helper.sh` — Updated `remove_worktree_for_branch` function (line 276+) to: - After git-tracked worktree removal, scan filesystem for orphaned `agent-*` dirs matching the merged branch - Extract branch from orphan's gitdir/HEAD metadata and compare - Remove matching orphan dirs + their gitdir metadata - Call `git worktree prune` after cleanup ## Test Plan - Verified syntax with `bash -n` on both files - Dry-run tested orphan detection against real pal-e-platform data: correctly identifies 37 orphans (26 would be cleaned as >7 days old, 11 safely skipped as too new) - Verified all 30 git-tracked worktrees are correctly excluded from orphan detection - All orphans confirmed empty (no `.git` file, 12K each) matching the expected pattern ## Review Checklist - [x] Syntax validated with `bash -n` on both modified files - [x] Orphan detection tested against real pal-e-platform data (37 orphans found, 30 tracked correctly excluded) - [x] Safety: unmerged branches with known branch info are preserved - [x] Safety: directories newer than 7 days are preserved regardless - [x] No changes to hook registration or settings.json ## Related Notes None -- this is a bug fix to existing hook scripts. ## Related - Closes #222 - Parent issue: forgejo_admin/pal-e-platform#243 - Board: board-pal-e-platform, item 637

forgejo_admin added 1 commit

2026-03-29 03:24:45 +00:00

fix: detect and clean orphaned worktree directories invisible to git e55e93e078

cleanup-worktrees.sh only scanned `git worktree list --porcelain`, missing
orphaned directories where git lost the worktree registration but the
directory persisted on disk. This caused 37+ empty dirs to accumulate in
pal-e-platform alone.

Adds filesystem scan of {repo}/.claude/worktrees/agent-* that compares
against git-tracked paths to find orphans. Three cleanup strategies:
- Orphans with .git file + known branch: verify merged, then remove
- Orphans with .git file + unknown branch: age-based removal (>7 days)
- Orphans without .git file (empty shells): age-based removal (>7 days)

Also updates remove_worktree_for_branch in forgejo-helper.sh to scan for
orphaned directories matching the merged branch, and improves /tmp clone
cleanup to catch orphans without .git directories.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin added 1 commit

2026-03-29 03:25:58 +00:00

fix: correct grep regex for branch --merged output padding bf30ff7bd5

git branch --merged uses two-character padding ("  " for non-current,
"* " for current). The original regex ^[* ] only matched one character,
missing non-current merged branches. Changed to ^[* ]+ to match one
or more padding characters.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented

2026-03-29 03:43:18 +00:00

Author

Contributor

PR #223 Review

DOMAIN REVIEW

Domain: Bash / Shell scripting / Git worktree management

This PR adds orphaned worktree directory detection to two hook scripts. The core problem is well-understood: git worktree list --porcelain misses directories where git lost the worktree registration but the directory persisted on disk.

Safety analysis -- merge check before deletion:

The orphan cleanup correctly implements a tiered safety model:

Orphans with .git file + known branch: checks git branch --merged before deleting. Unmerged branches younger than 7 days are preserved.
Orphans with .git file + unknown branch: age-based only (7+ days).
Orphans without .git file (empty shells): age-based only (7+ days).

This is sound. Active work on unmerged branches will not be deleted if the worktree is younger than 7 days.

Orphan detection correctness:

The tracked_paths set is built from git worktree list --porcelain output and checked with grep -qFx (fixed string, exact line match). This correctly excludes all git-registered worktrees from orphan treatment. No false positives possible from the tracking check itself.

The forgejo-helper.sh orphan scan uses exact string comparison ([ "$orphan_branch" = "$branch" ]) rather than regex -- this is correct and safe.

BLOCKERS

None.

NITS

Regex injection in branch name matching (cleanup-worktrees.sh, orphan section):
```
grep -qE "^[* ]+${orphan_branch}$"
```
$orphan_branch is interpolated directly into an ERE pattern. Branch names containing regex metacharacters (., +, *, (, )) would cause incorrect matching. For example, branch 123-fix.foo would also match 123-fixXfoo. Consider using grep -qF with the branch name after stripping the 2-character git branch prefix, or escaping the variable. Low severity since branch naming conventions avoid metacharacters, but worth hardening.
DRY: duplicated orphan detection logic across both files. The gitdir-metadata-to-branch extraction pattern (read .git file, parse gitdir path, read HEAD, strip ref: refs/heads/ prefix) is duplicated in cleanup-worktrees.sh (lines ~130-145 in new code) and forgejo-helper.sh (lines ~330-345 in new code). Consider extracting to a shared function in forgejo-helper.sh (e.g., _extract_branch_from_orphan_dir) that both can source. Not a blocker since they run in different hook contexts, but divergence risk over time.
/tmp cleanup broadening -- the relaxed filter now matches /tmp/{repo_name}-* directories without any .git indicator. The glob pattern could match unrelated directories if another tool creates dirs starting with the repo name. The 7-day age gate mitigates severity, but adding a secondary check (e.g., presence of typical repo files like README.md or a known directory structure) would reduce false positive risk.
git branch output format -- the regex ^[* ]+ is slightly loose. git branch uses a fixed 2-character prefix (* for current, for others). A more precise pattern would be ^[* ] + (exactly one char from [*, space] followed by spaces). Functionally equivalent in practice.

SOP COMPLIANCE

Branch named after issue: 222-bug-cleanup-worktrees-sh-misses-orphaned matches issue #222
PR body follows template: Summary, Changes, Test Plan, Related sections all present
Related references parent issue: forgejo_admin/pal-e-platform#243
No secrets committed
Scope is tight: only the two files that need orphan detection are changed
Related section does not reference a plan slug (noted as "None -- this is a bug fix" which is acceptable for a bug fix)

PROCESS OBSERVATIONS

Change failure risk: LOW. The orphan cleanup is additive -- it runs after the existing git-tracked cleanup and cannot affect the existing code path. The tiered safety model (merge check + age gate) provides defense in depth.
Test plan is manual but thorough. Dry-run against real production data (37 orphans, 30 tracked correctly excluded) is the right validation approach for infrastructure shell scripts. bash -n syntax validation is also good practice.
Deployment frequency impact: POSITIVE. Cleaning 37 orphaned directories (~700MB+ each) directly reduces disk pressure that could slow or block agent operations.

VERDICT: APPROVED

## PR #223 Review ### DOMAIN REVIEW **Domain: Bash / Shell scripting / Git worktree management** This PR adds orphaned worktree directory detection to two hook scripts. The core problem is well-understood: `git worktree list --porcelain` misses directories where git lost the worktree registration but the directory persisted on disk. **Safety analysis -- merge check before deletion:** The orphan cleanup correctly implements a tiered safety model: 1. Orphans with `.git` file + known branch: checks `git branch --merged` before deleting. Unmerged branches younger than 7 days are preserved. 2. Orphans with `.git` file + unknown branch: age-based only (7+ days). 3. Orphans without `.git` file (empty shells): age-based only (7+ days). This is sound. Active work on unmerged branches will not be deleted if the worktree is younger than 7 days. **Orphan detection correctness:** The `tracked_paths` set is built from `git worktree list --porcelain` output and checked with `grep -qFx` (fixed string, exact line match). This correctly excludes all git-registered worktrees from orphan treatment. No false positives possible from the tracking check itself. **The `forgejo-helper.sh` orphan scan** uses exact string comparison (`[ "$orphan_branch" = "$branch" ]`) rather than regex -- this is correct and safe. ### BLOCKERS None. ### NITS 1. **Regex injection in branch name matching** (cleanup-worktrees.sh, orphan section): ```bash grep -qE "^[* ]+${orphan_branch}$" ``` `$orphan_branch` is interpolated directly into an ERE pattern. Branch names containing regex metacharacters (`.`, `+`, `*`, `(`, `)`) would cause incorrect matching. For example, branch `123-fix.foo` would also match `123-fixXfoo`. Consider using `grep -qF` with the branch name after stripping the 2-character `git branch` prefix, or escaping the variable. Low severity since branch naming conventions avoid metacharacters, but worth hardening. 2. **DRY: duplicated orphan detection logic** across both files. The gitdir-metadata-to-branch extraction pattern (read `.git` file, parse gitdir path, read HEAD, strip `ref: refs/heads/` prefix) is duplicated in `cleanup-worktrees.sh` (lines ~130-145 in new code) and `forgejo-helper.sh` (lines ~330-345 in new code). Consider extracting to a shared function in `forgejo-helper.sh` (e.g., `_extract_branch_from_orphan_dir`) that both can source. Not a blocker since they run in different hook contexts, but divergence risk over time. 3. **`/tmp` cleanup broadening** -- the relaxed filter now matches `/tmp/{repo_name}-*` directories without any `.git` indicator. The glob pattern could match unrelated directories if another tool creates dirs starting with the repo name. The 7-day age gate mitigates severity, but adding a secondary check (e.g., presence of typical repo files like `README.md` or a known directory structure) would reduce false positive risk. 4. **`git branch` output format** -- the regex `^[* ]+` is slightly loose. `git branch` uses a fixed 2-character prefix (`* ` for current, ` ` for others). A more precise pattern would be `^[* ] +` (exactly one char from `[*, space]` followed by spaces). Functionally equivalent in practice. ### SOP COMPLIANCE - [x] Branch named after issue: `222-bug-cleanup-worktrees-sh-misses-orphaned` matches issue #222 - [x] PR body follows template: Summary, Changes, Test Plan, Related sections all present - [x] Related references parent issue: `forgejo_admin/pal-e-platform#243` - [x] No secrets committed - [x] Scope is tight: only the two files that need orphan detection are changed - [ ] Related section does not reference a plan slug (noted as "None -- this is a bug fix" which is acceptable for a bug fix) ### PROCESS OBSERVATIONS - **Change failure risk: LOW.** The orphan cleanup is additive -- it runs after the existing git-tracked cleanup and cannot affect the existing code path. The tiered safety model (merge check + age gate) provides defense in depth. - **Test plan is manual but thorough.** Dry-run against real production data (37 orphans, 30 tracked correctly excluded) is the right validation approach for infrastructure shell scripts. `bash -n` syntax validation is also good practice. - **Deployment frequency impact: POSITIVE.** Cleaning 37 orphaned directories (~700MB+ each) directly reduces disk pressure that could slow or block agent operations. ### VERDICT: APPROVED