fix(circuit-breaker): fix multi-line error matching in detect_stuck_loop
Addresses CodeRabbit outside diff range comment on lines 268-283:
CRITICAL BUG:
The detect_stuck_loop function had a multi-line string handling bug where
only the first error line was checked against historical outputs when
multiple distinct errors were present.
Problem:
grep -q "$current_errors" file.log
# When $current_errors has multiple lines, grep only matches first line
Impact:
Stuck loops with multiple recurring errors would not be detected correctly,
potentially allowing Ralph to continue running despite being genuinely stuck.
Fix:
Changed from simple grep to nested loop checking:
- For each historical output file
- For each error line in current output
- ALL error lines must appear in that file
- Only returns "stuck" if ALL files contain ALL current errors
Used grep -qF for literal fixed-string matching (not regex) to avoid
edge cases with special characters in error messages.
Test Coverage:
Added 2 new test scenarios (7 → 9 total tests):
Test 8: Multiple distinct errors where ALL repeat across history
Current: Error: Build failed + Fatal: DB lost + Exception: NPE
History: All 3 files contain all 3 errors
Expected: Stuck detected ✅
Test 9: Multiple errors where only some repeat
Current: Error: Build failed + Fatal: DB lost
History: Only first error appears consistently
Expected: Not stuck ✅
All existing tests continue to pass, validating backward compatibility.
Test results:
✓ Error detection tests: 13/13 passing
✓ Stuck loop tests: 9/9 passing (was 7/7)
✓ Total: 22/22 tests passing
This ensures detect_stuck_loop correctly handles the real-world scenario
where Ralph gets stuck on multiple simultaneous recurring errors.
Addresses CodeRabbit review comment:
- Outside diff range (lines 268-283): Multi-line error matching bug
This commit is contained in:
parent
890720a4f6
commit
63590b3d77
2 changed files with 57 additions and 5 deletions
|
|
@ -274,15 +274,26 @@ detect_stuck_loop() {
|
|||
fi
|
||||
|
||||
# Check if same errors appear in all recent outputs
|
||||
local stuck_count=0
|
||||
# For multi-line errors, verify ALL error lines appear in ALL history files
|
||||
local all_files_match=true
|
||||
while IFS= read -r output_file; do
|
||||
if grep -q "$current_errors" "$output_file" 2>/dev/null; then
|
||||
((stuck_count++))
|
||||
local file_matches_all=true
|
||||
while IFS= read -r error_line; do
|
||||
# Use -F for literal fixed-string matching (not regex)
|
||||
if ! grep -qF "$error_line" "$output_file" 2>/dev/null; then
|
||||
file_matches_all=false
|
||||
break
|
||||
fi
|
||||
done <<< "$current_errors"
|
||||
|
||||
if [[ "$file_matches_all" != "true" ]]; then
|
||||
all_files_match=false
|
||||
break
|
||||
fi
|
||||
done <<< "$recent_outputs"
|
||||
|
||||
if [[ $stuck_count -ge 3 ]]; then
|
||||
return 0 # Stuck on same error
|
||||
if [[ "$all_files_match" == "true" ]]; then
|
||||
return 0 # Stuck on same error(s)
|
||||
else
|
||||
return 1 # Making progress or different errors
|
||||
fi
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue