From b00a1d25960ad8f4ff9cf63eff70419b371e46ef Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sat, 11 Apr 2026 13:58:58 +0200 Subject: [PATCH] Keep agent-branch-finish fully automatic after PR merges When GitHub merges a PR but fails only on local branch deletion (active worktree), the finish script previously treated that as a merge failure and skipped cleanup. This patch detects that specific gh error signature, treats merge as successful, and continues local/remote branch cleanup. Constraint: Preserve existing PR fallback behavior for true policy/check blockers Rejected: Retrying gh merge without --delete-branch | still leaves local/remote cleanup fragmented across manual steps Confidence: high Scope-risk: narrow Reversibility: clean Directive: If gh merge output format changes, keep this matcher aligned with real CLI error text before loosening detection Tested: npm test (44/44), including new regression for local-branch-delete error path in PR mode Not-tested: Live GitHub PR merge with this exact local-delete failure on every gh version --- scripts/agent-branch-finish.sh | 15 +++++ templates/scripts/agent-branch-finish.sh | 15 +++++ test/install.test.js | 72 ++++++++++++++++++++++++ 3 files changed, 102 insertions(+) diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index a076529..1f33189 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -196,6 +196,17 @@ merge_status="direct" direct_push_error="" pr_url="" +is_local_branch_delete_error() { + local output="$1" + if [[ "$output" != *"failed to delete local branch"* ]]; then + return 1 + fi + if [[ "$output" == *"cannot delete branch"* ]] || [[ "$output" == *"used by worktree"* ]]; then + return 0 + fi + return 1 +} + run_pr_flow() { if ! command -v "$GH_BIN" >/dev/null 2>&1; then echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2 @@ -222,6 +233,10 @@ run_pr_flow() { if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then return 0 fi + if is_local_branch_delete_error "$merge_output"; then + echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2 + return 0 + fi auto_output="" if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index a076529..1f33189 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -196,6 +196,17 @@ merge_status="direct" direct_push_error="" pr_url="" +is_local_branch_delete_error() { + local output="$1" + if [[ "$output" != *"failed to delete local branch"* ]]; then + return 1 + fi + if [[ "$output" == *"cannot delete branch"* ]] || [[ "$output" == *"used by worktree"* ]]; then + return 0 + fi + return 1 +} + run_pr_flow() { if ! command -v "$GH_BIN" >/dev/null 2>&1; then echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2 @@ -222,6 +233,10 @@ run_pr_flow() { if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then return 0 fi + if is_local_branch_delete_error "$merge_output"; then + echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2 + return 0 + fi auto_output="" if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then diff --git a/test/install.test.js b/test/install.test.js index 140ad63..fcfc638 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -63,6 +63,14 @@ function createFakeCodexAuthScript(scriptBody) { return { fakeBin, fakePath }; } +function createFakeGhScript(scriptBody) { + const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'musafety-fake-gh-')); + const fakePath = path.join(fakeBin, 'gh'); + fs.writeFileSync(fakePath, `#!/usr/bin/env bash\nset -e\n${scriptBody}\n`, 'utf8'); + fs.chmodSync(fakePath, 0o755); + return { fakeBin, fakePath }; +} + function initRepo() { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'musafety-')); const repoDir = path.join(tempDir, 'repo'); @@ -755,6 +763,70 @@ test('agent-branch-finish blocks when source branch is behind origin/dev', () => assert.match(finish.stderr, /musafety sync --base dev/); }); +test('agent-branch-finish pr mode continues cleanup when gh merge only fails local branch deletion', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + attachOriginRemote(repoDir); + + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['add', '.'], repoDir); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['commit', '-m', 'apply musafety setup'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['push', 'origin', 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr); + + result = runCmd('git', ['checkout', '-b', 'agent/test-pr-delete-error'], repoDir); + assert.equal(result.status, 0, result.stderr); + commitFile(repoDir, 'agent-pr-delete.txt', 'agent change\n', 'agent change'); + + const { fakePath: fakeGhPath } = createFakeGhScript(` +if [[ "$1" == "pr" && "$2" == "create" ]]; then + exit 0 +fi +if [[ "$1" == "pr" && "$2" == "view" ]]; then + if [[ " $* " == *" --json url "* ]]; then + echo "https://example.test/pr/1" + exit 0 + fi + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + echo "failed to delete local branch $3: error: cannot delete branch '$3' used by worktree at '/tmp/demo-worktree'" >&2 + echo "/usr/bin/git: exit status 1" >&2 + exit 1 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + const finish = runCmd( + 'bash', + ['scripts/agent-branch-finish.sh', '--branch', 'agent/test-pr-delete-error', '--mode', 'pr'], + repoDir, + { MUSAFETY_GH_BIN: fakeGhPath }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match( + finish.stderr, + /PR merged but gh could not delete the local branch \(active worktree\); continuing local cleanup\./, + ); + assert.match( + finish.stdout, + /Merged 'agent\/test-pr-delete-error' into 'dev' via pr flow and removed branch\./, + ); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-delete-error'], repoDir); + assert.notEqual(result.status, 0, 'agent branch should be deleted locally'); + + result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-delete-error'], repoDir); + assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin'); +}); + test('OpenSpec plan workspace scaffold creates expected role/task structure', () => { const repoDir = initRepo();