Skip to content

feat: cross-repo worktree cleanup with squash-merge detection #2616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Feb 28, 2026
Merged

feat: cross-repo worktree cleanup with squash-merge detection #2616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 49 additions & 12 deletions .agents/scripts/pulse-wrapper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -534,29 +534,66 @@ ${state_content}
}

#######################################
# Clean up worktrees for merged/closed PRs
# Clean up worktrees for merged/closed PRs across ALL managed repos
#
# Runs worktree-helper.sh clean --auto to remove worktrees whose
# branches have been merged or deleted on the remote. This prevents
# stale worktrees from accumulating on disk after PR merges.
# Iterates repos.json and runs worktree-helper.sh clean --auto --force-merged
# in each repo directory. This prevents stale worktrees from accumulating
# on disk after PR merges — including squash merges that git branch --merged
# cannot detect.
#
# Safety: skips worktrees with uncommitted changes or owned by
# active sessions (handled by worktree-helper.sh).
# --force-merged: uses gh pr list to detect squash merges and force-removes
# dirty worktrees when the PR is confirmed merged (dirty state = abandoned WIP).
#
# Safety: skips worktrees owned by active sessions (handled by worktree-helper.sh).
#######################################
cleanup_worktrees() {
local helper="${HOME}/.aidevops/agents/scripts/worktree-helper.sh"
if [[ ! -x "$helper" ]]; then
return 0
fi

# Run from the main repo directory (worktree-helper needs a git context)
local cleaned_output
cleaned_output=$(bash "$helper" clean --auto 2>&1) || true
local repos_json="${HOME}/.config/aidevops/repos.json"
local total_removed=0

if [[ -f "$repos_json" ]] && command -v jq &>/dev/null; then
# Iterate all repos, skip local_only (no GitHub remote for PR detection)
local repo_paths
repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" 2>/dev/null || echo "")
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid redirecting stderr to /dev/null. If repos.json is malformed or jq fails for another reason, the error message will be suppressed, making debugging difficult. The || echo "" already provides a safe fallback if the command fails, so stderr can be allowed to print for better diagnostics.

Suggested change
repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" 2>/dev/null || echo "")
repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" || echo "")
References
  1. When reporting errors for failed file operations in shell scripts, such as 'jq' writes, include the file path in the error message. Avoid suppressing stderr with '2>/dev/null' to ensure that diagnostic information about malformed files or write failures is visible.


local repo_path
while IFS= read -r repo_path; do
[[ -z "$repo_path" ]] && continue
[[ ! -d "$repo_path/.git" ]] && continue

local cleaned_output
cleaned_output=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l)
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid redirecting stderr to /dev/null. While a check for the .git directory exists, the git command could still fail for other reasons (e.g., a corrupted repository). Suppressing stderr hides these potentially important errors, hindering debugging.

Suggested change
cleaned_output=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l)
cleaned_output=$(git -C "$repo_path" worktree list | wc -l)
References
  1. When using git commands (like 'init', 'remote') in shell scripts, use the '-q' flag to suppress standard output instead of '2>/dev/null', ensuring that error messages on stderr remain visible for debugging.

# Skip repos with only 1 worktree (the main one) — nothing to clean
if [[ "$cleaned_output" -le 1 ]]; then
continue
fi

# Log only if something was cleaned
if echo "$cleaned_output" | grep -q "Removing\|removed"; then
echo "[pulse-wrapper] Worktree cleanup: $(echo "$cleaned_output" | grep -c 'Removing') worktree(s) removed" >>"$LOGFILE"
# Run helper in a subshell cd'd to the repo (it uses git rev-parse --show-toplevel)
local clean_result
clean_result=$(cd "$repo_path" && bash "$helper" clean --auto --force-merged 2>&1) || true

local count
count=$(echo "$clean_result" | grep -c 'Removing' || echo "0")
if [[ "$count" -gt 0 ]]; then
local repo_name
repo_name=$(basename "$repo_path")
echo "[pulse-wrapper] Worktree cleanup ($repo_name): $count worktree(s) removed" >>"$LOGFILE"
total_removed=$((total_removed + count))
fi
done <<<"$repo_paths"
else
# Fallback: just clean the current repo (legacy behaviour)
local cleaned_output
cleaned_output=$(bash "$helper" clean --auto --force-merged 2>&1) || true
if echo "$cleaned_output" | grep -q "Removing\|removed"; then
echo "[pulse-wrapper] Worktree cleanup: $(echo "$cleaned_output" | grep -c 'Removing') worktree(s) removed" >>"$LOGFILE"
fi
fi

