GH#2898: Fix quality-debt in session-count-helper.sh from PR #2883 review feedback#2962
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR addresses quality feedback by removing stderr suppression ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.agents/scripts/session-count-helper.sh (2)
160-167: Broadaiderpattern may match unrelated processes.The pattern
'aider'will match any process with "aider" anywhere in its command line, including unrelated matches like paths containing "aider" or processes like "raider". Consider a more specific pattern:💡 Suggested refinement
# --- Aider sessions --- local aider_pids="" - aider_pids=$(pgrep -f 'aider' || true) + aider_pids=$(pgrep -f '[/[:space:]]aider([[:space:]]|$)' || true)Alternatively, if aider is typically invoked directly:
- aider_pids=$(pgrep -f 'aider' || true) + aider_pids=$(pgrep -x aider || pgrep -f 'python.*aider' || true)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/session-count-helper.sh around lines 160 - 167, The pgrep call using the broad pattern 'aider' can match unrelated commands; update the helper to match the actual process name or full executable path instead. Replace pgrep -f 'aider' with a stricter match (for example pgrep -x 'aider' || true) or use the full path (pgrep -f '/path/to/aider' || true) so the variables aider_pids and aider_count only include real aider processes; keep the existing logic that computes aider_count and increments count.
141-149: Cross-platform note: Cursor detection is macOS-centric.The pattern
Cursor\.apptargets macOS application bundles. On Linux, Cursor typically runs ascursoror similar. Consider adding a fallback pattern for Linux users:💡 Optional enhancement for Linux support
# --- Cursor sessions --- # Note: pgrep -c is Linux-only; use pgrep | wc -l for cross-platform local cursor_pids="" - cursor_pids=$(pgrep -f 'Cursor\.app' || true) + cursor_pids=$(pgrep -f 'Cursor\.app' || pgrep -xf 'cursor' || true) if [[ -n "$cursor_pids" ]]; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/session-count-helper.sh around lines 141 - 149, The cursor detection currently only looks for the macOS bundle pattern 'Cursor\.app' (variable cursor_pids / cursor_count); update the pgrep invocation to include a cross-platform fallback so Linux processes named "cursor" are matched too (e.g., broaden the search pattern or try a second pgrep for "cursor" when the first returns empty), and ensure the existing logic that counts lines into cursor_count and adds to count remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.agents/scripts/session-count-helper.sh:
- Around line 160-167: The pgrep call using the broad pattern 'aider' can match
unrelated commands; update the helper to match the actual process name or full
executable path instead. Replace pgrep -f 'aider' with a stricter match (for
example pgrep -x 'aider' || true) or use the full path (pgrep -f
'/path/to/aider' || true) so the variables aider_pids and aider_count only
include real aider processes; keep the existing logic that computes aider_count
and increments count.
- Around line 141-149: The cursor detection currently only looks for the macOS
bundle pattern 'Cursor\.app' (variable cursor_pids / cursor_count); update the
pgrep invocation to include a cross-platform fallback so Linux processes named
"cursor" are matched too (e.g., broaden the search pattern or try a second pgrep
for "cursor" when the first returns empty), and ensure the existing logic that
counts lines into cursor_count and adds to count remains unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6673766-57a5-427f-8fc9-94440defe286
📒 Files selected for processing (3)
.agents/scripts/aidevops-update-check.sh.agents/scripts/pre-edit-check.sh.agents/scripts/session-count-helper.sh
|
Merge conflict detected — cannot auto-merge. Dispatching worker to rebase and resolve. PR has APPROVED review from coderabbitai and passing CI. |
…ions() coverage Address PR #2883 review feedback (GH#2898): - Remove 2>/dev/null from all pgrep calls — || true already handles no-match exit codes, and stderr suppression hid real errors (e.g. pgrep -c unsupported on macOS) - Replace Linux-only pgrep -c with pgrep | wc -l for cross-platform compat - Extend list_sessions() to include Cursor, Windsurf, and Aider processes, matching the coverage of count_interactive_sessions() - Extract _print_session_detail() helper to reduce duplication in list output - Remove 2>/dev/null from session-count-helper.sh callers in aidevops-update-check.sh and pre-edit-check.sh Closes #2898
marcusquinn
force-pushed
the
bugfix/session-count-quality-debt
branch
from
18d89e9 to
360ef96
Compare
🔍 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: Fri Mar 6 03:04:55 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
1 participant
Summary
Fixes quality-debt from PR #2883 Gemini review feedback (2 medium-severity findings).
Changes
2>/dev/nullfrom allpgrepcalls (lines 84, 120, 143, 148, 153, 167, 204):|| trueis sufficient to handle the no-match exit code;2>/dev/nullwas hiding potential system errors like "command not found"list_sessions()to include Cursor, Windsurf, and Aider sessions: Previously only OpenCode and Claude Code were listed with details, causing a mismatch betweencountandlistoutput. Added_print_session_detail()helper to reduce duplicationpgrep -cwithpgrep | wc -lfor Cursor/Windsurf/Aider counting:pgrep -cis Linux-only; the new pattern is cross-platform (macOS + Linux)Verification
countandlistcommands work correctlyCloses #2898
Summary by CodeRabbit
Bug Fixes
Enhancements