fix: prevent arithmetic error when fix_plan.md has no matching checkboxes#260
fix: prevent arithmetic error when fix_plan.md has no matching checkboxes#260fengwuqingchen wants to merge 1 commit intofrankbria:mainfrom
Conversation
…oxes
`should_exit_gracefully` counts checkbox items in fix_plan.md to decide
whether all tasks are done. The previous form
local uncompleted_items=$(grep -cE "..." "$file" 2>/dev/null || echo "0")
breaks under BSD grep (macOS): when there are no matches, BSD grep
*both* prints "0" to stdout *and* exits 1. The `|| echo "0"` fallback
then appends a second "0", so the variable becomes a multi-line
"0\n0" string. The very next line
local total_items=$((uncompleted_items + completed_items))
then fails with:
syntax error in expression (error token is "0")
This silently disables the "all fix_plan.md items completed" exit
condition (the function falls through), but more visibly it spams
the loop log every iteration.
Pipe `grep -c` through `head -n1` instead. The pipeline's exit code
becomes head's (always 0), and BSD grep already prints "0" on no
match, so the count line is always available. Defensive `${var:-0}`
covers the edge case of grep producing no output at all (e.g. file
disappears mid-read).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughModified Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
should_exit_gracefullyinralph_loop.shcounts checkbox items infix_plan.mdto decide whether all tasks are done. The current form fails on macOS:Root cause
BSD grep (macOS default) both prints
"0"to stdout and exits 1 when there are no matches. The|| echo "0"fallback therefore appends a second"0", and the variable becomes a multi-line"0\n0"string. The next line:then crashes with:
Impact
syntax error in expressionmessage every iteration.grep -cexits 0 on zero matches, so the||fallback never runs.Fix
Pipe
grep -cthroughhead -n1. The pipeline's exit code becomeshead's (always 0), so the brittle||fallback is no longer needed. BSD grep already prints"0"on no match, so the count line is always available. A defensive${var:-0}covers the edge case of grep producing no output at all.Test plan
bash -n ralph_loop.sh— syntax OKfix_plan.md→u=0 c=0 total=0(no error)[x], 0[ ]) →u=0 c=2 total=2and triggers theplan_completebranchsyntax error in expressionline no longer appears in.ralph/logs/Summary by CodeRabbit
Release Notes