return 0
}

Expand Down
90 changes: 69 additions & 21 deletions .agents/scripts/worktree-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# remove <path|branch> Remove a worktree
# status Show current worktree info
# switch <branch> Open/create worktree for branch (prints path)
# clean [--auto] Remove worktrees for merged branches
# clean [--auto] [--force-merged] Remove worktrees for merged branches
# help Show this help
#
# Examples:
Expand Down Expand Up @@ -680,10 +680,15 @@ cmd_switch() {

cmd_clean() {
local auto_mode=false
if [[ "${1:-}" == "--auto" ]]; then
auto_mode=true
local force_merged=false
while [[ $# -gt 0 ]]; do
case "${1:-}" in
--auto) auto_mode=true ;;
--force-merged) force_merged=true ;;
*) break ;;
esac
shift
fi
done

echo -e "${BOLD}Checking for worktrees with merged branches...${NC}"
echo ""
Expand All @@ -698,6 +703,13 @@ cmd_clean() {
# Fetch to get current remote branch state (detects deleted branches)
git fetch --prune origin 2>/dev/null || true
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid redirecting stderr to /dev/null. If git fetch fails (e.g., due to network or authentication issues), the error message will be suppressed, making debugging difficult. The || true already prevents the script from exiting on failure, so it's safe to let error messages be displayed.

Suggested change
git fetch --prune origin 2>/dev/null || true
git fetch --prune origin || true
References
  1. Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.


# Build a lookup of merged PR branches for squash-merge detection.
# gh pr list only returns squash-merged PRs that git branch --merged misses.
local merged_pr_branches=""
if command -v gh &>/dev/null; then
merged_pr_branches=$(gh pr list --state merged --limit 200 --json headRefName --jq '.[].headRefName' 2>/dev/null || echo "")
fi

while IFS= read -r line; do
if [[ "$line" =~ ^worktree\ (.+)$ ]]; then
worktree_path="${BASH_REMATCH[1]}"
Expand All @@ -719,14 +731,13 @@ cmd_clean() {
! git show-ref --verify --quiet "refs/remotes/origin/$worktree_branch" 2>/dev/null; then
is_merged=true
merge_type="remote deleted"
fi

# Safety check: skip if worktree has uncommitted changes
if [[ "$is_merged" == "true" ]] && worktree_has_changes "$worktree_path"; then
echo -e " ${RED}$worktree_branch${NC} (has uncommitted changes - skipping)"
echo " $worktree_path"
echo ""
is_merged=false
# Check 3: Squash-merge detection via GitHub PR state
# GitHub squash merges create a new commit — the original branch is NOT
# an ancestor of the target, so git branch --merged misses it. The remote
# branch may still exist if "auto-delete head branches" is off.
elif [[ -n "$merged_pr_branches" ]] && echo "$merged_pr_branches" | grep -qx "$worktree_branch"; then
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

This line has two issues:

  1. Regex Injection Vulnerability (Medium Severity): The grep -qx "$worktree_branch" command treats the branch name as a regular expression, which can lead to unintended matches and potential deletion of unrelated worktrees if the branch name contains regex metacharacters.
  2. Performance Issue: Using grep inside a loop is inefficient.

To address both, it's recommended to use grep -Fqx to treat the branch name as a literal string, and for performance, consider loading merged branch names into a bash associative array for O(1) lookup. The suggested code below uses an associative array lookup, which also removes 2>/dev/null from the gh command, improving debuggability.

Suggested change
elif [[ -n "$merged_pr_branches" ]] && echo "$merged_pr_branches" | grep -qx "$worktree_branch"; then
elif [[ -v "merged_pr_lookup[$worktree_branch]" ]]; then

is_merged=true
merge_type="squash-merged PR"
fi

# Ownership check (t189): skip if owned by another active session
Expand All @@ -741,6 +752,19 @@ cmd_clean() {
is_merged=false
fi

# Dirty check: behaviour depends on --force-merged flag
if [[ "$is_merged" == "true" ]] && worktree_has_changes "$worktree_path"; then
if [[ "$force_merged" == "true" ]]; then
# PR is confirmed merged — dirty state is abandoned WIP, safe to force-remove
merge_type="$merge_type, dirty (force)"
else
echo -e " ${RED}$worktree_branch${NC} (has uncommitted changes - skipping)"
echo " $worktree_path"
echo ""
is_merged=false
fi
fi

if [[ "$is_merged" == "true" ]]; then
found_any=true
echo -e " ${YELLOW}$worktree_branch${NC} ($merge_type)"
Expand Down Expand Up @@ -780,13 +804,10 @@ cmd_clean() {
elif [[ -z "$line" ]]; then
if [[ -n "$worktree_branch" ]] && [[ "$worktree_branch" != "$default_branch" ]]; then
local should_remove=false
local use_force=false

# Safety check: never remove worktrees with uncommitted changes
if worktree_has_changes "$worktree_path"; then
echo -e "${RED}Skipping $worktree_branch - has uncommitted changes${NC}"
should_remove=false
# Ownership check (t189): never remove worktrees owned by other sessions
elif is_worktree_owned_by_others "$worktree_path"; then
if is_worktree_owned_by_others "$worktree_path"; then
local rm_owner_info
rm_owner_info=$(check_worktree_owner "$worktree_path")
local rm_owner_pid
Expand All @@ -800,17 +821,40 @@ cmd_clean() {
elif branch_was_pushed "$worktree_branch" &&
! git show-ref --verify --quiet "refs/remotes/origin/$worktree_branch" 2>/dev/null; then
should_remove=true
# Check 3: Squash-merged PR
elif [[ -n "$merged_pr_branches" ]] && echo "$merged_pr_branches" | grep -qx "$worktree_branch"; then
should_remove=true
fi

# If should_remove but has changes, need --force-merged to proceed
if [[ "$should_remove" == "true" ]] && worktree_has_changes "$worktree_path"; then
if [[ "$force_merged" == "true" ]]; then
use_force=true
else
echo -e "${RED}Skipping $worktree_branch - has uncommitted changes${NC}"
should_remove=false
fi
fi

if [[ "$should_remove" == "true" ]]; then
echo -e "${BLUE}Removing $worktree_branch...${NC}"
# Clean up aidevops runtime files before removal
# Clean up heavy directories first to speed up removal
# (node_modules, .next, .turbo can have 100k+ files)
rm -rf "$worktree_path/node_modules" 2>/dev/null || true
rm -rf "$worktree_path/.next" 2>/dev/null || true
rm -rf "$worktree_path/.turbo" 2>/dev/null || true
Comment on lines +843 to +845
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-high high

The cmd_clean function iterates over all worktrees returned by git worktree list. It does not distinguish between the main worktree and linked worktrees. If the main worktree is currently on a branch that has been merged (and is not the default branch), the script will proceed to "clean up" heavy directories like node_modules, .next, and .turbo using rm -rf before attempting to remove the worktree. While git worktree remove will fail for the main worktree, the rm -rf commands will succeed, leading to the loss of build artifacts and dependencies in the main repository.

Remediation: The script should identify and skip the main worktree. This can be done by checking if the worktree path is the same as the base repository path or by skipping the first entry in the git worktree list --porcelain output.

# Clean up aidevops runtime files
rm -rf "$worktree_path/.agents/loop-state" 2>/dev/null || true
rm -rf "$worktree_path/.agents/tmp" 2>/dev/null || true
rm -f "$worktree_path/.agents/.DS_Store" 2>/dev/null || true
rmdir "$worktree_path/.agent" 2>/dev/null || true
# Don't use --force to prevent data loss
if ! git worktree remove "$worktree_path" 2>/dev/null; then

local remove_flag=""
if [[ "$use_force" == "true" ]]; then
remove_flag="--force"
fi
# shellcheck disable=SC2086
if ! git worktree remove $remove_flag "$worktree_path" 2>/dev/null; then
Copy link

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid redirecting stderr to /dev/null. The error message from git worktree remove can be more informative than the custom message, especially if the failure is due to something other than uncommitted changes (e.g., permissions). Allowing the original error to be displayed will aid in debugging.

Suggested change
if ! git worktree remove $remove_flag "$worktree_path" 2>/dev/null; then
if ! git worktree remove $remove_flag "$worktree_path"; then
References
  1. Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.

echo -e "${RED}Failed to remove $worktree_branch - may have uncommitted changes${NC}"
else
# Unregister ownership (t189)
Expand Down Expand Up @@ -934,8 +978,12 @@ COMMANDS

switch <branch> Get/create worktree for branch (prints path)

clean [--auto] Remove worktrees for merged branches
clean [--auto] [--force-merged]
Remove worktrees for merged branches
--auto: skip confirmation prompt (for automated cleanup)
--force-merged: force-remove dirty worktrees when PR is
confirmed merged (dirty state = abandoned WIP). Also
detects squash merges via gh pr list.
Skips worktrees owned by other active sessions (t189)

registry [list|prune] View or prune the ownership registry (t189, t197)
Expand Down
Loading