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 <noreply@anthropic.com>
This commit is contained in:
openhands 2026-03-17 22:40:54 +00:00
parent 48683e508c
commit bfe0c09b5c
3 changed files with 14 additions and 12 deletions

View file

@ -1272,7 +1272,7 @@ Instructions:
HUMAN_REASON=$(sed -n '2p' "$PHASE_FILE" 2>/dev/null | sed 's/^Reason: //' || echo "") 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}}" 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" 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 ───────────────────────────────────────────────────────────── # ── PHASE: done ─────────────────────────────────────────────────────────────
elif [ "$CURRENT_PHASE" = "PHASE:done" ]; then elif [ "$CURRENT_PHASE" = "PHASE:done" ]; then

View file

@ -66,8 +66,10 @@ fi
# ── Inject human replies into needs_human dev sessions (backup to supervisor) ─ # ── Inject human replies into needs_human dev sessions (backup to supervisor) ─
HUMAN_REPLY_FILE="/tmp/dev-escalation-reply" HUMAN_REPLY_FILE="/tmp/dev-escalation-reply"
if [ -s "$HUMAN_REPLY_FILE" ]; then _gr_claimed="/tmp/dev-escalation-reply.gardener.$$"
_gr_reply=$(cat "$HUMAN_REPLY_FILE") 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 for _gr_phase_file in /tmp/dev-session-"${PROJECT_NAME}"-*.phase; do
[ -f "$_gr_phase_file" ] || continue [ -f "$_gr_phase_file" ] || continue
_gr_phase=$(head -1 "$_gr_phase_file" 2>/dev/null | tr -d '[:space:]' || true) _gr_phase=$(head -1 "$_gr_phase_file" 2>/dev/null | tr -d '[:space:]' || true)
@ -87,8 +89,7 @@ ${_gr_reply}
Instructions: Instructions:
1. Read the human's guidance carefully. 1. Read the human's guidance carefully.
2. Continue your work based on their input. 2. Continue your work based on their input.
3. When done, push your changes and write the appropriate phase: 3. When done, push your changes and write the appropriate phase."
echo \"PHASE:awaiting_ci\" > \"${_gr_phase_file}\""
_gr_tmpfile=$(mktemp /tmp/human-inject-XXXXXX) _gr_tmpfile=$(mktemp /tmp/human-inject-XXXXXX)
printf '%s' "$_gr_inject_msg" > "$_gr_tmpfile" printf '%s' "$_gr_inject_msg" > "$_gr_tmpfile"
@ -99,7 +100,7 @@ Instructions:
tmux delete-buffer -b "human-inject-${_gr_issue}" 2>/dev/null || true tmux delete-buffer -b "human-inject-${_gr_issue}" 2>/dev/null || true
rm -f "$_gr_tmpfile" 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)" log "${PROJECT_NAME}: #${_gr_issue} human reply injected into session ${_gr_session} (gardener)"
break # only one reply to deliver break # only one reply to deliver
done done

View file

@ -536,9 +536,11 @@ check_project() {
continue continue
fi fi
# Inject human reply if available # Inject human reply if available (atomic mv to prevent double-injection with gardener)
if [ -s "$HUMAN_REPLY_FILE" ]; then _nh_claimed="/tmp/dev-escalation-reply.supervisor.$$"
_nh_reply=$(cat "$HUMAN_REPLY_FILE") 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_inject_msg="Human reply received for issue #${_nh_issue}:
${_nh_reply} ${_nh_reply}
@ -546,8 +548,7 @@ ${_nh_reply}
Instructions: Instructions:
1. Read the human's guidance carefully. 1. Read the human's guidance carefully.
2. Continue your work based on their input. 2. Continue your work based on their input.
3. When done, push your changes and write the appropriate phase: 3. When done, push your changes and write the appropriate phase."
echo \"PHASE:awaiting_ci\" > \"${_nh_phase_file}\""
_nh_tmpfile=$(mktemp /tmp/human-inject-XXXXXX) _nh_tmpfile=$(mktemp /tmp/human-inject-XXXXXX)
printf '%s' "$_nh_inject_msg" > "$_nh_tmpfile" printf '%s' "$_nh_inject_msg" > "$_nh_tmpfile"
@ -559,10 +560,10 @@ Instructions:
tmux delete-buffer -b "human-inject-${_nh_issue}" 2>/dev/null || true tmux delete-buffer -b "human-inject-${_nh_issue}" 2>/dev/null || true
rm -f "$_nh_tmpfile" rm -f "$_nh_tmpfile"
rm -f "$HUMAN_REPLY_FILE"
rm -f "/tmp/dev-renotify-${proj_name}-${_nh_issue}" rm -f "/tmp/dev-renotify-${proj_name}-${_nh_issue}"
flog "${proj_name}: #${_nh_issue} human reply injected into session ${_nh_session}" flog "${proj_name}: #${_nh_issue} human reply injected into session ${_nh_session}"
fixed "${proj_name}: Injected human reply into dev session #${_nh_issue}" fixed "${proj_name}: Injected human reply into dev session #${_nh_issue}"
break # one reply to deliver
else else
# No reply yet — check for timeout (re-notify at 6h, alert at 24h) # 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) _nh_mtime=$(stat -c %Y "$_nh_phase_file" 2>/dev/null || echo 0)