From 6f30614dda1f1baf906f696a984225fa7255e633 Mon Sep 17 00:00:00 2001 From: openhands Date: Fri, 20 Mar 2026 07:37:32 +0000 Subject: [PATCH] =?UTF-8?q?fix:=20fix:=20guard=20blocks=20merge=20injectio?= =?UTF-8?q?n=20=E2=80=94=20Claude=20closes=20issue=20without=20merging=20(?= =?UTF-8?q?#344)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) --- dev/dev-poll.sh | 66 ++++++++++++++++++++++++++++++-- dev/phase-handler.sh | 28 +++----------- lib/hooks/on-pretooluse-guard.sh | 9 +++++ 3 files changed, 76 insertions(+), 27 deletions(-) diff --git a/dev/dev-poll.sh b/dev/dev-poll.sh index 120403e..5cfa004 100755 --- a/dev/dev-poll.sh +++ b/dev/dev-poll.sh @@ -150,6 +150,52 @@ handle_ci_exhaustion() { return 0 } +# ============================================================================= +# HELPER: merge an approved PR directly (no Claude needed) +# +# Merging an approved, CI-green PR is a single API call. Spawning dev-agent +# for this fails when the issue is already closed (Codeberg auto-closes issues +# on PR creation when body contains "Fixes #N"), causing a respawn loop (#344). +# ============================================================================= +try_direct_merge() { + local pr_num="$1" issue_num="$2" + + log "PR #${pr_num} (issue #${issue_num}) approved + CI green → attempting direct merge" + + local merge_resp merge_http + merge_resp=$(curl -sf -w '\n%{http_code}' -X POST \ + -H "Authorization: token ${CODEBERG_TOKEN}" \ + -H 'Content-Type: application/json' \ + "${API}/pulls/${pr_num}/merge" \ + -d '{"Do":"merge","delete_branch_after_merge":true}' 2>/dev/null) || true + + merge_http=$(echo "$merge_resp" | tail -1) + + if [ "${merge_http:-0}" = "200" ] || [ "${merge_http:-0}" = "204" ]; then + log "PR #${pr_num} merged successfully" + # Close the issue (may already be closed by Codeberg auto-close) + curl -sf -X PATCH \ + -H "Authorization: token ${CODEBERG_TOKEN}" \ + -H 'Content-Type: application/json' \ + "${API}/issues/${issue_num}" \ + -d '{"state":"closed"}' >/dev/null 2>&1 || true + # Remove in-progress label + curl -sf -X DELETE \ + -H "Authorization: token ${CODEBERG_TOKEN}" \ + "${API}/issues/${issue_num}/labels/in-progress" >/dev/null 2>&1 || true + # Clean up CI fix tracker + ci_fix_reset "$pr_num" + # Clean up phase/session artifacts + rm -f "/tmp/dev-session-${PROJECT_NAME}-${issue_num}.phase" \ + "/tmp/dev-impl-summary-${PROJECT_NAME}-${issue_num}.txt" + matrix_send "dev" "✅ PR #${pr_num} (issue #${issue_num}) merged directly by dev-poll" 2>/dev/null || true + return 0 + fi + + log "PR #${pr_num} direct merge failed (HTTP ${merge_http:-?}) — falling back to dev-agent" + return 1 +} + # shellcheck disable=SC2034 REPO="${CODEBERG_REPO}" @@ -284,7 +330,11 @@ if [ "$ORPHAN_COUNT" -gt 0 ]; then jq -r '[.[] | select(.state == "REQUEST_CHANGES")] | length') || true if ci_passed "$CI_STATE" && [ "${HAS_APPROVE:-0}" -gt 0 ]; then - log "PR #${HAS_PR} approved + CI green → spawning dev-agent to merge" + if try_direct_merge "$HAS_PR" "$ISSUE_NUM"; then + exit 0 + fi + # Direct merge failed (conflicts?) — fall back to dev-agent + log "falling back to dev-agent for PR #${HAS_PR} merge" nohup "${SCRIPT_DIR}/dev-agent.sh" "$ISSUE_NUM" >> "$LOGFILE" 2>&1 & log "started dev-agent PID $! for issue #${ISSUE_NUM} (agent-merge)" exit 0 @@ -363,9 +413,13 @@ for i in $(seq 0 $(($(echo "$OPEN_PRS" | jq 'length') - 1))); do HAS_APPROVE=$(echo "$REVIEWS_JSON" | \ jq -r '[.[] | select(.state == "APPROVED") | select(.stale == false)] | length') || true - # Spawn agent to merge if approved + CI green + # Merge directly if approved + CI green (no Claude needed — single API call) if ci_passed "$CI_STATE" && [ "${HAS_APPROVE:-0}" -gt 0 ]; then - log "PR #${PR_NUM} (issue #${STUCK_ISSUE}) approved + CI green → spawning dev-agent to merge" + if try_direct_merge "$PR_NUM" "$STUCK_ISSUE"; then + exit 0 + fi + # Direct merge failed (conflicts?) — fall back to dev-agent + log "falling back to dev-agent for PR #${PR_NUM} merge" nohup "${SCRIPT_DIR}/dev-agent.sh" "$STUCK_ISSUE" >> "$LOGFILE" 2>&1 & log "started dev-agent PID $! for stuck PR #${PR_NUM} (agent-merge)" exit 0 @@ -453,7 +507,11 @@ for i in $(seq 0 $((BACKLOG_COUNT - 1))); do jq -r '[.[] | select(.state == "REQUEST_CHANGES")] | length') || true if ci_passed "$CI_STATE" && [ "${HAS_APPROVE:-0}" -gt 0 ]; then - log "#${ISSUE_NUM} PR #${EXISTING_PR} approved + CI green → spawning dev-agent to merge" + if try_direct_merge "$EXISTING_PR" "$ISSUE_NUM"; then + exit 0 + fi + # Direct merge failed (conflicts?) — fall back to dev-agent + log "falling back to dev-agent for PR #${EXISTING_PR} merge" nohup "${SCRIPT_DIR}/dev-agent.sh" "$ISSUE_NUM" >> "$LOGFILE" 2>&1 & log "started dev-agent PID $! for issue #${ISSUE_NUM} (agent-merge)" exit 0 diff --git a/dev/phase-handler.sh b/dev/phase-handler.sh index 1e4ef78..5c329d5 100644 --- a/dev/phase-handler.sh +++ b/dev/phase-handler.sh @@ -392,33 +392,15 @@ Instructions: printf 'PHASE:done\n' > "$PHASE_FILE" elif [ "$_merge_rc" -ne 2 ]; then # Other merge failure (conflict, etc.) — delegate to Claude for rebase + retry - agent_inject_into_session "$SESSION_NAME" "Approved! PR #${PR_NUMBER} has been approved. + agent_inject_into_session "$SESSION_NAME" "Approved! PR #${PR_NUMBER} has been approved, but the merge failed (likely conflicts). -Merge the PR and close the issue directly — do NOT wait for the orchestrator: - - # Merge the PR: - curl -sf -X POST \\ - -H \"Authorization: token \${CODEBERG_TOKEN}\" \\ - -H 'Content-Type: application/json' \\ - \"${API}/pulls/${PR_NUMBER}/merge\" \\ - -d '{\"Do\":\"merge\",\"delete_branch_after_merge\":true}' - - # Close the issue: - curl -sf -X PATCH \\ - -H \"Authorization: token \${CODEBERG_TOKEN}\" \\ - -H 'Content-Type: application/json' \\ - \"${API}/issues/${ISSUE}\" \\ - -d '{\"state\":\"closed\"}' - -If merge fails due to conflicts, rebase first: +Rebase onto ${PRIMARY_BRANCH} and push: git fetch origin ${PRIMARY_BRANCH} && git rebase origin/${PRIMARY_BRANCH} git push --force-with-lease origin ${BRANCH} - # Then retry the merge curl above. + echo \"PHASE:awaiting_ci\" > \"${PHASE_FILE}\" -After a successful merge write PHASE:done: - echo \"PHASE:done\" > \"${PHASE_FILE}\" - -If merge repeatedly fails, write PHASE:needs_human with a reason." +Do NOT merge or close the issue — the orchestrator handles that after CI passes. +If rebase repeatedly fails, write PHASE:needs_human with a reason." fi # _merge_rc=2: PHASE:needs_human already written by do_merge() break diff --git a/lib/hooks/on-pretooluse-guard.sh b/lib/hooks/on-pretooluse-guard.sh index 58ff61a..c159223 100755 --- a/lib/hooks/on-pretooluse-guard.sh +++ b/lib/hooks/on-pretooluse-guard.sh @@ -6,6 +6,7 @@ # - git push --force / -f to primary branch # - rm -rf targeting paths outside the worktree # - Direct Codeberg API merge calls (should go through phase protocol) +# - Direct issue close calls (should go through phase protocol) # - git checkout / git switch to primary branch (stay on feature branch) # # Usage (in .claude/settings.json): @@ -68,6 +69,14 @@ if printf '%s' "$command_str" | grep -qE '/pulls/[0-9]+/merge'; then exit 2 fi +# --- Guard 5: Direct issue close calls --- +# Defense in depth: Claude should never close issues directly — the orchestrator +# closes issues after a successful merge via the phase protocol. +if printf '%s' "$command_str" | grep -qE '/issues/[0-9]+.*state.*closed'; then + printf 'BLOCKED: Closing issues must go through the phase protocol. Write PHASE:done — the orchestrator closes issues after merge.\n' + exit 2 +fi + # --- Guard 4: checkout/switch to primary branch --- # Blocks: git checkout main, git switch main, git switch --detach main, etc. # Allows: git checkout -b branch main, git checkout -- file