From bf4c70086eec573fbba561eb1897e1b6118e06d3 Mon Sep 17 00:00:00 2001
From: openhands <openhands@all-hands.dev>
Date: Wed, 18 Mar 2026 00:04:30 +0000
Subject: [PATCH] fix: address review findings from issue #78

- Guard inject_into_session wait_for_claude_ready with || true
- Guard all tmux calls in inject_into_session with || true
- Add worktree cleanup to idle-timeout branch in review-poll.sh
- Check phase before sleep in wait_for_review_output (no 10s delay)
- Prune review-thread-map entries during session cleanup
- Skip human question injection during active review (phase check)
- Remove no-op tmux kill-session after has-session returns false
- Add ASCII fallback for Claude prompt detection (locale safety)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 lib/matrix_listener.sh | 30 +++++++++++++++++++-----------
 review/review-poll.sh  |  7 +++++++
 review/review-pr.sh    | 24 ++++++++++++++----------
 3 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/lib/matrix_listener.sh b/lib/matrix_listener.sh
index 84b576b..9dbd35d 100755
--- a/lib/matrix_listener.sh
+++ b/lib/matrix_listener.sh
@@ -150,22 +150,30 @@ while true; do
         REVIEW_PR_NUM=$(awk -F'\t' -v id="$THREAD_ROOT" '$1 == id {print $2}' /tmp/review-thread-map 2>/dev/null || true)
         if [ -n "$REVIEW_PR_NUM" ]; then
           REVIEW_SESSION="review-${PROJECT_NAME}-${REVIEW_PR_NUM}"
+          REVIEW_PHASE_FILE="/tmp/review-session-${PROJECT_NAME}-${REVIEW_PR_NUM}.phase"
           if tmux has-session -t "$REVIEW_SESSION" 2>/dev/null; then
-            REVIEW_INJECT_MSG="Human question from ${SENDER} in Matrix:
+            # Skip injection if Claude is mid-review (phase file absent = actively writing)
+            REVIEW_CUR_PHASE=$(head -1 "$REVIEW_PHASE_FILE" 2>/dev/null | tr -d '[:space:]' || true)
+            if [ -z "$REVIEW_CUR_PHASE" ]; then
+              log "review session ${REVIEW_SESSION} is mid-review, deferring question"
+              matrix_send "review" "reviewer is busy — question queued, try again shortly" "$THREAD_ROOT" >/dev/null 2>&1 || true
+            else
+              REVIEW_INJECT_MSG="Human question from ${SENDER} in Matrix:
 
 ${BODY}
 
 Please answer this question about your review. Explain your reasoning."
-            REVIEW_INJECT_TMP=$(mktemp /tmp/review-q-inject-XXXXXX)
-            printf '%s' "$REVIEW_INJECT_MSG" > "$REVIEW_INJECT_TMP"
-            tmux load-buffer -b "review-q-${REVIEW_PR_NUM}" "$REVIEW_INJECT_TMP" || true
-            tmux paste-buffer -t "$REVIEW_SESSION" -b "review-q-${REVIEW_PR_NUM}" || true
-            sleep 0.5
-            tmux send-keys -t "$REVIEW_SESSION" "" Enter || true
-            tmux delete-buffer -b "review-q-${REVIEW_PR_NUM}" 2>/dev/null || true
-            rm -f "$REVIEW_INJECT_TMP"
-            log "review question from ${SENDER} injected into ${REVIEW_SESSION}"
-            matrix_send "review" "✓ question forwarded to reviewer session" "$THREAD_ROOT" >/dev/null 2>&1 || true
+              REVIEW_INJECT_TMP=$(mktemp /tmp/review-q-inject-XXXXXX)
+              printf '%s' "$REVIEW_INJECT_MSG" > "$REVIEW_INJECT_TMP"
+              tmux load-buffer -b "review-q-${REVIEW_PR_NUM}" "$REVIEW_INJECT_TMP" || true
+              tmux paste-buffer -t "$REVIEW_SESSION" -b "review-q-${REVIEW_PR_NUM}" || true
+              sleep 0.5
+              tmux send-keys -t "$REVIEW_SESSION" "" Enter || true
+              tmux delete-buffer -b "review-q-${REVIEW_PR_NUM}" 2>/dev/null || true
+              rm -f "$REVIEW_INJECT_TMP"
+              log "review question from ${SENDER} injected into ${REVIEW_SESSION}"
+              matrix_send "review" "✓ question forwarded to reviewer session" "$THREAD_ROOT" >/dev/null 2>&1 || true
+            fi
           else
             log "review session ${REVIEW_SESSION} not found for PR #${REVIEW_PR_NUM}"
             matrix_send "review" "review session not active for PR #${REVIEW_PR_NUM}" "$THREAD_ROOT" >/dev/null 2>&1 || true
diff --git a/review/review-poll.sh b/review/review-poll.sh
index 73e47eb..3734592 100755
--- a/review/review-poll.sh
+++ b/review/review-poll.sh
@@ -52,6 +52,8 @@ if [ -n "$REVIEW_SESSIONS" ]; then
       log "cleanup: killing session ${session} (PR #${pr_num} state=${pr_state})"
       tmux kill-session -t "$session" 2>/dev/null || true
       rm -f "$phase_file" "/tmp/${PROJECT_NAME}-review-output-${pr_num}.json"
