From e150b30cc14007ffa03979f361b4109f423f2c41 Mon Sep 17 00:00:00 2001 From: fullsend-code Date: Thu, 21 May 2026 14:55:41 +0000 Subject: [PATCH] fix(#1301): delete stale remote branches before pushing When a human closes a code agent PR and re-triggers /fs-code, the old remote branch persists. The post-code script's plain git push fails with non-fast-forward because the new local branch diverges from the stale remote. This caused silent failures requiring manual intervention (e.g., PR #1234 needed 3 attempts). Changes to post-code.sh: - Section 7a: before pushing, check if the remote branch exists via git ls-remote. If it does and no open PR references it, delete the stale remote branch so the fresh push succeeds. - Section 7b: capture push exit code and output. On non-fast-forward errors, retry with --force-with-lease as a fallback safety net. - Error reporting: add an ERR trap that posts a comment on the originating issue when the post-code script fails, including the exit code and a link to the workflow run. This ensures humans get feedback without checking workflow logs. - Move GH_TOKEN export earlier (before section 7a) since both the stale branch check and error reporting need it. Changes to post-code-test.sh: - Add stale branch decision tests (no remote, stale with no PR, branch with open PR). - Add push retry decision tests (success, non-fast-forward, rejected, unexpected error). - Add error comment content tests (exit code, workflow link, retry hint, warning emoji). Note: make lint could not run due to a Go toolchain permission error in the sandbox (unrelated to these shell script changes). The post-code-test.sh suite passes all 38 tests. Closes #1301 Signed-off-by: fullsend-code --- .../fullsend-repo/scripts/post-code-test.sh | 194 ++++++++++++++++++ .../fullsend-repo/scripts/post-code.sh | 76 ++++++- 2 files changed, 265 insertions(+), 5 deletions(-) diff --git a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh index 9e0b496e94..f665e04465 100644 --- a/internal/scaffold/fullsend-repo/scripts/post-code-test.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code-test.sh @@ -324,6 +324,200 @@ run_noop_test "proceed-feature-branch-with-changes" \ run_noop_test "noop-on-main-with-changes" \ "main" "src/widget.go" "noop:branch" +# --------------------------------------------------------------------------- +# Test helper — reimplements the stale branch cleanup decision logic from +# post-code.sh section 7a. Given whether a remote branch exists and whether +# an open PR references it, returns the action the script would take. +# --------------------------------------------------------------------------- +decide_stale_branch_action() { + local remote_ref="$1" # non-empty if remote branch exists + local open_pr_num="$2" # non-empty if an open PR uses the branch + + if [ -z "${remote_ref}" ]; then + echo "skip:no-remote-branch" + return 0 + fi + + if [ -z "${open_pr_num}" ]; then + echo "delete:stale-branch" + return 0 + fi + + echo "keep:open-pr:${open_pr_num}" + return 0 +} + +run_stale_branch_test() { + local test_name="$1" + local remote_ref="$2" + local open_pr_num="$3" + local expected_prefix="$4" + + local actual + actual="$(decide_stale_branch_action "${remote_ref}" "${open_pr_num}")" + + if [[ "${actual}" != ${expected_prefix}* ]]; then + echo "FAIL: ${test_name}" + echo " remote_ref: '${remote_ref}'" + echo " open_pr_num: '${open_pr_num}'" + echo " expected prefix: '${expected_prefix}'" + echo " actual: '${actual}'" + FAILURES=$((FAILURES + 1)) + return + fi + + echo "PASS: ${test_name}" +} + +# --- Stale branch cleanup test cases --- + +# No remote branch → skip (normal first push) +run_stale_branch_test "no-remote-branch" \ + "" "" "skip:no-remote-branch" + +# Remote branch exists, no open PR → delete stale branch +run_stale_branch_test "stale-branch-no-pr" \ + "abc123 refs/heads/agent/42-fix-widget" "" "delete:stale-branch" + +# Remote branch exists, open PR → keep branch (push will update PR) +run_stale_branch_test "branch-with-open-pr" \ + "abc123 refs/heads/agent/42-fix-widget" "99" "keep:open-pr" + +# --------------------------------------------------------------------------- +# Test helper — reimplements the push retry logic from post-code.sh +# section 7b. Given a push exit code and output, returns the action. +# --------------------------------------------------------------------------- +decide_push_retry() { + local push_rc="$1" + local push_output="$2" + + if [ "${push_rc}" -eq 0 ]; then + echo "success" + return 0 + fi + + if echo "${push_output}" | grep -qi "non-fast-forward\|rejected\|fetch first"; then + echo "retry:force-with-lease" + return 0 + fi + + echo "fail:unexpected-error" + return 0 +} + +run_push_retry_test() { + local test_name="$1" + local push_rc="$2" + local push_output="$3" + local expected_prefix="$4" + + local actual + actual="$(decide_push_retry "${push_rc}" "${push_output}")" + + if [[ "${actual}" != ${expected_prefix}* ]]; then + echo "FAIL: ${test_name}" + echo " push_rc: '${push_rc}'" + echo " push_output: '${push_output}'" + echo " expected prefix: '${expected_prefix}'" + echo " actual: '${actual}'" + FAILURES=$((FAILURES + 1)) + return + fi + + echo "PASS: ${test_name}" +} + +# --- Push retry test cases --- + +# Successful push → no retry needed +run_push_retry_test "push-success" \ + "0" "Everything up-to-date" "success" + +# Non-fast-forward error → retry with --force-with-lease +run_push_retry_test "push-non-fast-forward" \ + "1" "error: failed to push some refs: non-fast-forward" "retry:force-with-lease" + +# Rejected error → retry with --force-with-lease +run_push_retry_test "push-rejected" \ + "1" "! [rejected] agent/42 -> agent/42 (fetch first)" "retry:force-with-lease" + +# Unknown error → fail +run_push_retry_test "push-unexpected-error" \ + "1" "fatal: repository not found" "fail:unexpected-error" + +# --------------------------------------------------------------------------- +# Test helper — reimplements the error reporting comment builder from +# post-code.sh. Verifies the comment body contains expected content. +# --------------------------------------------------------------------------- +build_error_comment() { + local exit_code="$1" + local repo_full_name="$2" + local run_id="$3" + + local run_url="https://github.com/${repo_full_name}/actions/runs/${run_id}" + echo "⚠️ **Post-code script failed** (exit code ${exit_code}) + +The code agent completed, but the post-code script failed while \ +pushing the branch or creating the PR. + +**Workflow run:** ${run_url} + +Please check the workflow logs for details and retry with \`/fs-code\` \ +if appropriate." +} + +run_error_comment_test() { + local test_name="$1" + local exit_code="$2" + local repo="$3" + local run_id="$4" + local check_pattern="$5" + local expect_present="$6" + + local actual + actual="$(build_error_comment "${exit_code}" "${repo}" "${run_id}")" + + if [ "${expect_present}" = "yes" ]; then + if ! echo "${actual}" | grep -qF "${check_pattern}"; then + echo "FAIL: ${test_name}" + echo " expected to find: '${check_pattern}'" + echo " in body:" + echo "${actual}" | sed 's/^/ /' + FAILURES=$((FAILURES + 1)) + return + fi + else + if echo "${actual}" | grep -qF "${check_pattern}"; then + echo "FAIL: ${test_name}" + echo " expected NOT to find: '${check_pattern}'" + echo " in body:" + echo "${actual}" | sed 's/^/ /' + FAILURES=$((FAILURES + 1)) + return + fi + fi + + echo "PASS: ${test_name}" +} + +# --- Error comment test cases --- + +run_error_comment_test "error-comment-has-exit-code" \ + "1" "my-org/my-repo" "12345" \ + "exit code 1" "yes" + +run_error_comment_test "error-comment-has-workflow-link" \ + "1" "my-org/my-repo" "12345" \ + "https://github.com/my-org/my-repo/actions/runs/12345" "yes" + +run_error_comment_test "error-comment-has-retry-hint" \ + "1" "my-org/my-repo" "12345" \ + "/fs-code" "yes" + +run_error_comment_test "error-comment-has-warning-emoji" \ + "1" "my-org/my-repo" "12345" \ + "⚠️" "yes" + # --- Summary --- echo "" diff --git a/internal/scaffold/fullsend-repo/scripts/post-code.sh b/internal/scaffold/fullsend-repo/scripts/post-code.sh index bafaf21c7b..62b7110482 100755 --- a/internal/scaffold/fullsend-repo/scripts/post-code.sh +++ b/internal/scaffold/fullsend-repo/scripts/post-code.sh @@ -61,6 +61,38 @@ TARGET_BRANCH="${TARGET_BRANCH:-main}" echo "::add-mask::${PUSH_TOKEN}" +# --------------------------------------------------------------------------- +# Error reporting — post a comment on the issue when the post-script fails. +# +# This ensures humans get feedback without checking workflow logs. The +# function is called from a trap on ERR. It is a best-effort operation: +# if the comment fails (e.g. token expired), we still exit non-zero. +# --------------------------------------------------------------------------- +report_failure_to_issue() { + local exit_code=$? + # Only report if we have the necessary context + if [ -z "${GH_TOKEN:-}" ]; then + export GH_TOKEN="${PUSH_TOKEN}" + fi + local run_url="${GITHUB_SERVER_URL:-https://github.com}/${REPO_FULL_NAME}/actions/runs/${GITHUB_RUN_ID:-unknown}" + local comment_body="⚠️ **Post-code script failed** (exit code ${exit_code}) + +The code agent completed, but the post-code script failed while \ +pushing the branch or creating the PR. + +**Workflow run:** ${run_url} + +Please check the workflow logs for details and retry with \`/fs-code\` \ +if appropriate." + + echo "::warning::Posting failure comment to issue #${ISSUE_NUMBER}..." + gh issue comment "${ISSUE_NUMBER}" \ + --repo "${REPO_FULL_NAME}" \ + --body "${comment_body}" 2>/dev/null || \ + echo "::warning::Failed to post error comment to issue #${ISSUE_NUMBER}" +} +trap report_failure_to_issue ERR + # --------------------------------------------------------------------------- # 1. Verify feature branch # --------------------------------------------------------------------------- @@ -196,16 +228,50 @@ fi git remote set-url origin \ "https://x-access-token:${PUSH_TOKEN}@github.com/${REPO_FULL_NAME}.git" -# Plain push (no --force-with-lease). Agents always create new -# commits (amend is in disallowedTools), so force-push is unnecessary -# and plain push is safer (refuses diverged branches). +export GH_TOKEN="${PUSH_TOKEN}" + +# --------------------------------------------------------------------------- +# 7a. Delete stale remote branch if it exists with no open PR. +# +# When a human closes a code agent PR and re-triggers /fs-code, the old +# remote branch still exists. A plain push will fail with non-fast-forward +# because the local branch was created fresh from origin/main. Delete the +# stale remote branch so the push succeeds. +# --------------------------------------------------------------------------- +REMOTE_REF="$(git ls-remote --heads origin "${BRANCH}" 2>/dev/null | head -1 || true)" +if [ -n "${REMOTE_REF}" ]; then + echo "Remote branch ${BRANCH} already exists — checking for open PRs..." + OPEN_PR="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${BRANCH}" \ + --state open --json number --jq '.[0].number' 2>/dev/null || true)" + if [ -z "${OPEN_PR}" ]; then + echo "No open PR uses ${BRANCH} — deleting stale remote branch" + git push origin --delete "${BRANCH}" 2>&1 || \ + echo "::warning::Failed to delete stale remote branch ${BRANCH}" + else + echo "Open PR #${OPEN_PR} uses ${BRANCH} — keeping remote branch" + fi +fi + +# --------------------------------------------------------------------------- +# 7b. Push, with --force-with-lease fallback for non-fast-forward errors. +# --------------------------------------------------------------------------- echo "Pushing branch ${BRANCH}..." -git push -u origin -- "${BRANCH}" 2>&1 +PUSH_OUTPUT="$(git push -u origin -- "${BRANCH}" 2>&1)" && PUSH_RC=0 || PUSH_RC=$? +echo "${PUSH_OUTPUT}" + +if [ "${PUSH_RC}" -ne 0 ]; then + if echo "${PUSH_OUTPUT}" | grep -qi "non-fast-forward\|rejected\|fetch first"; then + echo "::warning::Plain push failed (non-fast-forward) — retrying with --force-with-lease" + git push --force-with-lease -u origin -- "${BRANCH}" 2>&1 + else + echo "::error::Push failed with unexpected error" + exit 1 + fi +fi # --------------------------------------------------------------------------- # 8. Create PR # --------------------------------------------------------------------------- -export GH_TOKEN="${PUSH_TOKEN}" EXISTING_PR_NUM="$(gh pr list --repo "${REPO_FULL_NAME}" --head "${BRANCH}" \ --json number --jq '.[0].number' 2>/dev/null || true)"