diff --git a/CLAUDE.md b/CLAUDE.md index 750a93c..6f88a03 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -165,11 +165,33 @@ Each loop iteration injects context via `build_loop_context()`: - Disable with `--no-continue` for isolated iterations ### Intelligent Exit Detection -The loop automatically exits when it detects project completion through: -- Multiple consecutive "done" signals from Claude Code -- Too many test-only loops indicating feature completeness +The loop uses a dual-condition check to prevent premature exits during productive iterations: + +**Exit requires BOTH conditions:** +1. `recent_completion_indicators >= 2` (heuristic-based detection from natural language patterns) +2. Claude's explicit `EXIT_SIGNAL: true` in the RALPH_STATUS block + +The `EXIT_SIGNAL` value is read from `.response_analysis` (at `.analysis.exit_signal`) which is populated by `response_analyzer.sh` from Claude's RALPH_STATUS output block. + +**Other exit conditions (checked before completion indicators):** +- Multiple consecutive "done" signals from Claude Code (`done_signals >= 2`) +- Too many test-only loops indicating feature completeness (`test_loops >= 3`) - All items in @fix_plan.md marked as completed -- Strong completion indicators in responses + +**Example behavior when EXIT_SIGNAL is false:** +``` +Loop 5: Claude outputs "Phase complete, moving to next feature" + → completion_indicators: 3 (high confidence from patterns) + → EXIT_SIGNAL: false (Claude explicitly says more work needed) + → Result: CONTINUE (respects Claude's explicit intent) + +Loop 8: Claude outputs "All tasks complete, project ready" + → completion_indicators: 4 + → EXIT_SIGNAL: true (Claude confirms project is done) + → Result: EXIT with "project_complete" +``` + +**Rationale:** Natural language patterns like "done" or "complete" can trigger false positives during productive work (e.g., "feature done, moving to tests"). By requiring Claude's explicit EXIT_SIGNAL confirmation, Ralph avoids exiting mid-iteration when Claude is still working. ## CI/CD Pipeline @@ -256,6 +278,33 @@ Ralph uses multiple mechanisms to detect when to exit: - `TEST_PERCENTAGE_THRESHOLD=30%` - Flag if testing dominates recent loops - Completion detection via @fix_plan.md checklist items +### Completion Indicators with EXIT_SIGNAL Gate + +The `completion_indicators` exit condition requires dual verification: + +| completion_indicators | EXIT_SIGNAL | .response_analysis | Result | +|-----------------------|-------------|-------------------|--------| +| >= 2 | `true` | exists | **Exit** ("project_complete") | +| >= 2 | `false` | exists | **Continue** (Claude still working) | +| >= 2 | N/A | missing | **Continue** (defaults to false) | +| >= 2 | N/A | malformed | **Continue** (defaults to false) | +| < 2 | `true` | exists | **Continue** (threshold not met) | + +**Implementation** (`ralph_loop.sh:312-327`): +```bash +local claude_exit_signal="false" +if [[ -f ".response_analysis" ]]; then + claude_exit_signal=$(jq -r '.analysis.exit_signal // false' ".response_analysis" 2>/dev/null || echo "false") +fi + +if [[ $recent_completion_indicators -ge 2 ]] && [[ "$claude_exit_signal" == "true" ]]; then + echo "project_complete" + return 0 +fi +``` + +**Conflict Resolution:** When `STATUS: COMPLETE` but `EXIT_SIGNAL: false` in RALPH_STATUS, the explicit EXIT_SIGNAL takes precedence. This allows Claude to mark a phase complete while indicating more phases remain. + ### Circuit Breaker Thresholds - `CB_NO_PROGRESS_THRESHOLD=3` - Open circuit after 3 loops with no file changes - `CB_SAME_ERROR_THRESHOLD=5` - Open circuit after 5 loops with repeated errors diff --git a/lib/response_analyzer.sh b/lib/response_analyzer.sh index 107326d..b451d42 100644 --- a/lib/response_analyzer.sh +++ b/lib/response_analyzer.sh @@ -297,13 +297,29 @@ analyze_response() { # Text parsing fallback (original logic) + # Track whether an explicit EXIT_SIGNAL was found in RALPH_STATUS block + # If explicit signal found, heuristics should NOT override Claude's intent + local explicit_exit_signal_found=false + # 1. Check for explicit structured output (if Claude follows schema) if grep -q -- "---RALPH_STATUS---" "$output_file"; then # Parse structured output local status=$(grep "STATUS:" "$output_file" | cut -d: -f2 | xargs) local exit_sig=$(grep "EXIT_SIGNAL:" "$output_file" | cut -d: -f2 | xargs) - if [[ "$exit_sig" == "true" || "$status" == "COMPLETE" ]]; then + # If EXIT_SIGNAL is explicitly provided, respect it + if [[ -n "$exit_sig" ]]; then + explicit_exit_signal_found=true + if [[ "$exit_sig" == "true" ]]; then + has_completion_signal=true + exit_signal=true + confidence_score=100 + else + # Explicit EXIT_SIGNAL: false - Claude says to continue + exit_signal=false + fi + elif [[ "$status" == "COMPLETE" ]]; then + # No explicit EXIT_SIGNAL but STATUS is COMPLETE has_completion_signal=true exit_signal=true confidence_score=100 @@ -398,9 +414,13 @@ analyze_response() { fi fi - # 9. Determine exit signal based on confidence - if [[ $confidence_score -ge 40 || "$has_completion_signal" == "true" ]]; then - exit_signal=true + # 9. Determine exit signal based on confidence (heuristic) + # IMPORTANT: Only apply heuristics if no explicit EXIT_SIGNAL was found in RALPH_STATUS + # Claude's explicit intent takes precedence over natural language pattern matching + if [[ "$explicit_exit_signal_found" != "true" ]]; then + if [[ $confidence_score -ge 40 || "$has_completion_signal" == "true" ]]; then + exit_signal=true + fi fi # Write analysis results to file (text parsing path) using jq for safe construction diff --git a/ralph_loop.sh b/ralph_loop.sh index 98551cf..ccd039c 100755 --- a/ralph_loop.sh +++ b/ralph_loop.sh @@ -309,11 +309,21 @@ should_exit_gracefully() { return 0 fi - # 3. Strong completion indicators - if [[ $recent_completion_indicators -ge 2 ]]; then - log_status "WARN" "Exit condition: Strong completion indicators ($recent_completion_indicators)" + # 3. Strong completion indicators (only if Claude's EXIT_SIGNAL is true) + # This prevents premature exits when heuristics detect completion patterns + # but Claude explicitly indicates work is still in progress via RALPH_STATUS block. + # The exit_signal in .response_analysis represents Claude's explicit intent. + local claude_exit_signal="false" + if [[ -f ".response_analysis" ]]; then + claude_exit_signal=$(jq -r '.analysis.exit_signal // false' ".response_analysis" 2>/dev/null || echo "false") + fi + + if [[ $recent_completion_indicators -ge 2 ]] && [[ "$claude_exit_signal" == "true" ]]; then + log_status "WARN" "Exit condition: Strong completion indicators ($recent_completion_indicators) with EXIT_SIGNAL=true" >&2 echo "project_complete" return 0 + elif [[ $recent_completion_indicators -ge 2 ]]; then + log_status "INFO" "DEBUG: Completion indicators ($recent_completion_indicators) present but EXIT_SIGNAL=false, continuing..." >&2 fi # 4. Check fix_plan.md for completion diff --git a/tests/integration/test_edge_cases.bats b/tests/integration/test_edge_cases.bats index 81a03fb..d8b17e1 100644 --- a/tests/integration/test_edge_cases.bats +++ b/tests/integration/test_edge_cases.bats @@ -411,3 +411,197 @@ EOF [[ "$result" -eq 0 || "$result" -eq 1 ]] } + +# ============================================================================= +# EXIT_SIGNAL INTEGRATION TESTS +# Tests for the fix that ensures completion indicators only trigger exit +# when Claude's explicit EXIT_SIGNAL is true +# ============================================================================= + +# Edge Case 21: Multiple loops with EXIT_SIGNAL=false should continue +@test "multiple loops continue when confidence high but EXIT_SIGNAL=false" { + local output_file="$LOG_DIR/loop.log" + + # Simulate 3 loops with explicit EXIT_SIGNAL: false + for i in {1..3}; do + cat > "$output_file" << 'EOF' +---RALPH_STATUS--- +STATUS: IN_PROGRESS +EXIT_SIGNAL: false +WORK_TYPE: IMPLEMENTATION +---END_RALPH_STATUS--- + +Work complete for this iteration. +Project progressing well, all tasks for this phase done. +Ready for next steps. +EOF + + analyze_response "$output_file" $i + update_exit_signals + + # After each loop, check that exit_signal is correctly captured as false + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "false" + done + + # Verify that analyze_response correctly captures EXIT_SIGNAL=false + local final_exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$final_exit_signal" "false" + + # Key test: Even with high completion indicators set externally, + # the exit_signal should still be false (respecting Claude's explicit intent) + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2,3]}' > "$EXIT_SIGNALS_FILE" + local last_exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$last_exit_signal" "false" +} + +# Edge Case 22: Transition from IN_PROGRESS to COMPLETE +@test "loop exits when transitioning from EXIT_SIGNAL=false to EXIT_SIGNAL=true" { + local output_file="$LOG_DIR/loop.log" + + # Loop 1-2: IN_PROGRESS with EXIT_SIGNAL=false + for i in 1 2; do + cat > "$output_file" << 'EOF' +---RALPH_STATUS--- +STATUS: IN_PROGRESS +EXIT_SIGNAL: false +---END_RALPH_STATUS--- + +Feature implementation in progress. +EOF + + analyze_response "$output_file" $i + update_exit_signals + + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "false" + done + + # Loop 3: COMPLETE with EXIT_SIGNAL=true + cat > "$output_file" << 'EOF' +---RALPH_STATUS--- +STATUS: COMPLETE +EXIT_SIGNAL: true +---END_RALPH_STATUS--- + +All tasks complete. Project ready for review. +EOF + + analyze_response "$output_file" 3 + update_exit_signals + + # Exit signal should now be true + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "true" + + # Confidence should be >= 100 (100 from EXIT_SIGNAL: true, plus any natural language bonuses) + local confidence=$(jq -r '.analysis.confidence_score' .response_analysis) + [[ "$confidence" -ge 100 ]] +} + +# Edge Case 23: Missing .response_analysis mid-loop +@test "graceful handling when .response_analysis deleted mid-loop" { + local output_file="$LOG_DIR/loop.log" + + # Create initial analysis + cat > "$output_file" << 'EOF' +---RALPH_STATUS--- +STATUS: IN_PROGRESS +EXIT_SIGNAL: false +---END_RALPH_STATUS--- + +Working on implementation. +EOF + + analyze_response "$output_file" 1 + update_exit_signals + + # Verify file exists + assert_file_exists ".response_analysis" + + # Simulate file deletion (e.g., cleanup script ran) + rm -f ".response_analysis" + + # Add more completion indicators + cat > "$output_file" << 'EOF' +Project complete. +EOF + + analyze_response "$output_file" 2 + update_exit_signals + + # File should be recreated + assert_file_exists ".response_analysis" +} + +# Edge Case 24: STATUS=COMPLETE but EXIT_SIGNAL=false conflict in RALPH_STATUS +@test "analyze_response respects EXIT_SIGNAL=false even when STATUS=COMPLETE" { + local output_file="$LOG_DIR/conflict.log" + + # Create output with conflicting signals + # This can happen when Claude completes a phase but has more phases to do + cat > "$output_file" << 'EOF' +---RALPH_STATUS--- +STATUS: COMPLETE +EXIT_SIGNAL: false +WORK_TYPE: IMPLEMENTATION +---END_RALPH_STATUS--- + +Phase 1 implementation complete. +Moving on to Phase 2 next. +EOF + + analyze_response "$output_file" 1 + + # EXIT_SIGNAL: false should take precedence over STATUS: COMPLETE + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "false" + + # has_completion_signal can still be true (STATUS was COMPLETE) + # but exit_signal must be false per Claude's explicit intent +} + +# Edge Case 25: JSON format response with EXIT_SIGNAL handling +@test "JSON format response correctly handles EXIT_SIGNAL" { + local output_file="$LOG_DIR/json_response.log" + + # Create JSON format response (Claude CLI format) + cat > "$output_file" << 'EOF' +{ + "result": "Implementation in progress, more work needed", + "sessionId": "test-session-123", + "metadata": { + "files_changed": 5, + "has_errors": false, + "completion_status": "in_progress" + } +} +EOF + + analyze_response "$output_file" 1 + update_exit_signals + + # Exit signal should be false (completion_status is in_progress) + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "false" + + # Now test with complete status + cat > "$output_file" << 'EOF' +{ + "result": "All tasks completed successfully", + "sessionId": "test-session-124", + "metadata": { + "files_changed": 0, + "has_errors": false, + "completion_status": "complete" + } +} +EOF + + analyze_response "$output_file" 2 + update_exit_signals + + # Exit signal should be true (completion_status is complete) + local exit_signal=$(jq -r '.analysis.exit_signal' .response_analysis) + assert_equal "$exit_signal" "true" +} diff --git a/tests/unit/test_exit_detection.bats b/tests/unit/test_exit_detection.bats index 0482e2e..8ab7c3d 100644 --- a/tests/unit/test_exit_detection.bats +++ b/tests/unit/test_exit_detection.bats @@ -9,6 +9,7 @@ setup() { # Set up environment export EXIT_SIGNALS_FILE=".exit_signals" + export RESPONSE_ANALYSIS_FILE=".response_analysis" export MAX_CONSECUTIVE_TEST_LOOPS=3 export MAX_CONSECUTIVE_DONE_SIGNALS=2 @@ -26,6 +27,7 @@ teardown() { } # Helper function: should_exit_gracefully (extracted from ralph_loop.sh) +# Updated to respect EXIT_SIGNAL from .response_analysis for completion indicators should_exit_gracefully() { if [[ ! -f "$EXIT_SIGNALS_FILE" ]]; then echo "" # Return empty string instead of using return code @@ -57,8 +59,15 @@ should_exit_gracefully() { return 0 fi - # 3. Strong completion indicators - if [[ $recent_completion_indicators -ge 2 ]]; then + # 3. Strong completion indicators (only if Claude's EXIT_SIGNAL is true) + # This prevents premature exits when heuristics detect completion patterns + # but Claude explicitly indicates work is still in progress + local claude_exit_signal="false" + if [[ -f "$RESPONSE_ANALYSIS_FILE" ]]; then + claude_exit_signal=$(jq -r '.analysis.exit_signal // false' "$RESPONSE_ANALYSIS_FILE" 2>/dev/null || echo "false") + fi + + if [[ $recent_completion_indicators -ge 2 ]] && [[ "$claude_exit_signal" == "true" ]]; then echo "project_complete" return 0 fi @@ -138,10 +147,21 @@ should_exit_gracefully() { assert_equal "$result" "" } -# Test 8: Exit on completion indicators (2 indicators) +# Test 8: Exit on completion indicators (2 indicators) with EXIT_SIGNAL=true @test "should_exit_gracefully exits on 2 completion indicators" { echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2]}' > "$EXIT_SIGNALS_FILE" + # Must also have exit_signal=true in .response_analysis (after fix) + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 2, + "analysis": { + "exit_signal": true, + "confidence_score": 80 + } +} +EOF + result=$(should_exit_gracefully || true) assert_equal "$result" "project_complete" } @@ -265,3 +285,235 @@ EOF result=$(should_exit_gracefully) assert_equal "$result" "completion_signals" } + +# ============================================================================= +# EXIT_SIGNAL RESPECT TESTS (Issue: Premature exit when EXIT_SIGNAL=false) +# ============================================================================= +# These tests verify that completion indicators only trigger exit when +# Claude's explicit EXIT_SIGNAL is true, preventing premature exits during +# productive iterations. + +# Test 21: Completion indicators with EXIT_SIGNAL=false should continue +@test "should_exit_gracefully continues when completion indicators high but EXIT_SIGNAL=false" { + # Setup: High completion indicators (would normally exit) + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2,3]}' > "$EXIT_SIGNALS_FILE" + + # Setup: Claude's explicit exit signal is false (still working) + mkdir -p "$(dirname "$RESPONSE_ANALYSIS_FILE")" + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 3, + "timestamp": "2026-01-12T10:00:00Z", + "output_format": "text", + "analysis": { + "has_completion_signal": true, + "is_test_only": false, + "is_stuck": false, + "has_progress": true, + "files_modified": 5, + "confidence_score": 70, + "exit_signal": false, + "work_summary": "Implementing feature, still in progress" + } +} +EOF + + result=$(should_exit_gracefully || true) + # Should NOT exit because EXIT_SIGNAL is false + assert_equal "$result" "" +} + +# Test 22: Completion indicators with EXIT_SIGNAL=true should exit +@test "should_exit_gracefully exits when completion indicators high AND EXIT_SIGNAL=true" { + # Setup: High completion indicators + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2]}' > "$EXIT_SIGNALS_FILE" + + # Setup: Claude's explicit exit signal is true (project complete) + mkdir -p "$(dirname "$RESPONSE_ANALYSIS_FILE")" + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 2, + "timestamp": "2026-01-12T10:00:00Z", + "output_format": "text", + "analysis": { + "has_completion_signal": true, + "is_test_only": false, + "is_stuck": false, + "has_progress": false, + "files_modified": 0, + "confidence_score": 100, + "exit_signal": true, + "work_summary": "All tasks complete, project ready for review" + } +} +EOF + + result=$(should_exit_gracefully) + # Should exit because BOTH conditions are met + assert_equal "$result" "project_complete" +} + +# Test 23: Completion indicators without .response_analysis file should continue +@test "should_exit_gracefully continues when .response_analysis file missing" { + # Setup: High completion indicators + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2,3]}' > "$EXIT_SIGNALS_FILE" + + # Don't create .response_analysis - defaults to exit_signal=false + rm -f "$RESPONSE_ANALYSIS_FILE" + + result=$(should_exit_gracefully || true) + # Should NOT exit because exit_signal defaults to false + assert_equal "$result" "" +} + +# Test 24: Completion indicators with malformed .response_analysis should continue +@test "should_exit_gracefully continues when .response_analysis has invalid JSON" { + # Setup: High completion indicators + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2]}' > "$EXIT_SIGNALS_FILE" + + # Setup: Corrupted/invalid JSON in .response_analysis + echo 'invalid json{broken' > "$RESPONSE_ANALYSIS_FILE" + + result=$(should_exit_gracefully || true) + # Should NOT exit because jq parsing fails, defaults to false + assert_equal "$result" "" +} + +# Test 25: EXIT_SIGNAL=true but completion indicators below threshold should continue +@test "should_exit_gracefully continues when EXIT_SIGNAL=true but indicators below threshold" { + # Setup: Only 1 completion indicator (below threshold of 2) + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1]}' > "$EXIT_SIGNALS_FILE" + + # Setup: Claude says exit is true + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 1, + "analysis": { + "exit_signal": true, + "confidence_score": 100 + } +} +EOF + + result=$(should_exit_gracefully || true) + # Should NOT exit because indicators below threshold + assert_equal "$result" "" +} + +# Test 26: EXIT_SIGNAL=false with explicit false value in JSON +@test "should_exit_gracefully handles explicit false exit_signal" { + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2,3,4,5]}' > "$EXIT_SIGNALS_FILE" + + # Explicit false value + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "analysis": { + "exit_signal": false + } +} +EOF + + result=$(should_exit_gracefully || true) + assert_equal "$result" "" +} + +# Test 27: EXIT_SIGNAL missing from analysis object should default to false +@test "should_exit_gracefully defaults to false when exit_signal field missing" { + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2]}' > "$EXIT_SIGNALS_FILE" + + # analysis object exists but no exit_signal field + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 5, + "analysis": { + "confidence_score": 80, + "has_completion_signal": true, + "is_test_only": false + } +} +EOF + + result=$(should_exit_gracefully || true) + # Missing exit_signal should default to false, so continue + assert_equal "$result" "" +} + +# Test 28: Test priority - test_saturation still takes priority over completion indicators +@test "should_exit_gracefully test_saturation takes priority even with EXIT_SIGNAL=false" { + # Test loops should still trigger exit regardless of EXIT_SIGNAL + echo '{"test_only_loops": [1,2,3,4], "done_signals": [], "completion_indicators": [1]}' > "$EXIT_SIGNALS_FILE" + + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "analysis": { + "exit_signal": false + } +} +EOF + + result=$(should_exit_gracefully) + # test_saturation is checked before completion_indicators + assert_equal "$result" "test_saturation" +} + +# Test 29: done_signals still takes priority over completion indicators +@test "should_exit_gracefully done_signals takes priority even with EXIT_SIGNAL=false" { + echo '{"test_only_loops": [], "done_signals": [1,2,3], "completion_indicators": [1]}' > "$EXIT_SIGNALS_FILE" + + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "analysis": { + "exit_signal": false + } +} +EOF + + result=$(should_exit_gracefully) + # done_signals is checked before completion_indicators + assert_equal "$result" "completion_signals" +} + +# Test 30: Empty analysis object in .response_analysis should default to false +@test "should_exit_gracefully handles empty analysis object" { + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2]}' > "$EXIT_SIGNALS_FILE" + + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 3, + "analysis": {} +} +EOF + + result=$(should_exit_gracefully || true) + assert_equal "$result" "" +} + +# Test 31: STATUS=COMPLETE but EXIT_SIGNAL=false conflict - EXIT_SIGNAL takes precedence +@test "should_exit_gracefully respects EXIT_SIGNAL=false even when STATUS=COMPLETE" { + # Setup: High completion indicators + echo '{"test_only_loops": [], "done_signals": [], "completion_indicators": [1,2,3]}' > "$EXIT_SIGNALS_FILE" + + # Setup: Conflicting signals - STATUS says COMPLETE but EXIT_SIGNAL explicitly false + # This can happen when Claude marks a phase complete but has more work to do + cat > "$RESPONSE_ANALYSIS_FILE" << 'EOF' +{ + "loop_number": 3, + "timestamp": "2026-01-12T10:00:00Z", + "output_format": "text", + "analysis": { + "has_completion_signal": true, + "is_test_only": false, + "is_stuck": false, + "has_progress": true, + "files_modified": 3, + "confidence_score": 100, + "exit_signal": false, + "work_summary": "Phase complete, but more phases remain" + } +} +EOF + + result=$(should_exit_gracefully || true) + # EXIT_SIGNAL=false should take precedence, continue working + assert_equal "$result" "" +}