From d59c09eb5b276f5fb48ab520efc0ee971c4a480c Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 17 Mar 2026 20:40:35 +0000 Subject: [PATCH] fix: address review findings from issue #80 phase protocol - Add missing MAX_CI_FIXES=3 and MAX_REVIEW_ROUNDS=5 constants to the config section; referencing undefined variables with set -euo pipefail caused an abort on first CI failure or REQUEST_CHANGES review. - cleanup() trap now calls kill_tmux_session() so any unexpected exit (SIGTERM, errexit, unbound variable) kills the Claude session rather than leaving it running autonomously without an orchestrator. - do_merge() initial CI wait loop now breaks and returns 1 immediately on failure/error states, avoiding a full 10-minute poll before a merge attempt that would also fail. - Inner review-poll loop no longer updates LAST_PHASE_MTIME when it detects a mid-wait phase-file change; leaving it stale ensures the outer loop detects and dispatches the new phase on its next tick (previously the phase was silently swallowed). - post_refusal_comment dedup now fetches the last 5 comments and checks any of them, so a human reply between two agent runs no longer causes a duplicate refusal comment. - Remove duplicate DELETE labels/backlog call in claim section. Co-Authored-By: Claude Sonnet 4.6 --- dev/dev-agent.sh | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/dev/dev-agent.sh b/dev/dev-agent.sh index b166b97..b22264b 100755 --- a/dev/dev-agent.sh +++ b/dev/dev-agent.sh @@ -50,6 +50,10 @@ IDLE_TIMEOUT=7200 # 2h: kill session if phase stale this long CI_POLL_TIMEOUT=1800 # 30min max for CI to complete REVIEW_POLL_TIMEOUT=10800 # 3h max wait for review +# Limits +MAX_CI_FIXES=3 +MAX_REVIEW_ROUNDS=5 + # Counters — global state across phase transitions CI_RETRY_COUNT=0 CI_FIX_COUNT=0 @@ -97,8 +101,8 @@ post_refusal_comment() { local emoji="$1" title="$2" body="$3" local last_has_title last_has_title=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" \ - "${API}/issues/${ISSUE}/comments?limit=1" | \ - jq -r --arg t "Dev-agent: ${title}" '.[0].body // "" | contains($t)') || true + "${API}/issues/${ISSUE}/comments?limit=5" | \ + jq -r --arg t "Dev-agent: ${title}" '[.[] | .body // ""] | any(contains($t)) | tostring') || true if [ "$last_has_title" = "true" ]; then log "skipping duplicate refusal comment: ${title}" return 0 @@ -139,6 +143,8 @@ cleanup_labels() { CLAIMED=false cleanup() { rm -f "$LOCKFILE" "$STATUSFILE" + # Kill any live session so Claude doesn't run without an orchestrator attached + kill_tmux_session # If we claimed the issue but never created a PR, unclaim it if [ "$CLAIMED" = true ] && [ -z "${PR_NUMBER:-}" ]; then log "cleanup: unclaiming issue (no PR created)" @@ -166,6 +172,11 @@ do_merge() { ci=$(curl -sf -H "Authorization: token ${CODEBERG_TOKEN}" \ "${API}/commits/${sha}/status" | jq -r '.state // "unknown"') [ "$ci" = "success" ] && break + if [ "$ci" = "failure" ] || [ "$ci" = "error" ]; then + log "CI is red before merge attempt — aborting" + notify "PR #${pr} CI is failing; cannot merge." + return 1 + fi sleep 30 done @@ -476,10 +487,6 @@ curl -sf -X DELETE \ -H "Authorization: token ${CODEBERG_TOKEN}" \ "${API}/issues/${ISSUE}/labels/backlog" >/dev/null 2>&1 || true -curl -sf -X DELETE \ - -H "Authorization: token ${CODEBERG_TOKEN}" \ - "${API}/issues/${ISSUE}/labels/backlog" >/dev/null 2>&1 || true - CLAIMED=true # ============================================================================= @@ -1138,7 +1145,8 @@ Instructions: NEW_MTIME=$(stat -c %Y "$PHASE_FILE" 2>/dev/null || echo 0) if [ "$NEW_MTIME" -gt "$LAST_PHASE_MTIME" ]; then log "phase file updated during review wait — re-entering main loop" - LAST_PHASE_MTIME="$NEW_MTIME" + # Do NOT update LAST_PHASE_MTIME here — leave it stale so the outer + # loop detects the change on its next tick and dispatches the new phase. REVIEW_FOUND=true # Prevent timeout injection break fi