Skip to content

Commit 193bb4f

Browse files
NagyViktNagyVikt
andauthored
Keep agent-branch-finish fully automatic after PR merges (#16)
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 Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent 26c0ffa commit 193bb4f

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

scripts/agent-branch-finish.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ merge_status="direct"
196196
direct_push_error=""
197197
pr_url=""
198198

199+
is_local_branch_delete_error() {
200+
local output="$1"
201+
if [[ "$output" != *"failed to delete local branch"* ]]; then
202+
return 1
203+
fi
204+
if [[ "$output" == *"cannot delete branch"* ]] || [[ "$output" == *"used by worktree"* ]]; then
205+
return 0
206+
fi
207+
return 1
208+
}
209+
199210
run_pr_flow() {
200211
if ! command -v "$GH_BIN" >/dev/null 2>&1; then
201212
echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2
@@ -222,6 +233,10 @@ run_pr_flow() {
222233
if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then
223234
return 0
224235
fi
236+
if is_local_branch_delete_error "$merge_output"; then
237+
echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2
238+
return 0
239+
fi
225240

226241
auto_output=""
227242
if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then

templates/scripts/agent-branch-finish.sh

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,17 @@ merge_status="direct"
196196
direct_push_error=""
197197
pr_url=""
198198

199+
is_local_branch_delete_error() {
200+
local output="$1"
201+
if [[ "$output" != *"failed to delete local branch"* ]]; then
202+
return 1
203+
fi
204+
if [[ "$output" == *"cannot delete branch"* ]] || [[ "$output" == *"used by worktree"* ]]; then
205+
return 0
206+
fi
207+
return 1
208+
}
209+
199210
run_pr_flow() {
200211
if ! command -v "$GH_BIN" >/dev/null 2>&1; then
201212
echo "[agent-branch-finish] PR fallback requested but GitHub CLI not found: ${GH_BIN}" >&2
@@ -222,6 +233,10 @@ run_pr_flow() {
222233
if merge_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch 2>&1)"; then
223234
return 0
224235
fi
236+
if is_local_branch_delete_error "$merge_output"; then
237+
echo "[agent-branch-finish] PR merged but gh could not delete the local branch (active worktree); continuing local cleanup." >&2
238+
return 0
239+
fi
225240

226241
auto_output=""
227242
if auto_output="$("$GH_BIN" pr merge "$SOURCE_BRANCH" --squash --delete-branch --auto 2>&1)"; then

test/install.test.js

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,14 @@ function createFakeCodexAuthScript(scriptBody) {
6363
return { fakeBin, fakePath };
6464
}
6565

66+
function createFakeGhScript(scriptBody) {
67+
const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'musafety-fake-gh-'));
68+
const fakePath = path.join(fakeBin, 'gh');
69+
fs.writeFileSync(fakePath, `#!/usr/bin/env bash\nset -e\n${scriptBody}\n`, 'utf8');
70+
fs.chmodSync(fakePath, 0o755);
71+
return { fakeBin, fakePath };
72+
}
73+
6674
function initRepo() {
6775
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'musafety-'));
6876
const repoDir = path.join(tempDir, 'repo');
@@ -755,6 +763,70 @@ test('agent-branch-finish blocks when source branch is behind origin/dev', () =>
755763
assert.match(finish.stderr, /musafety sync --base dev/);
756764
});
757765

766+
test('agent-branch-finish pr mode continues cleanup when gh merge only fails local branch deletion', () => {
767+
const repoDir = initRepo();
768+
seedCommit(repoDir);
769+
attachOriginRemote(repoDir);
770+
771+
let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);
772+
assert.equal(result.status, 0, result.stderr || result.stdout);
773+
result = runCmd('git', ['add', '.'], repoDir);
774+
assert.equal(result.status, 0, result.stderr);
775+
result = runCmd('git', ['commit', '-m', 'apply musafety setup'], repoDir, {
776+
ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1',
777+
});
778+
assert.equal(result.status, 0, result.stderr);
779+
result = runCmd('git', ['push', 'origin', 'dev'], repoDir);
780+
assert.equal(result.status, 0, result.stderr);
781+
782+
result = runCmd('git', ['checkout', '-b', 'agent/test-pr-delete-error'], repoDir);
783+
assert.equal(result.status, 0, result.stderr);
784+
commitFile(repoDir, 'agent-pr-delete.txt', 'agent change\n', 'agent change');
785+
786+
const { fakePath: fakeGhPath } = createFakeGhScript(`
787+
if [[ "$1" == "pr" && "$2" == "create" ]]; then
788+
exit 0
789+
fi
790+
if [[ "$1" == "pr" && "$2" == "view" ]]; then
791+
if [[ " $* " == *" --json url "* ]]; then
792+
echo "https://example.test/pr/1"
793+
exit 0
794+
fi
795+
echo "unexpected gh pr view args: $*" >&2
796+
exit 1
797+
fi
798+
if [[ "$1" == "pr" && "$2" == "merge" ]]; then
799+
echo "failed to delete local branch $3: error: cannot delete branch '$3' used by worktree at '/tmp/demo-worktree'" >&2
800+
echo "/usr/bin/git: exit status 1" >&2
801+
exit 1
802+
fi
803+
echo "unexpected gh args: $*" >&2
804+
exit 1
805+
`);
806+
807+
const finish = runCmd(
808+
'bash',
809+
['scripts/agent-branch-finish.sh', '--branch', 'agent/test-pr-delete-error', '--mode', 'pr'],
810+
repoDir,
811+
{ MUSAFETY_GH_BIN: fakeGhPath },
812+
);
813+
assert.equal(finish.status, 0, finish.stderr || finish.stdout);
814+
assert.match(
815+
finish.stderr,
816+
/PR merged but gh could not delete the local branch \(active worktree\); continuing local cleanup\./,
817+
);
818+
assert.match(
819+
finish.stdout,
820+
/Merged 'agent\/test-pr-delete-error' into 'dev' via pr flow and removed branch\./,
821+
);
822+
823+
result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-delete-error'], repoDir);
824+
assert.notEqual(result.status, 0, 'agent branch should be deleted locally');
825+
826+
result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-delete-error'], repoDir);
827+
assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin');
828+
});
829+
758830
test('OpenSpec plan workspace scaffold creates expected role/task structure', () => {
759831
const repoDir = initRepo();
760832

0 commit comments

Comments
 (0)