From ab122c97013355d0ce0b3cc84a213eecfa6eaab1 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 21 Mar 2026 03:50:21 +0000 Subject: [PATCH] fix: PHASE:needs_human missing from crash-path terminal set in monitor_phase_loop (#342) Co-Authored-By: Claude Opus 4.6 (1M context) --- dev/phase-test.sh | 53 ++++++++++++++++++++++++++++++++++++ lib/agent-session.sh | 2 +- lib/hooks/on-stop-failure.sh | 2 +- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/dev/phase-test.sh b/dev/phase-test.sh index 15c59b8..2ce3309 100755 --- a/dev/phase-test.sh +++ b/dev/phase-test.sh @@ -316,6 +316,18 @@ if [ -x "$STOP_FAILURE_HOOK" ]; then fail "StopFailure hook overwrote PHASE:merged: first='$sf_first'" fi rm -f "$SF_MARKER" "$PHASE_FILE" + + # 10i: terminal phase guard — does not overwrite PHASE:needs_human + echo "PHASE:needs_human" > "$PHASE_FILE" + printf '{"stop_reason":"rate_limit"}' \ + | "$STOP_FAILURE_HOOK" "$PHASE_FILE" "$SF_MARKER" + sf_first=$(head -1 "$PHASE_FILE" 2>/dev/null) + if [ "$sf_first" = "PHASE:needs_human" ] && [ ! -f "$SF_MARKER" ]; then + ok "StopFailure hook does not overwrite terminal PHASE:needs_human" + else + fail "StopFailure hook overwrote PHASE:needs_human: first='$sf_first'" + fi + rm -f "$SF_MARKER" "$PHASE_FILE" else fail "StopFailure hook script not found or not executable: $STOP_FAILURE_HOOK" fi @@ -348,6 +360,47 @@ else fail "phase-changed marker did not reset mtime guard" fi +# ── Test 12: crash handler treats PHASE:needs_human as terminal ─────────── +# Simulates the monitor_phase_loop crash handler: when a session exits while +# the phase file holds PHASE:needs_human, it must be treated as terminal +# (fall through to the phase handler) rather than invoking callback with +# PHASE:crashed, which would lose the escalation intent. +CRASH_CALLBACK_PHASE="" +mock_crash_callback() { CRASH_CALLBACK_PHASE="$1"; } + +echo "PHASE:needs_human" > "$PHASE_FILE" +current_phase=$(head -1 "$PHASE_FILE" 2>/dev/null | tr -d '[:space:]' || true) +case "$current_phase" in + PHASE:done|PHASE:failed|PHASE:merged|PHASE:needs_human) + # terminal — fall through to phase handler (correct behavior) + mock_crash_callback "$current_phase" + ;; + *) + # would invoke callback with PHASE:crashed (incorrect for needs_human) + mock_crash_callback "PHASE:crashed" + ;; +esac + +if [ "$CRASH_CALLBACK_PHASE" = "PHASE:needs_human" ]; then + ok "crash handler preserves PHASE:needs_human (not replaced by PHASE:crashed)" +else + fail "crash handler lost escalation intent: expected PHASE:needs_human, got $CRASH_CALLBACK_PHASE" +fi + +# Also verify the other terminal phases still work in crash handler +for tp in "PHASE:done" "PHASE:failed" "PHASE:merged"; do + echo "$tp" > "$PHASE_FILE" + current_phase=$(head -1 "$PHASE_FILE" 2>/dev/null | tr -d '[:space:]' || true) + case "$current_phase" in + PHASE:done|PHASE:failed|PHASE:merged|PHASE:needs_human) + ok "crash handler treats $tp as terminal" + ;; + *) + fail "crash handler does not treat $tp as terminal" + ;; + esac +done + # ── Cleanup ─────────────────────────────────────────────────────────────────── rm -f "$PHASE_FILE" diff --git a/lib/agent-session.sh b/lib/agent-session.sh index 501d955..ffd71b1 100644 --- a/lib/agent-session.sh +++ b/lib/agent-session.sh @@ -330,7 +330,7 @@ monitor_phase_loop() { local current_phase current_phase=$(head -1 "$phase_file" 2>/dev/null | tr -d '[:space:]' || true) case "$current_phase" in - PHASE:done|PHASE:failed|PHASE:merged) + PHASE:done|PHASE:failed|PHASE:merged|PHASE:needs_human) ;; # terminal — fall through to phase handler *) # Call callback with "crashed" — let agent-specific code handle recovery diff --git a/lib/hooks/on-stop-failure.sh b/lib/hooks/on-stop-failure.sh index 9c61648..99b78ee 100755 --- a/lib/hooks/on-stop-failure.sh +++ b/lib/hooks/on-stop-failure.sh @@ -30,7 +30,7 @@ reason=$(printf '%s' "$input" | jq -r ' # the PostToolUse hook already recorded the correct terminal phase. existing=$(head -1 "$phase_file" 2>/dev/null | tr -d '[:space:]') case "$existing" in - PHASE:done|PHASE:merged) exit 0 ;; + PHASE:done|PHASE:merged|PHASE:needs_human) exit 0 ;; esac # Write phase file immediately — orchestrator reads first line as phase sentinel