fix(supervisor): add cron PATH resolution and no_pr retry limit #435
Conversation
- Add PATH augmentation at script startup for cron environments that lack /opt/homebrew/bin, ~/.local/bin, etc. This fixes gh/opencode/node being unreachable from cron-triggered pulses, which caused all pr_review tasks to loop forever in a no_pr retry cycle. - Add retry counter (max 5) to the no_pr case in pr_lifecycle to prevent infinite loops. After 5 failures, task transitions to blocked with diagnostic info (whether gh is in PATH). - Fix test suite false positive: use git ls-files instead of find to validate JSON configs, excluding gitignored working copies.
WalkthroughThe PR refactors error handling in clawdhub-helper.sh to improve fallback execution flow, enhances supervisor-helper.sh with PATH augmentation for cron-triggered runs and introduces a no_pr retry-tracking mechanism with a 5-attempt blocking threshold, and restricts quality test scope to git-tracked configuration files only. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 |
Summary of ChangesHello @marcusquinn, 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 critical reliability issues within the supervisor system, primarily focusing on robust operation within cron environments and preventing task stagnation. By enhancing PATH resolution, it ensures essential command-line tools are always accessible. Additionally, it introduces a crucial retry limit for PR-related tasks, preventing infinite loops and providing clearer diagnostic feedback. Finally, it refines the test suite to improve accuracy and avoid misleading failures. 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
|
🔍 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 Feb 7 15:34:48 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![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 introduces important robustness improvements to the supervisor script. The addition of PATH resolution for cron jobs is a critical fix for ensuring tool availability, and the retry limit on the no_pr case effectively prevents infinite loops. The test suite is also improved by scoping JSON validation to tracked files. I have a couple of suggestions to further enhance the code quality and correctness.
…eck SC1073/SC1072 The pattern 'if (...); then || exit' is invalid bash - the 'then' clause already handles the conditional, making '|| exit' both syntactically wrong and semantically redundant. Removed from 3 occurrences.
🔍 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 Feb 7 15:49:47 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.agents/scripts/supervisor-helper.sh:
- Around line 3165-3167: The current SELECT using json_extract on the
tasks.error column loses the no_pr_retries counter when other code
(cmd_transition) overwrites error with plain text; fix by changing
cmd_transition so when it writes a plain-text error it first reads the existing
tasks.error (via the same db/select used for no_pr_count), extracts any existing
no_pr_retries value, and then writes back a JSON object like {"msg":"CI checks
failed","no_pr_retries":<existing_or_0>} instead of a raw string; update
cmd_transition's error-write path to preserve or merge the no_pr_retries field
rather than replacing error with plain text so the json_extract(...) used for
no_pr_count continues to work.
- Around line 3162-3183: The db invocations that read and write the no_pr retry
counter are missing the SUPERVISOR_DB argument causing sqlite3 to misinterpret
the SQL as a filename and the retry counter (no_pr_count) to never persist;
update both calls to db so the first argument is $SUPERVISOR_DB (e.g., db
"$SUPERVISOR_DB" "...SQL...") when selecting and updating the tasks.error JSON,
and remove the unused no_pr_key variable to avoid dead code.
The apply_automatic_fixes() function in monitor-code-review.sh used a broad sed regex to add '|| exit' after 'cd' commands, but it matched cd inside subshells within if conditions, producing invalid syntax: if (cd "$dir" && cmd); then || exit This caused ShellCheck SC1073/SC1072 failures (PR #435, commit aa276b3). Disabled the auto-fix until a safe implementation with post-fix ShellCheck validation is available.
… marketplace path (#436) * fix(security): replace eval with arrays in 4 scripts, fix marketplace.json path t130.1: Fix .claude-plugin/marketplace.json source path from .agent to .agents t135.7: Replace eval with bash arrays in wp-helper.sh (3), coderabbit-cli.sh (2), codacy-cli.sh (3), and pandoc-helper.sh (1) - total 9 eval removals. Refactored wp-helper build_ssh_command to execute_wp_via_ssh (direct execution). All scripts pass ShellCheck -S error. * fix(security): add trap cleanup for temp files in secret-helper and version-manager t135.9 (partial): Add EXIT trap for mktemp cleanup in secret-helper.sh (prevents credential leaks on interrupt) and version-manager.sh (prevents partial changelog files). Trap is reset after normal cleanup completes. * refactor: archive 12 completed one-time fix scripts to _archive/ t135.12: Move 11 fix-*.sh scripts and add-missing-returns.sh to .agents/scripts/_archive/ with README documenting purpose and origin. All scripts had 0 references in active codebase. Preserves git history and patterns for future bulk fix operations. * fix(clawdhub): remove invalid '; then || exit' re-introduced by auto-fix bot The auto-fix GitHub Action (aa276b3) re-introduced the invalid '; then || exit' syntax that was fixed in PR #435. This is a known issue with the auto-fix workflow applying incorrect patterns. * fix(ci): disable broken auto-fix that introduces ShellCheck regressions The apply_automatic_fixes() function in monitor-code-review.sh used a broad sed regex to add '|| exit' after 'cd' commands, but it matched cd inside subshells within if conditions, producing invalid syntax: if (cd "$dir" && cmd); then || exit This caused ShellCheck SC1073/SC1072 failures (PR #435, commit aa276b3). Disabled the auto-fix until a safe implementation with post-fix ShellCheck validation is available.
… HOMEBREW_PREFIX guard (t147.1) - Add $SUPERVISOR_DB as first arg to both db() calls in no_pr retry counter logic (lines 3165, 3183). Without it, sqlite3 treated the SQL as a filename and failed silently — retry counter never persisted, making the 5-attempt threshold unreachable. - Remove unused no_pr_key variable. - Remove HOMEBREW_PREFIX guard around PATH augmentation. The idempotent PATH check already prevents duplicates; the guard was overly restrictive for cron environments where HOMEBREW_PREFIX may be set without all tool paths present. Triaged all 4 unresolved review threads from PR #435: - 3 fixed (critical db bug, unused var, PATH guard) - 1 acknowledged won't-fix (json_extract counter reset is by design) Refs: GH#438, t147.1
…147.1) (#450) * chore: mark t142 complete in TODO.md (#449) * chore: mark t135.9 complete in TODO.md (#448) * fix(supervisor): add missing $SUPERVISOR_DB arg to db() calls, remove HOMEBREW_PREFIX guard (t147.1) - Add $SUPERVISOR_DB as first arg to both db() calls in no_pr retry counter logic (lines 3165, 3183). Without it, sqlite3 treated the SQL as a filename and failed silently — retry counter never persisted, making the 5-attempt threshold unreachable. - Remove unused no_pr_key variable. - Remove HOMEBREW_PREFIX guard around PATH augmentation. The idempotent PATH check already prevents duplicates; the guard was overly restrictive for cron environments where HOMEBREW_PREFIX may be set without all tool paths present. Triaged all 4 unresolved review threads from PR #435: - 3 fixed (critical db bug, unused var, PATH guard) - 1 acknowledged won't-fix (json_extract counter reset is by design) Refs: GH#438, t147.1
1 participant
Summary
/opt/homebrew/bin,~/.local/bin, etc. This fixesgh/opencode/nodebeing unreachable from cron-triggered pulses, which caused allpr_reviewtasks to loop forever in ano_prretry cycle.no_prcase incmd_pr_lifecycleto prevent infinite loops. After 5 failures, task transitions toblockedwith diagnostic info (whetherghis in PATH).git ls-filesinstead offindto validate JSON configs intest-batch-quality-hardening.sh, excluding gitignored working copies that caused a false positive failure.Root Cause
Discovered during PR verification batch: cron jobs run with minimal PATH (
/usr/bin:/bin:/usr/sbin:/sbin). TheghCLI at/opt/homebrew/bin/ghwas unreachable, causingcheck_pr_status()to silently returnno_prfor every task. Theno_prhandler had no retry limit, creating an infinite loop across 69+ pulse cycles.Changes
.agents/scripts/supervisor-helper.shtests/test-batch-quality-hardening.shgit ls-filesfor JSON validation scopeTesting
shellcheck -S error supervisor-helper.sh-- 0 errorsbash -n supervisor-helper.sh-- syntax OKtest-batch-quality-hardening.sh-- 55/56 pass (1 expected diff: deployed vs repo version)Summary by CodeRabbit
Bug Fixes
Chores