Skip to content

GH#2941: fix multi-repo worktree auto-cleanup in pulse-wrapper.sh #2944

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
Mar 6, 2026
Merged

GH#2941: fix multi-repo worktree auto-cleanup in pulse-wrapper.sh #2944

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
45 changes: 30 additions & 15 deletions .agents/scripts/pulse-wrapper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,15 +1209,22 @@ ${state_content}
#######################################
# Clean up worktrees for merged/closed PRs across ALL managed repos
#
# 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.
# Iterates repos.json (.initialized_repos[]) 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.
#
# --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).
# worktree-helper.sh clean internally:
# 1. Runs git fetch --prune origin (prunes deleted remote branches)
# 2. Checks refs/remotes/origin/<branch> for each worktree
# 3. Detects squash merges via gh pr list --state merged
# 4. Removes worktrees + deletes local branches for merged PRs
#
# Safety: skips worktrees owned by active sessions (handled by worktree-helper.sh).
# --force-merged: force-removes dirty worktrees when the PR is confirmed
# merged (dirty state = abandoned WIP from a completed worker).
#
# Safety: skips worktrees owned by active sessions (handled by
# worktree-helper.sh ownership registry, t189).
#######################################
cleanup_worktrees() {
local helper="${HOME}/.aidevops/agents/scripts/worktree-helper.sh"
Expand All @@ -1229,19 +1236,22 @@ cleanup_worktrees() {
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)
# Iterate all initialized repos — clean worktrees for any repo with
# a git directory, not just pulse-enabled ones. Workers can create
# worktrees in any managed repo. Skip local_only repos since
# worktree-helper.sh uses gh pr list for squash-merge detection.
local repo_paths
repo_paths=$(jq -r '.[] | select(.local_only != true) | .path' "$repos_json" 2>/dev/null || echo "")
repo_paths=$(jq -r '.initialized_repos[] | select((.local_only // false) == false) | .path' "$repos_json" 2>/dev/null || echo "")
Copy link

@gemini-code-assist gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

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

medium

While the || echo "" correctly handles potential jq command failures when set -e is active, suppressing stderr with 2>/dev/null can hide valuable diagnostic information. If the repos.json file is malformed or there are permission issues, those errors would be silenced. It's better to let these errors be visible for easier debugging.

As per the repository's general rules, it's preferable to avoid blanket error suppression.

Suggested change
repo_paths=$(jq -r '.initialized_repos[] | select((.local_only // false) == false) | .path' "$repos_json" 2>/dev/null || echo "")
repo_paths=$(jq -r '.initialized_repos[] | select((.local_only // false) == false) | .path' "$repos_json" || echo "")
References
  1. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.


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)
local wt_count
wt_count=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l | tr -d ' ')
Copy link

@gemini-code-assist gemini-code-assist bot Mar 6, 2026

Choose a reason for hiding this comment

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

medium

Similar to the jq command, suppressing stderr for this git command with 2>/dev/null is not ideal. Although you check for the existence of the .git directory beforehand, other issues like file permissions could cause git worktree list to fail. Allowing these errors to be reported would make debugging easier.

The repository's general rules advise against suppressing stderr when a file's existence has already been checked, as it can mask other important issues.

Suggested change
wt_count=$(git -C "$repo_path" worktree list 2>/dev/null | wc -l | tr -d ' ')
wt_count=$(git -C "$repo_path" worktree list | wc -l | tr -d ' ')
References
  1. Avoid using 2>/dev/null to suppress errors on file operations if the file's existence has already been verified by a preceding check (e.g., [[ -f "$file" ]] or an early return). This practice is redundant for 'file not found' errors and can mask other important issues like permissions problems.

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

Expand All @@ -1260,15 +1270,20 @@ cleanup_worktrees() {
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
local clean_result
clean_result=$(bash "$helper" clean --auto --force-merged 2>&1) || true
local fallback_count
fallback_count=$(echo "$cleaned_output" | grep -c 'Removing') || fallback_count=0
fallback_count=$(echo "$clean_result" | grep -c 'Removing') || fallback_count=0
if [[ "$fallback_count" -gt 0 ]]; then
echo "[pulse-wrapper] Worktree cleanup: $fallback_count worktree(s) removed" >>"$LOGFILE"
total_removed=$((total_removed + fallback_count))
fi
fi

if [[ "$total_removed" -gt 0 ]]; then
echo "[pulse-wrapper] Worktree cleanup total: $total_removed worktree(s) removed across all repos" >>"$LOGFILE"
fi

return 0
}

Expand Down
Loading