+      # Prune review-thread-map entries for this PR
+      sed -i "/\t${pr_num}$/d" /tmp/review-thread-map 2>/dev/null || true
       cd "$REPO_ROOT"
       git worktree remove "/tmp/${PROJECT_NAME}-review-${pr_num}" --force 2>/dev/null || true
       rm -rf "/tmp/${PROJECT_NAME}-review-${pr_num}" 2>/dev/null || true
@@ -65,6 +67,11 @@ if [ -n "$REVIEW_SESSIONS" ]; then
       log "cleanup: killing session ${session} (idle > 4h)"
       tmux kill-session -t "$session" 2>/dev/null || true
       rm -f "$phase_file" "/tmp/${PROJECT_NAME}-review-output-${pr_num}.json"
+      # Prune review-thread-map entries for this PR
+      sed -i "/\t${pr_num}$/d" /tmp/review-thread-map 2>/dev/null || true
+      cd "$REPO_ROOT"
+      git worktree remove "/tmp/${PROJECT_NAME}-review-${pr_num}" --force 2>/dev/null || true
+      rm -rf "/tmp/${PROJECT_NAME}-review-${pr_num}" 2>/dev/null || true
       continue
     fi
   done <<< "$REVIEW_SESSIONS"
diff --git a/review/review-pr.sh b/review/review-pr.sh
index 955b30a..bc19d1e 100755
--- a/review/review-pr.sh
+++ b/review/review-pr.sh
@@ -97,7 +97,10 @@ wait_for_claude_ready() {
   local timeout="${1:-120}"
   local elapsed=0
   while [ "$elapsed" -lt "$timeout" ]; do
-    if tmux capture-pane -t "${SESSION_NAME}" -p 2>/dev/null | grep -q '❯'; then
+    # Check for Claude prompt: ❯ (UTF-8) or fallback to $ at line start
+    local pane_out
+    pane_out=$(tmux capture-pane -t "${SESSION_NAME}" -p 2>/dev/null || true)
+    if printf '%s' "$pane_out" | grep -qE '❯|^\$' 2>/dev/null; then
       return 0
     fi
     sleep 2
@@ -110,13 +113,16 @@ wait_for_claude_ready() {
 inject_into_session() {
   local text="$1"
   local tmpfile
-  wait_for_claude_ready 120
+  wait_for_claude_ready 120 || true
   tmpfile=$(mktemp /tmp/review-inject-XXXXXX)
   printf '%s' "$text" > "$tmpfile"
-  tmux load-buffer -b "review-inject-${PR_NUMBER}" "$tmpfile"
-  tmux paste-buffer -t "${SESSION_NAME}" -b "review-inject-${PR_NUMBER}"
+  # All tmux calls guarded with || true: the session is external and may die
+  # between the has-session check and here; a non-zero exit must not abort
+  # the script under set -euo pipefail.
+  tmux load-buffer -b "review-inject-${PR_NUMBER}" "$tmpfile" || true
+  tmux paste-buffer -t "${SESSION_NAME}" -b "review-inject-${PR_NUMBER}" || true
   sleep 0.5
-  tmux send-keys -t "${SESSION_NAME}" "" Enter
+  tmux send-keys -t "${SESSION_NAME}" "" Enter || true
   tmux delete-buffer -b "review-inject-${PR_NUMBER}" 2>/dev/null || true
   rm -f "$tmpfile"
 }
@@ -125,8 +131,7 @@ wait_for_review_output() {
   local timeout="$REVIEW_WAIT_TIMEOUT"
   local elapsed=0
   while [ "$elapsed" -lt "$timeout" ]; do
-    sleep "$REVIEW_WAIT_INTERVAL"
-    elapsed=$((elapsed + REVIEW_WAIT_INTERVAL))
+    # Check phase file before sleeping (avoids mandatory delay on fast reviews)
     if ! tmux has-session -t "${SESSION_NAME}" 2>/dev/null; then
       log "ERROR: session died during review"
       return 1
@@ -136,6 +141,8 @@ wait_for_review_output() {
     if [ "$phase" = "PHASE:review_complete" ]; then
       return 0
     fi
+    sleep "$REVIEW_WAIT_INTERVAL"
+    elapsed=$((elapsed + REVIEW_WAIT_INTERVAL))
   done
   log "ERROR: review did not complete within ${timeout}s"
   return 1
@@ -486,9 +493,6 @@ log "Prompt: ${PROMPT_SIZE} bytes (re-review: ${IS_RE_REVIEW})"
 status "preparing tmux session: ${SESSION_NAME}"
 
 if ! tmux has-session -t "${SESSION_NAME}" 2>/dev/null; then
-  # Kill any stale entry
-  tmux kill-session -t "${SESSION_NAME}" 2>/dev/null || true
-
   # Create new detached session running interactive claude in the review worktree
   tmux new-session -d -s "${SESSION_NAME}" -c "${REVIEW_WORKTREE}" \
     "claude --model sonnet --dangerously-skip-permissions"