diff --git a/openspec/changes/agent-codex-fix-branch-finish-remote-delete-tail-2026-04-22-15-42/notes.md b/openspec/changes/agent-codex-fix-branch-finish-remote-delete-tail-2026-04-22-15-42/notes.md new file mode 100644 index 0000000..4318101 --- /dev/null +++ b/openspec/changes/agent-codex-fix-branch-finish-remote-delete-tail-2026-04-22-15-42/notes.md @@ -0,0 +1,20 @@ +# agent-codex-fix-branch-finish-remote-delete-tail-2026-04-22-15-42 (minimal / T1) + +Branch: `agent/codex/fix-branch-finish-remote-delete-tail-2026-04-22-15-42` + +`gx branch finish --via-pr --wait-for-merge --cleanup` can succeed in merging a PR, delete the local agent branch, and still exit non-zero if the trailing `git push origin --delete ` races with a branch that is already gone on the remote. The merge is already done at that point, so the cleanup tail should be idempotent. + +Scope: +- Treat `remote ref does not exist` during the explicit remote-delete tail as a successful already-cleaned state. +- Keep real remote-delete failures strict instead of masking them with a blanket ignore. +- Add a focused finish regression that simulates the race after a successful PR merge. + +Verification: +- `bash -n scripts/agent-branch-finish.sh templates/scripts/agent-branch-finish.sh` +- `node --test test/finish.test.js` + +## Cleanup + +- [ ] Run `gx branch finish --branch agent/codex/fix-branch-finish-remote-delete-tail-2026-04-22-15-42 --base main --via-pr --wait-for-merge --cleanup` +- [ ] Record PR URL + `MERGED` state in the completion handoff. +- [ ] Confirm sandbox worktree is gone (`git worktree list`, `git branch -a`). diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index d086624..6c47249 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -401,6 +401,14 @@ is_local_branch_delete_error() { return 1 } +is_remote_branch_missing_error() { + local output="$1" + if [[ "$output" == *"remote ref does not exist"* ]] || [[ "$output" == *"failed to push some refs"* ]]; then + return 0 + fi + return 1 +} + read_pr_state() { local state_line state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | join("\u001f")' 2>/dev/null || true)" @@ -603,7 +611,15 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then if [[ "$PUSH_ENABLED" -eq 1 && "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then if git -C "$repo_root" ls-remote --exit-code --heads origin "$SOURCE_BRANCH" >/dev/null 2>&1; then - git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" + remote_delete_output="" + if ! remote_delete_output="$(git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" 2>&1)"; then + if is_remote_branch_missing_error "$remote_delete_output"; then + echo "[agent-branch-finish] Remote branch '${SOURCE_BRANCH}' was already deleted; continuing cleanup." >&2 + else + echo "$remote_delete_output" >&2 + exit 1 + fi + fi fi fi diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index d086624..6c47249 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -401,6 +401,14 @@ is_local_branch_delete_error() { return 1 } +is_remote_branch_missing_error() { + local output="$1" + if [[ "$output" == *"remote ref does not exist"* ]] || [[ "$output" == *"failed to push some refs"* ]]; then + return 0 + fi + return 1 +} + read_pr_state() { local state_line state_line="$("$GH_BIN" pr view "$SOURCE_BRANCH" --json state,mergedAt,url --jq '[.state, (.mergedAt // ""), (.url // "")] | join("\u001f")' 2>/dev/null || true)" @@ -603,7 +611,15 @@ if [[ "$CLEANUP_AFTER_MERGE" -eq 1 ]]; then if [[ "$PUSH_ENABLED" -eq 1 && "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then if git -C "$repo_root" ls-remote --exit-code --heads origin "$SOURCE_BRANCH" >/dev/null 2>&1; then - git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" + remote_delete_output="" + if ! remote_delete_output="$(git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" 2>&1)"; then + if is_remote_branch_missing_error "$remote_delete_output"; then + echo "[agent-branch-finish] Remote branch '${SOURCE_BRANCH}' was already deleted; continuing cleanup." >&2 + else + echo "$remote_delete_output" >&2 + exit 1 + fi + fi fi fi diff --git a/test/finish.test.js b/test/finish.test.js index 871fc5f..9160e21 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -306,6 +306,84 @@ exit 1 }); +test('agent-branch-finish cleanup succeeds when remote delete reports an already-removed branch', () => { + 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 gx 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-remote-delete-race'], repoDir); + assert.equal(result.status, 0, result.stderr); + commitFile(repoDir, 'agent-pr-remote-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/2" + exit 0 + fi + echo "unexpected gh pr view args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && "$2" == "merge" ]]; then + exit 0 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + const realGit = runCmd('bash', ['-lc', 'command -v git'], repoDir); + assert.equal(realGit.status, 0, realGit.stderr || realGit.stdout); + const realGitPath = realGit.stdout.trim(); + const { fakeBin } = createFakeBin('git', ` +real_git="${realGitPath}" +if [[ "$1" == "-C" && "$3" == "push" && "$4" == "origin" && "$5" == "--delete" && "$6" == "agent/test-pr-remote-delete-race" ]]; then + "$real_git" "$@" >/dev/null 2>&1 || true + echo "error: unable to delete 'agent/test-pr-remote-delete-race': remote ref does not exist" >&2 + echo "error: failed to push some refs to 'origin'" >&2 + exit 1 +fi +"$real_git" "$@" +`); + + const finish = runBranchFinish( + ['--branch', 'agent/test-pr-remote-delete-race', '--mode', 'pr', '--cleanup'], + repoDir, + { + GUARDEX_GH_BIN: fakeGhPath, + PATH: `${fakeBin}:${process.env.PATH || ''}`, + }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match( + finish.stderr, + /Remote branch 'agent\/test-pr-remote-delete-race' was already deleted; continuing cleanup\./, + ); + assert.match( + finish.stdout, + /Merged 'agent\/test-pr-remote-delete-race' into 'dev' via pr flow and cleaned source branch\/worktree\./, + ); + + result = runCmd('git', ['show-ref', '--verify', '--quiet', 'refs/heads/agent/test-pr-remote-delete-race'], repoDir); + assert.notEqual(result.status, 0, 'agent branch should be deleted locally'); + + result = runCmd('git', ['ls-remote', '--heads', 'origin', 'agent/test-pr-remote-delete-race'], repoDir); + assert.equal(result.stdout.trim(), '', 'agent branch should be absent on origin'); +}); + + test('agent-branch-finish cleanup succeeds from active agent worktree when base branch is checked out elsewhere', () => { const repoDir = initRepo(); seedCommit(repoDir);