From 9690a1afb2e47b950399a31f4fbac965ccd9086e Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sat, 25 Apr 2026 15:39:41 +0200 Subject: [PATCH] Treat already-pruned finish reruns as complete A finish rerun can happen after GitHub already merged the PR and branch cleanup already removed the local source branch. The old path failed before it could verify remote PR truth, which made successful manual closeouts look failed. Constraint: GitHub PR state is the only durable proof after local branch/worktree cleanup. Rejected: Ignore every missing local branch | would hide unmerged or mistyped branch names. Confidence: high Scope-risk: narrow Directive: Keep missing-branch success gated on merged PR evidence; do not turn it into unconditional cleanup success. Tested: bash -n scripts/agent-branch-finish.sh templates/scripts/agent-branch-finish.sh Tested: node --test test/finish.test.js Tested: node --test test/metadata.test.js Tested: openspec validate --specs --- scripts/agent-branch-finish.sh | 61 ++++++++++++++++++++++ templates/scripts/agent-branch-finish.sh | 61 ++++++++++++++++++++++ test/finish.test.js | 65 ++++++++++++++++++++++++ 3 files changed, 187 insertions(+) diff --git a/scripts/agent-branch-finish.sh b/scripts/agent-branch-finish.sh index d63539c..104ef8f 100755 --- a/scripts/agent-branch-finish.sh +++ b/scripts/agent-branch-finish.sh @@ -227,7 +227,68 @@ if [[ "$SOURCE_BRANCH" == "$BASE_BRANCH" ]]; then exit 1 fi +cleanup_missing_merged_source_branch() { + local state_line="" + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + local remote_delete_output="" + local prune_args=() + + if [[ "$MERGE_MODE" != "pr" || "$CLEANUP_AFTER_MERGE" -ne 1 ]]; then + return 1 + fi + if ! command -v "$GH_BIN" >/dev/null 2>&1; then + return 1 + fi + + state_line="$("$GH_BIN" pr list \ + --state merged \ + --head "$SOURCE_BRANCH" \ + --base "$BASE_BRANCH" \ + --json state,mergedAt,url \ + --jq 'sort_by(.mergedAt // "") | reverse | (.[0] // {}) | [(.state // ""), (.mergedAt // ""), (.url // "")] | join("\u001f")' \ + 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + IFS=$'\x1f' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + if [[ "$parsed_state" != "MERGED" && -z "$parsed_merged_at" ]]; then + return 1 + fi + + echo "[agent-branch-finish] Local source branch '${SOURCE_BRANCH}' is already absent, but a merged PR exists; continuing cleanup." >&2 + if [[ -n "$parsed_url" ]]; then + echo "[agent-branch-finish] Merged PR: ${parsed_url}" >&2 + fi + + run_guardex_cli locks release --branch "$SOURCE_BRANCH" >/dev/null 2>&1 || true + + 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 + if ! remote_delete_output="$(git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" 2>&1)"; then + echo "[agent-branch-finish] Warning: remote branch cleanup failed for '${SOURCE_BRANCH}'." >&2 + [[ -n "$remote_delete_output" ]] && echo "$remote_delete_output" >&2 + fi + fi + fi + + prune_args=(worktree prune --base "$BASE_BRANCH" --only-dirty-worktrees --delete-branches) + if [[ "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then + prune_args+=(--delete-remote-branches) + fi + if ! run_guardex_cli "${prune_args[@]}"; then + echo "[agent-branch-finish] Warning: automatic worktree prune failed." >&2 + echo "[agent-branch-finish] You can run manual cleanup: gx cleanup --base ${BASE_BRANCH}" >&2 + fi + + echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via pr flow and found source branch/worktree already cleaned." + exit 0 +} + if ! git -C "$repo_root" show-ref --verify --quiet "refs/heads/${SOURCE_BRANCH}"; then + cleanup_missing_merged_source_branch echo "[agent-branch-finish] Local source branch does not exist: ${SOURCE_BRANCH}" >&2 exit 1 fi diff --git a/templates/scripts/agent-branch-finish.sh b/templates/scripts/agent-branch-finish.sh index d63539c..104ef8f 100755 --- a/templates/scripts/agent-branch-finish.sh +++ b/templates/scripts/agent-branch-finish.sh @@ -227,7 +227,68 @@ if [[ "$SOURCE_BRANCH" == "$BASE_BRANCH" ]]; then exit 1 fi +cleanup_missing_merged_source_branch() { + local state_line="" + local parsed_state="" + local parsed_merged_at="" + local parsed_url="" + local remote_delete_output="" + local prune_args=() + + if [[ "$MERGE_MODE" != "pr" || "$CLEANUP_AFTER_MERGE" -ne 1 ]]; then + return 1 + fi + if ! command -v "$GH_BIN" >/dev/null 2>&1; then + return 1 + fi + + state_line="$("$GH_BIN" pr list \ + --state merged \ + --head "$SOURCE_BRANCH" \ + --base "$BASE_BRANCH" \ + --json state,mergedAt,url \ + --jq 'sort_by(.mergedAt // "") | reverse | (.[0] // {}) | [(.state // ""), (.mergedAt // ""), (.url // "")] | join("\u001f")' \ + 2>/dev/null || true)" + if [[ -z "$state_line" ]]; then + return 1 + fi + + IFS=$'\x1f' read -r parsed_state parsed_merged_at parsed_url <<< "$state_line" + if [[ "$parsed_state" != "MERGED" && -z "$parsed_merged_at" ]]; then + return 1 + fi + + echo "[agent-branch-finish] Local source branch '${SOURCE_BRANCH}' is already absent, but a merged PR exists; continuing cleanup." >&2 + if [[ -n "$parsed_url" ]]; then + echo "[agent-branch-finish] Merged PR: ${parsed_url}" >&2 + fi + + run_guardex_cli locks release --branch "$SOURCE_BRANCH" >/dev/null 2>&1 || true + + 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 + if ! remote_delete_output="$(git -C "$repo_root" push origin --delete "$SOURCE_BRANCH" 2>&1)"; then + echo "[agent-branch-finish] Warning: remote branch cleanup failed for '${SOURCE_BRANCH}'." >&2 + [[ -n "$remote_delete_output" ]] && echo "$remote_delete_output" >&2 + fi + fi + fi + + prune_args=(worktree prune --base "$BASE_BRANCH" --only-dirty-worktrees --delete-branches) + if [[ "$DELETE_REMOTE_BRANCH" -eq 1 ]]; then + prune_args+=(--delete-remote-branches) + fi + if ! run_guardex_cli "${prune_args[@]}"; then + echo "[agent-branch-finish] Warning: automatic worktree prune failed." >&2 + echo "[agent-branch-finish] You can run manual cleanup: gx cleanup --base ${BASE_BRANCH}" >&2 + fi + + echo "[agent-branch-finish] Merged '${SOURCE_BRANCH}' into '${BASE_BRANCH}' via pr flow and found source branch/worktree already cleaned." + exit 0 +} + if ! git -C "$repo_root" show-ref --verify --quiet "refs/heads/${SOURCE_BRANCH}"; then + cleanup_missing_merged_source_branch echo "[agent-branch-finish] Local source branch does not exist: ${SOURCE_BRANCH}" >&2 exit 1 fi diff --git a/test/finish.test.js b/test/finish.test.js index 0691c30..be40a3d 100644 --- a/test/finish.test.js +++ b/test/finish.test.js @@ -698,6 +698,71 @@ exit 1 assert.equal(result.stdout.trim(), '', 'agent branch should be deleted on origin'); }); +test('agent-branch-finish cleanup is idempotent after merged PR already pruned branch and worktree', () => { + 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-already-cleaned'], repoDir); + assert.equal(result.status, 0, result.stderr); + commitFile(repoDir, 'already-cleaned.txt', 'already cleaned\n', 'already cleaned change'); + result = runCmd('git', ['push', '-u', 'origin', 'agent/test-pr-already-cleaned'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['checkout', 'dev'], repoDir); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['branch', '-D', 'agent/test-pr-already-cleaned'], repoDir); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['push', 'origin', '--delete', 'agent/test-pr-already-cleaned'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const { fakePath: fakeGhPath } = createFakeGhScript(` +if [[ "$1" == "pr" && "$2" == "list" ]]; then + if [[ " $* " == *" --state merged "* ]] && [[ " $* " == *" --head agent/test-pr-already-cleaned "* ]] && [[ " $* " == *" --base dev "* ]]; then + printf 'MERGED\\x1f2026-04-25T11:03:00Z\\x1fhttps://example.test/pr/already-cleaned\\n' + exit 0 + fi + echo "unexpected gh pr list args: $*" >&2 + exit 1 +fi +if [[ "$1" == "pr" && ( "$2" == "create" || "$2" == "merge" || "$2" == "view" ) ]]; then + echo "already-cleaned rerun should not call gh pr $2" >&2 + exit 1 +fi +echo "unexpected gh args: $*" >&2 +exit 1 +`); + + const finish = runBranchFinish( + ['--branch', 'agent/test-pr-already-cleaned', '--base', 'dev', '--mode', 'pr', '--cleanup'], + repoDir, + { GUARDEX_GH_BIN: fakeGhPath }, + ); + assert.equal(finish.status, 0, finish.stderr || finish.stdout); + assert.match( + finish.stderr, + /Local source branch 'agent\/test-pr-already-cleaned' is already absent, but a merged PR exists; continuing cleanup\./, + ); + assert.match( + finish.stderr, + /Merged PR: https:\/\/example\.test\/pr\/already-cleaned/, + ); + assert.match( + finish.stdout, + /Merged 'agent\/test-pr-already-cleaned' into 'dev' via pr flow and found source branch\/worktree already cleaned\./, + ); +}); + test('agent-branch-finish cleanup succeeds from active agent worktree when base branch is checked out elsewhere', () => { const repoDir = initRepo();