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>
This commit is contained in:
openhands
parent
85d05cdee2
commit
bf4c70086e
3 changed files with 40 additions and 21 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue