From 68a2f90a2b042bbcd93b92c9969fee0b0e0a80e8 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Wed, 15 Apr 2026 11:10:49 +0200 Subject: [PATCH] Handle squash-merged PR branches in automated cleanup Extend cleanup with an optional merged-PR detection path so agent branches merged via squash can still be safely pruned. The post-merge hook now enables this mode, preserving clean unmerged worktrees while auto-removing branches confirmed as merged PR heads. Constraint: Existing branch ancestry checks miss squash merges because branch commits are not ancestors of base Rejected: Force-delete all clean agent branches on base merge | risks deleting active but unpushed work Confidence: high Scope-risk: moderate Directive: Keep PR-based branch deletion gated behind explicit include-pr-merged mode and clean-worktree checks Tested: node --check bin/multiagent-safety.js Tested: bash -n scripts/agent-worktree-prune.sh templates/githooks/post-merge .githooks/post-merge Tested: Manual post-merge hook simulation (flag wiring + non-base skip) Tested: Manual prune simulation with fake gh merged PR head (non-ancestor branch removed) Not-tested: Full node --test test/install.test.js behavioral suite in this sandbox (spawn-limited runner executes smoke path only) --- .githooks/post-merge | 1 + bin/multiagent-safety.js | 8 ++++ scripts/agent-worktree-prune.sh | 84 ++++++++++++++++++++++++++++++--- templates/githooks/post-merge | 1 + test/install.test.js | 47 ++++++++++++++++++ 5 files changed, 134 insertions(+), 7 deletions(-) diff --git a/.githooks/post-merge b/.githooks/post-merge index 5c690b5..20dfd41 100755 --- a/.githooks/post-merge +++ b/.githooks/post-merge @@ -37,6 +37,7 @@ fi "$node_bin" "$cli_path" cleanup \ --target "$repo_root" \ --base "$base_branch" \ + --include-pr-merged \ --keep-clean-worktrees >/dev/null 2>&1 || true exit 0 diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 01615ac..45a84b2 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -2404,6 +2404,7 @@ function parseCleanupArgs(rawArgs) { forceDirty: false, keepRemote: false, keepCleanWorktrees: false, + includePrMerged: false, idleMinutes: 0, watch: false, intervalSeconds: 60, @@ -2455,6 +2456,10 @@ function parseCleanupArgs(rawArgs) { options.keepCleanWorktrees = true; continue; } + if (arg === '--include-pr-merged') { + options.includePrMerged = true; + continue; + } if (arg === '--idle-minutes') { const next = rawArgs[index + 1]; if (!next) { @@ -4564,6 +4569,9 @@ function cleanup(rawArgs) { if (!options.keepCleanWorktrees) { args.push('--only-dirty-worktrees'); } + if (options.includePrMerged) { + args.push('--include-pr-merged'); + } if (options.idleMinutes > 0) { args.push('--idle-minutes', String(options.idleMinutes)); } diff --git a/scripts/agent-worktree-prune.sh b/scripts/agent-worktree-prune.sh index 7da09cd..4bc8162 100755 --- a/scripts/agent-worktree-prune.sh +++ b/scripts/agent-worktree-prune.sh @@ -8,11 +8,16 @@ FORCE_DIRTY=0 DELETE_BRANCHES=0 DELETE_REMOTE_BRANCHES=0 ONLY_DIRTY_WORKTREES=0 +INCLUDE_PR_MERGED=0 TARGET_BRANCH="" IDLE_MINUTES=0 NOW_EPOCH_RAW="${MUSAFETY_PRUNE_NOW_EPOCH:-}" IDLE_SECONDS=0 NOW_EPOCH=0 +GH_BIN="${MUSAFETY_GH_BIN:-gh}" +PR_MERGED_LOOKUP_DISABLED=0 +PR_MERGED_LOOKUP_LOADED=0 +declare -A MERGED_PR_BRANCHES=() if [[ -n "$BASE_BRANCH" ]]; then BASE_BRANCH_EXPLICIT=1 @@ -45,6 +50,10 @@ while [[ $# -gt 0 ]]; do ONLY_DIRTY_WORKTREES=1 shift ;; + --include-pr-merged) + INCLUDE_PR_MERGED=1 + shift + ;; --branch) TARGET_BRANCH="${2:-}" shift 2 @@ -55,7 +64,7 @@ while [[ $# -gt 0 ]]; do ;; *) echo "[agent-worktree-prune] Unknown argument: $1" >&2 - echo "Usage: $0 [--base ] [--dry-run] [--force-dirty] [--delete-branches] [--delete-remote-branches] [--only-dirty-worktrees] [--branch ] [--idle-minutes ]" >&2 + echo "Usage: $0 [--base ] [--dry-run] [--force-dirty] [--delete-branches] [--delete-remote-branches] [--only-dirty-worktrees] [--include-pr-merged] [--branch ] [--idle-minutes ]" >&2 exit 1 ;; esac @@ -101,6 +110,44 @@ resolve_base_branch() { printf '%s' "" } +load_merged_pr_branches() { + if [[ "$INCLUDE_PR_MERGED" -ne 1 ]]; then + return 1 + fi + if [[ "$PR_MERGED_LOOKUP_DISABLED" -eq 1 ]]; then + return 1 + fi + if [[ "$PR_MERGED_LOOKUP_LOADED" -eq 1 ]]; then + return 0 + fi + if ! command -v "$GH_BIN" >/dev/null 2>&1; then + PR_MERGED_LOOKUP_DISABLED=1 + return 1 + fi + + local merged_branches="" + merged_branches="$( + "$GH_BIN" pr list --state merged --base "$BASE_BRANCH" --limit 200 --json headRefName --jq '.[].headRefName' 2>/dev/null || true + )" + if [[ -n "$merged_branches" ]]; then + while IFS= read -r merged_branch; do + [[ -z "$merged_branch" ]] && continue + MERGED_PR_BRANCHES["$merged_branch"]=1 + done <<< "$merged_branches" + fi + PR_MERGED_LOOKUP_LOADED=1 + return 0 +} + +branch_has_merged_pr() { + local branch="$1" + if [[ "$INCLUDE_PR_MERGED" -ne 1 ]]; then + return 1 + fi + load_merged_pr_branches || return 1 + [[ -n "${MERGED_PR_BRANCHES[$branch]:-}" ]] +} + if [[ "$BASE_BRANCH_EXPLICIT" -eq 1 && -z "$BASE_BRANCH" ]]; then echo "[agent-worktree-prune] --base requires a non-empty branch name." >&2 exit 1 @@ -342,6 +389,7 @@ process_entry() { fi local remove_reason="" + local branch_delete_mode="safe" if [[ -z "$branch_ref" ]]; then remove_reason="detached-worktree" @@ -352,6 +400,9 @@ process_entry() { if [[ "$DELETE_BRANCHES" -eq 1 ]]; then remove_reason="merged-agent-branch" fi + elif [[ "$DELETE_BRANCHES" -eq 1 ]] && branch_has_merged_pr "$branch"; then + remove_reason="merged-agent-pr" + branch_delete_mode="force" elif [[ "$ONLY_DIRTY_WORKTREES" -eq 1 ]] && is_clean_worktree "$wt"; then remove_reason="clean-agent-worktree" fi @@ -383,13 +434,19 @@ process_entry() { if git -C "$repo_root" show-ref --verify --quiet "refs/heads/${branch}" && ! branch_has_worktree "$branch"; then if [[ "$branch" == agent/* && "$DELETE_BRANCHES" -eq 1 ]]; then - if run_cmd git -C "$repo_root" branch -d "$branch" >/dev/null 2>&1; then + local delete_flag="-d" + local deleted_label="merged" + if [[ "$branch_delete_mode" == "force" ]]; then + delete_flag="-D" + deleted_label="merged PR" + fi + if run_cmd git -C "$repo_root" branch "$delete_flag" "$branch" >/dev/null 2>&1; then removed_branches=$((removed_branches + 1)) - echo "[agent-worktree-prune] Deleted merged branch: ${branch}" + echo "[agent-worktree-prune] Deleted ${deleted_label} branch: ${branch}" if [[ "$DELETE_REMOTE_BRANCHES" -eq 1 ]]; then if git -C "$repo_root" ls-remote --exit-code --heads origin "$branch" >/dev/null 2>&1; then run_cmd git -C "$repo_root" push origin --delete "$branch" >/dev/null 2>&1 || true - echo "[agent-worktree-prune] Deleted merged remote branch: ${branch}" + echo "[agent-worktree-prune] Deleted ${deleted_label} remote branch: ${branch}" fi fi fi @@ -436,14 +493,27 @@ if [[ "$DELETE_BRANCHES" -eq 1 ]]; then if ! branch_idle_gate "$branch" "" "stale-merged-branch"; then continue fi + local merged_by_ancestor=0 + local merged_by_pr=0 if git -C "$repo_root" merge-base --is-ancestor "$branch" "$BASE_BRANCH"; then - if run_cmd git -C "$repo_root" branch -d "$branch" >/dev/null 2>&1; then + merged_by_ancestor=1 + elif branch_has_merged_pr "$branch"; then + merged_by_pr=1 + fi + if [[ "$merged_by_ancestor" -eq 1 || "$merged_by_pr" -eq 1 ]]; then + local delete_flag="-d" + local deleted_label="merged" + if [[ "$merged_by_pr" -eq 1 && "$merged_by_ancestor" -eq 0 ]]; then + delete_flag="-D" + deleted_label="merged PR" + fi + if run_cmd git -C "$repo_root" branch "$delete_flag" "$branch" >/dev/null 2>&1; then removed_branches=$((removed_branches + 1)) - echo "[agent-worktree-prune] Deleted stale merged branch: ${branch}" + echo "[agent-worktree-prune] Deleted stale ${deleted_label} branch: ${branch}" if [[ "$DELETE_REMOTE_BRANCHES" -eq 1 ]]; then if git -C "$repo_root" ls-remote --exit-code --heads origin "$branch" >/dev/null 2>&1; then run_cmd git -C "$repo_root" push origin --delete "$branch" >/dev/null 2>&1 || true - echo "[agent-worktree-prune] Deleted stale merged remote branch: ${branch}" + echo "[agent-worktree-prune] Deleted stale ${deleted_label} remote branch: ${branch}" fi fi fi diff --git a/templates/githooks/post-merge b/templates/githooks/post-merge index 5c690b5..20dfd41 100755 --- a/templates/githooks/post-merge +++ b/templates/githooks/post-merge @@ -37,6 +37,7 @@ fi "$node_bin" "$cli_path" cleanup \ --target "$repo_root" \ --base "$base_branch" \ + --include-pr-merged \ --keep-clean-worktrees >/dev/null 2>&1 || true exit 0 diff --git a/test/install.test.js b/test/install.test.js index 46eec84..806e99e 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -1857,6 +1857,7 @@ test('post-merge auto-runs cleanup on base branch and skips non-base branches', assert.match(invocations[0], /^cleanup /); assert.match(invocations[0], new RegExp(`--target ${escapeRegexLiteral(repoDir)}`)); assert.match(invocations[0], /--base dev/); + assert.match(invocations[0], /--include-pr-merged/); assert.match(invocations[0], /--keep-clean-worktrees/); result = runCmd('git', ['checkout', '-b', 'feature/post-merge-skip'], repoDir); @@ -3333,6 +3334,52 @@ test('cleanup command keeps unmerged agent branch refs but removes clean agent w assert.equal(localBranch.status, 0, 'cleanup should keep unmerged local branch'); }); +test('cleanup command can remove squash-merged agent branches via merged PR detection', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const worktreePath = path.join(repoDir, '.omx', 'agent-worktrees', 'agent__cleanup-pr-merged'); + result = runCmd('git', ['worktree', 'add', '-b', 'agent/test-cleanup-pr-merged', worktreePath, 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.writeFileSync(path.join(worktreePath, 'feature.txt'), 'feature branch commit\n', 'utf8'); + result = runCmd('git', ['-C', worktreePath, 'add', 'feature.txt'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['-C', worktreePath, 'commit', '-m', 'feature commit'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const { fakePath: fakeGhPath } = createFakeGhScript( + 'if [[ "$1" == "pr" && "$2" == "list" ]]; then\n' + + ' printf \'%s\\n\' "agent/test-cleanup-pr-merged"\n' + + ' exit 0\n' + + 'fi\n' + + 'exit 1', + ); + + result = runNodeWithEnv( + [ + 'cleanup', + '--target', + repoDir, + '--branch', + 'agent/test-cleanup-pr-merged', + '--keep-remote', + '--keep-clean-worktrees', + '--include-pr-merged', + ], + repoDir, + { MUSAFETY_GH_BIN: fakeGhPath }, + ); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const localBranch = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-cleanup-pr-merged'], repoDir); + assert.notEqual(localBranch.status, 0, 'cleanup should remove merged PR local branch'); + assert.equal(fs.existsSync(worktreePath), false, 'cleanup should remove merged PR worktree'); +}); + test('cleanup command watch mode defaults to 10-minute idle threshold and supports one-cycle execution', () => { const repoDir = initRepo(); const scriptsDir = path.join(repoDir, 'scripts');