From 63e60de9d6e124cb6e9a01924814aaf57dd900b2 Mon Sep 17 00:00:00 2001 From: openhands Date: Tue, 17 Mar 2026 22:59:05 +0000 Subject: [PATCH] fix: address round 2 review findings from issue #81 - Move atomic mv inside gardener loop so reply is only claimed when a matching needs_human session exists (fixes reply-loss regression) - Delay rm of claimed file until after successful injection in both supervisor and gardener (OOM/SIGKILL leaves file recoverable) - Fix matrix_listener ack message: 'next poll' instead of 'next supervisor poll' Co-Authored-By: Claude Opus 4.6 --- gardener/gardener-poll.sh | 56 +++++++++++++++++------------------ lib/matrix_listener.sh | 2 +- supervisor/supervisor-poll.sh | 3 +- 3 files changed, 30 insertions(+), 31 deletions(-) diff --git a/gardener/gardener-poll.sh b/gardener/gardener-poll.sh index 896d77a..c87d57b 100755 --- a/gardener/gardener-poll.sh +++ b/gardener/gardener-poll.sh @@ -66,23 +66,24 @@ fi # ── Inject human replies into needs_human dev sessions (backup to supervisor) ─ HUMAN_REPLY_FILE="/tmp/dev-escalation-reply" -_gr_claimed="/tmp/dev-escalation-reply.gardener.$$" -if [ -s "$HUMAN_REPLY_FILE" ] && mv "$HUMAN_REPLY_FILE" "$_gr_claimed" 2>/dev/null; then +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) + [ "$_gr_phase" = "PHASE:needs_human" ] || continue + + _gr_issue=$(basename "$_gr_phase_file" .phase) + _gr_issue="${_gr_issue#dev-session-${PROJECT_NAME}-}" + [ -z "$_gr_issue" ] && continue + _gr_session="dev-${PROJECT_NAME}-${_gr_issue}" + + tmux has-session -t "$_gr_session" 2>/dev/null || continue + + # Atomic claim — only take the file once we know a session needs it + _gr_claimed="/tmp/dev-escalation-reply.gardener.$$" + [ -s "$HUMAN_REPLY_FILE" ] && mv "$HUMAN_REPLY_FILE" "$_gr_claimed" 2>/dev/null || continue _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) - [ "$_gr_phase" = "PHASE:needs_human" ] || continue - _gr_issue=$(basename "$_gr_phase_file" .phase) - _gr_issue="${_gr_issue#dev-session-${PROJECT_NAME}-}" - [ -z "$_gr_issue" ] && continue - _gr_session="dev-${PROJECT_NAME}-${_gr_issue}" - - tmux has-session -t "$_gr_session" 2>/dev/null || continue - - _gr_inject_msg="Human reply received for issue #${_gr_issue}: + _gr_inject_msg="Human reply received for issue #${_gr_issue}: ${_gr_reply} @@ -91,20 +92,19 @@ Instructions: 2. Continue your work based on their input. 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" - tmux load-buffer -b "human-inject-${_gr_issue}" "$_gr_tmpfile" || true - tmux paste-buffer -t "$_gr_session" -b "human-inject-${_gr_issue}" || true - sleep 0.5 - tmux send-keys -t "$_gr_session" "" Enter || true - tmux delete-buffer -b "human-inject-${_gr_issue}" 2>/dev/null || true - rm -f "$_gr_tmpfile" + _gr_tmpfile=$(mktemp /tmp/human-inject-XXXXXX) + printf '%s' "$_gr_inject_msg" > "$_gr_tmpfile" + tmux load-buffer -b "human-inject-${_gr_issue}" "$_gr_tmpfile" || true + tmux paste-buffer -t "$_gr_session" -b "human-inject-${_gr_issue}" || true + sleep 0.5 + tmux send-keys -t "$_gr_session" "" Enter || true + tmux delete-buffer -b "human-inject-${_gr_issue}" 2>/dev/null || true + rm -f "$_gr_tmpfile" "$_gr_claimed" - 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 -fi + 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 # ── Fetch all open issues ───────────────────────────────────────────────── ISSUES_JSON=$(codeberg_api GET "/issues?state=open&type=issues&limit=50&sort=updated&direction=desc" 2>/dev/null || true) diff --git a/lib/matrix_listener.sh b/lib/matrix_listener.sh index a87e57c..d083243 100755 --- a/lib/matrix_listener.sh +++ b/lib/matrix_listener.sh @@ -143,7 +143,7 @@ while true; do ;; dev) printf '%s\t%s\t%s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$SENDER" "$BODY" >> /tmp/dev-escalation-reply - matrix_send "dev" "✓ received, will inject on next supervisor poll" "$THREAD_ROOT" >/dev/null 2>&1 || true + matrix_send "dev" "✓ received, will inject on next poll" "$THREAD_ROOT" >/dev/null 2>&1 || true ;; vault) # Parse APPROVE or REJECT from reply diff --git a/supervisor/supervisor-poll.sh b/supervisor/supervisor-poll.sh index 124fcef..0b0ba9b 100755 --- a/supervisor/supervisor-poll.sh +++ b/supervisor/supervisor-poll.sh @@ -540,7 +540,6 @@ check_project() { _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} @@ -558,7 +557,7 @@ Instructions: sleep 0.5 tmux send-keys -t "$_nh_session" "" Enter || true tmux delete-buffer -b "human-inject-${_nh_issue}" 2>/dev/null || true - rm -f "$_nh_tmpfile" + rm -f "$_nh_tmpfile" "$_nh_claimed" rm -f "/tmp/dev-renotify-${proj_name}-${_nh_issue}" flog "${proj_name}: #${_nh_issue} human reply injected into session ${_nh_session}"