From bfe0c09b5c4f6dbcaefce7c3e874daeb2af03418 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 17 Mar 2026 22:40:54 +0000 Subject: [PATCH] fix: address review findings from issue #81 - Fix dev-agent.sh comment: gardener-poll.sh is the backup injector, not review-poll.sh - Add renotify marker cleanup to gardener injection path - Use atomic mv to claim reply file, preventing double-injection race between supervisor and gardener - Add break after supervisor injection for symmetry with gardener - Remove overly prescriptive PHASE:awaiting_ci hardcode from injection instructions Co-Authored-By: Claude Opus 4.6 --- dev/dev-agent.sh | 2 +- gardener/gardener-poll.sh | 11 ++++++----- supervisor/supervisor-poll.sh | 13 +++++++------ 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/dev/dev-agent.sh b/dev/dev-agent.sh index b1208f9..dffe7e9 100755 --- a/dev/dev-agent.sh +++ b/dev/dev-agent.sh @@ -1272,7 +1272,7 @@ Instructions: HUMAN_REASON=$(sed -n '2p' "$PHASE_FILE" 2>/dev/null | sed 's/^Reason: //' || echo "") notify "⚠️ Issue #${ISSUE} (PR #${PR_NUMBER:-none}) needs human input.${HUMAN_REASON:+ Reason: ${HUMAN_REASON}}" log "phase: needs_human — notified via Matrix, waiting for external injection" - # Don't inject anything — supervisor-poll.sh (#81) and review-poll.sh (#82) inject replies + # Don't inject anything — supervisor-poll.sh (#81) injects human replies, gardener-poll.sh as backup # ── PHASE: done ───────────────────────────────────────────────────────────── elif [ "$CURRENT_PHASE" = "PHASE:done" ]; then diff --git a/gardener/gardener-poll.sh b/gardener/gardener-poll.sh index 3dd58a6..896d77a 100755 --- a/gardener/gardener-poll.sh +++ b/gardener/gardener-poll.sh @@ -66,8 +66,10 @@ fi # ── Inject human replies into needs_human dev sessions (backup to supervisor) ─ HUMAN_REPLY_FILE="/tmp/dev-escalation-reply" -if [ -s "$HUMAN_REPLY_FILE" ]; then - _gr_reply=$(cat "$HUMAN_REPLY_FILE") +_gr_claimed="/tmp/dev-escalation-reply.gardener.$$" +if [ -s "$HUMAN_REPLY_FILE" ] && mv "$HUMAN_REPLY_FILE" "$_gr_claimed" 2>/dev/null; then + _gr_reply=$(cat "$_gr_claimed") + rm -f "$_gr_claimed" for _gr_phase_file in /tmp/dev-session-"${PROJECT_NAME}"-*.phase; do [ -f "$_gr_phase_file" ] || continue _gr_phase=$(head -1 "$_gr_phase_file" 2>/dev/null | tr -d '[:space:]' || true) @@ -87,8 +89,7 @@ ${_gr_reply} Instructions: 1. Read the human's guidance carefully. 2. Continue your work based on their input. -3. When done, push your changes and write the appropriate phase: - echo \"PHASE:awaiting_ci\" > \"${_gr_phase_file}\"" +3. When done, push your changes and write the appropriate phase." _gr_tmpfile=$(mktemp /tmp/human-inject-XXXXXX) printf '%s' "$_gr_inject_msg" > "$_gr_tmpfile" @@ -99,7 +100,7 @@ Instructions: tmux delete-buffer -b "human-inject-${_gr_issue}" 2>/dev/null || true rm -f "$_gr_tmpfile" - rm -f "$HUMAN_REPLY_FILE" + rm -f "/tmp/dev-renotify-${PROJECT_NAME}-${_gr_issue}" log "${PROJECT_NAME}: #${_gr_issue} human reply injected into session ${_gr_session} (gardener)" break # only one reply to deliver done diff --git a/supervisor/supervisor-poll.sh b/supervisor/supervisor-poll.sh index b1615f0..124fcef 100755 --- a/supervisor/supervisor-poll.sh +++ b/supervisor/supervisor-poll.sh @@ -536,9 +536,11 @@ check_project() { continue fi - # Inject human reply if available - if [ -s "$HUMAN_REPLY_FILE" ]; then - _nh_reply=$(cat "$HUMAN_REPLY_FILE") + # Inject human reply if available (atomic mv to prevent double-injection with gardener) + _nh_claimed="/tmp/dev-escalation-reply.supervisor.$$" + if [ -s "$HUMAN_REPLY_FILE" ] && mv "$HUMAN_REPLY_FILE" "$_nh_claimed" 2>/dev/null; then + _nh_reply=$(cat "$_nh_claimed") + rm -f "$_nh_claimed" _nh_inject_msg="Human reply received for issue #${_nh_issue}: ${_nh_reply} @@ -546,8 +548,7 @@ ${_nh_reply} Instructions: 1. Read the human's guidance carefully. 2. Continue your work based on their input. -3. When done, push your changes and write the appropriate phase: - echo \"PHASE:awaiting_ci\" > \"${_nh_phase_file}\"" +3. When done, push your changes and write the appropriate phase." _nh_tmpfile=$(mktemp /tmp/human-inject-XXXXXX) printf '%s' "$_nh_inject_msg" > "$_nh_tmpfile" @@ -559,10 +560,10 @@ Instructions: tmux delete-buffer -b "human-inject-${_nh_issue}" 2>/dev/null || true rm -f "$_nh_tmpfile" - rm -f "$HUMAN_REPLY_FILE" rm -f "/tmp/dev-renotify-${proj_name}-${_nh_issue}" flog "${proj_name}: #${_nh_issue} human reply injected into session ${_nh_session}" fixed "${proj_name}: Injected human reply into dev session #${_nh_issue}" + break # one reply to deliver else # No reply yet — check for timeout (re-notify at 6h, alert at 24h) _nh_mtime=$(stat -c %Y "$_nh_phase_file" 2>/dev/null || echo 0)