fix: guard SUPERVISOR_EVAL_TIMEOUT against non-numeric values (PR #1958 quality-debt)#4581
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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: Sat Mar 14 03:30:03 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
…e.sh PR #1958 review feedback (CodeRabbit nitpick): arithmetic expansion on heartbeat_window fails if SUPERVISOR_EVAL_TIMEOUT is set to a non-integer (e.g. '90s'). Add a regex guard that defaults to 90 and logs a warning when the value is non-numeric. Note: the critical finding (heartbeat cleanup scope ordering in evaluate.sh) was already resolved by t1312 (PR #2221), which removed the heartbeat/watchdog code entirely from the active supervisor path before archiving. Closes #3364
alex-solovyev
force-pushed
the
bugfix/pr1958-quality-debt
branch
from
1a0eb47 to
43eead5
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: Sat Mar 14 03:36:26 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
…e.sh (#4581) PR #1958 review feedback (CodeRabbit nitpick): arithmetic expansion on heartbeat_window fails if SUPERVISOR_EVAL_TIMEOUT is set to a non-integer (e.g. '90s'). Add a regex guard that defaults to 90 and logs a warning when the value is non-numeric. Note: the critical finding (heartbeat cleanup scope ordering in evaluate.sh) was already resolved by t1312 (PR #2221), which removed the heartbeat/watchdog code entirely from the active supervisor path before archiving. Closes #3364
… quality-debt) (#4582) * fix(t3364): add numeric validation for SUPERVISOR_EVAL_TIMEOUT in archived pulse.sh (#4572) The heartbeat_window arithmetic at line 230 would emit a bash arithmetic error if SUPERVISOR_EVAL_TIMEOUT is set to a non-integer value (e.g. '90s'). Add a regex guard that defaults to 90 and logs a warning when the value is non-numeric. Note: the critical evaluate.sh heartbeat cleanup scope bug (the other finding from issue #3364 / PR #1958 review) was already resolved — evaluate_with_ai() containing the buggy push_cleanup/save_cleanup_scope ordering was deleted in PR #2221 (t1312 dead code removal) before the supervisor was archived in PR #2291. No change needed to evaluate.sh. Shellcheck: evaluate.sh passes clean (0 violations). pulse.sh passes bash -n syntax check; shellcheck OOMs on the 3983-line archived file (pre-existing, unrelated to this change). Closes #3364 * fix: guard SUPERVISOR_EVAL_TIMEOUT against non-numeric values in pulse.sh (#4581) PR #1958 review feedback (CodeRabbit nitpick): arithmetic expansion on heartbeat_window fails if SUPERVISOR_EVAL_TIMEOUT is set to a non-integer (e.g. '90s'). Add a regex guard that defaults to 90 and logs a warning when the value is non-numeric. Note: the critical finding (heartbeat cleanup scope ordering in evaluate.sh) was already resolved by t1312 (PR #2221), which removed the heartbeat/watchdog code entirely from the active supervisor path before archiving. Closes #3364 --------- Co-authored-by: alex-solovyev <1556417+alex-solovyev@users.noreply.github.com>
1 participant
Summary
Addresses the unactioned CodeRabbit review feedback from PR #1958 tracked in issue #3364.
Findings vs Current Code
Critical: heartbeat cleanup scope ordering (evaluate.sh)
Status: Already resolved. The bug existed in the pre-t1312 version of
supervisor/evaluate.sh(lines 1442-1478). The t1312 refactor (PR #2221) removed the heartbeat/watchdog code entirely before the file was archived assupervisor-archived/evaluate.sh. The archived file does not contain the buggy ordering — no fix needed.Nitpick: SUPERVISOR_EVAL_TIMEOUT non-numeric guard (pulse.sh)
Status: Fixed in this PR. Added a regex guard before the arithmetic expansion at line 229 of
supervisor-archived/pulse.sh. IfSUPERVISOR_EVAL_TIMEOUTis set to a non-integer (e.g.90s), bash arithmetic expansion would emit an error and the heartbeat window calculation would degrade silently. The guard defaults to 90 and emits alog_warnso the misconfiguration is visible in logs.Change
local eval_timeout_cfg="${SUPERVISOR_EVAL_TIMEOUT:-90}" + if ! [[ "$eval_timeout_cfg" =~ ^[0-9]+$ ]]; then + log_warn "_diagnose_stale_root_cause: SUPERVISOR_EVAL_TIMEOUT is non-numeric ('$eval_timeout_cfg'); defaulting to 90" + eval_timeout_cfg=90 + fi local heartbeat_window=$((eval_timeout_cfg * 2 + 60))Verification
bash -n .agents/scripts/supervisor-archived/pulse.sh— clean (no syntax errors)log_warnpattern used throughout the fileCloses #3364