fix: remaining grep -c || echo multiline output breaking pulse-wrapper arithmetic#2745
Conversation
…n pulse-wrapper Two instances of `grep -c ... || echo "0"` were missed in bd3225e. When grep -c finds zero matches, it outputs "0" with exit code 1, triggering the || echo which appends another "0", producing "0\n0". This multiline value breaks bash arithmetic: $(( ... + 0\n0 )). Fix: use `count=$(grep -c ...) || count=0` pattern (assignment captures grep output; || handles the exit code separately).
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical issue in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughThis PR refactors error handling in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Mar 3 03:20:16 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves a bug where multiline output from a grep ... || echo pattern caused arithmetic errors in bash. The fix, which correctly separates the command's output assignment from its exit code handling, is applied consistently. The changes also include a beneficial refactoring that improves efficiency by replacing two grep calls with a single one. I've added a couple of minor suggestions to further improve efficiency by using here-strings, which avoid creating unnecessary subshells.
|
|
||
| local count | ||
| count=$(echo "$clean_result" | grep -c 'Removing' || echo "0") | ||
| count=$(echo "$clean_result" | grep -c 'Removing') || count=0 |
There was a problem hiding this comment.
This is a good fix. For a minor efficiency improvement, you can use a here-string (<<<) instead of piping from echo. This avoids creating an extra process and subshell.
| count=$(echo "$clean_result" | grep -c 'Removing') || count=0 | |
| count=$(grep -c 'Removing' <<< "$clean_result") || count=0 |
| if echo "$cleaned_output" | grep -q "Removing\|removed"; then | ||
| echo "[pulse-wrapper] Worktree cleanup: $(echo "$cleaned_output" | grep -c 'Removing') worktree(s) removed" >>"$LOGFILE" | ||
| local fallback_count | ||
| fallback_count=$(echo "$cleaned_output" | grep -c 'Removing') || fallback_count=0 |
There was a problem hiding this comment.
This refactoring is a good improvement as it avoids running grep twice. Similar to the previous comment, you can use a here-string (<<<) here for a minor efficiency gain by avoiding the echo subshell.
| fallback_count=$(echo "$cleaned_output" | grep -c 'Removing') || fallback_count=0 | |
| fallback_count=$(grep -c 'Removing' <<< "$cleaned_output") || fallback_count=0 |
1 participant
Summary
grep -c ... || echo "0"pattern incleanup_worktrees()that were missed in bd3225e"0\n0") when grep finds no matches, breaking bash arithmetic and causing pulse-wrapper to crash every 2 minutesRoot Cause
grep -c 'pattern' || echo "0"— when grep finds 0 matches, it outputs0with exit code 1. The|| echo "0"appends another0, resulting in"0\n0". This value in$((...))arithmetic triggerssyntax error in expression.Fix
Use
count=$(grep -c ...) || count=0— assignment captures grep's stdout;||handles the exit code separately without concatenation.Closes the recurring error:
Summary by CodeRabbit