fix: coderabbit pulse gh auth fails in cron (no keyring access)#1288
fix: coderabbit pulse gh auth fails in cron (no keyring access)#1288alex-solovyev merged 1 commit intomainfrom
Conversation
The cron pulse was failing every 2 minutes because coderabbit-pulse-helper.sh relied solely on 'gh auth status' which requires macOS keyring access. In cron environments, the keyring is unavailable even though supervisor-helper.sh correctly resolves GH_TOKEN from its cache file. Fix: check GH_TOKEN env var before falling back to gh auth status, matching the pattern already used by supervisor-helper.sh and issue-sync-helper.sh.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughThe shell script refactors function bodies with multiline local variable blocks and improved formatting throughout. Notably, GitHub API authentication handling is enhanced to accept GH_TOKEN environment variables, and the command runner introduces explicit option parsing for Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ 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 |
🔍 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: Thu Feb 12 19:52:17 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/coderabbit-pulse-helper.sh:
- Around line 139-141: The function run_gh_api_review currently only reads local
output_file="$1" but is called with three args (repo_path, output_file,
repo_id); change run_gh_api_review to accept explicit parameters (e.g., local
repo_path="$1" local output_file="$2" local repo_id="$3") and update all
references inside the function that rely on caller scope (uses of repo_id in the
body) to use the new local repo_id variable; ensure the call site passes the
three arguments in the correct order ("$repo_path" "$output_file" "$repo_id") so
the JSON write and other operations target the intended output path and repo id.
- Around line 74-85: The SSH remote parsing never runs because you overwrite
remote_url before checking its original form; preserve the original value (e.g.,
save remote_url to orig_remote or pre_strip) before you strip the .git suffix
into cleaned, then change the SSH-detection conditional to compare cleaned
against the preserved original (use the preserved var in the glob test instead
of "$remote_url") so the `${cleaned#*:}` branch executes for SSH URLs and
get_repo_id returns owner/repo.
🧹 Nitpick comments (1)
.agents/scripts/coderabbit-pulse-helper.sh (1)
147-153: Core fix looks correct — redundant redirect on line 150.The GH_TOKEN-first authentication check is the right approach for cron environments. However,
&>/dev/null 2>&1is redundant —&>/dev/nullalready redirects both stdout and stderr. The trailing2>&1is a no-op.✨ Remove redundant redirect
- if [[ -z "${GH_TOKEN:-}" ]] && ! gh auth status &>/dev/null 2>&1; then + if [[ -z "${GH_TOKEN:-}" ]] && ! gh auth status &>/dev/null; thenAs per coding guidelines, "Run ShellCheck with zero violations on all scripts in .agents/scripts/"
| # Extract owner/repo from various URL formats | ||
| # Remove .git suffix and protocol prefix, then get last two path segments | ||
| local cleaned | ||
| cleaned="${remote_url%.git}" | ||
| # Remove protocol (https://github.com/ or git@github.com:) | ||
| cleaned="${cleaned#*://*/}" # https://host/owner/repo -> owner/repo | ||
| if [[ "$cleaned" == "$remote_url"* ]]; then | ||
| # SSH format: git@host:owner/repo | ||
| cleaned="${cleaned#*:}" | ||
| fi | ||
| echo "$cleaned" | ||
| return 0 |
There was a problem hiding this comment.
Bug: SSH remote URL is never correctly parsed to owner/repo.
Line 80 compares $cleaned (.git already stripped) against $remote_url (still contains .git), so the glob "$remote_url"* can never match when the URL was SSH-format. The SSH-specific ${cleaned#*:} cleanup on line 82 is therefore dead code, and get_repo_id will return git@github.com:owner/repo for SSH remotes — breaking every downstream gh command that expects owner/repo.
Save the pre-removal value and compare against that instead:
🐛 Proposed fix
# Remove protocol (https://github.com/ or git@github.com:)
- cleaned="${cleaned#*://*/}" # https://host/owner/repo -> owner/repo
- if [[ "$cleaned" == "$remote_url"* ]]; then
+ local cleaned_before="$cleaned"
+ cleaned="${cleaned#*://*/}" # https://host/owner/repo -> owner/repo
+ if [[ "$cleaned" == "$cleaned_before" ]]; then
# SSH format: git@host:owner/repo
cleaned="${cleaned#*:}"
fi🤖 Prompt for AI Agents
In @.agents/scripts/coderabbit-pulse-helper.sh around lines 74 - 85, The SSH
remote parsing never runs because you overwrite remote_url before checking its
original form; preserve the original value (e.g., save remote_url to orig_remote
or pre_strip) before you strip the .git suffix into cleaned, then change the
SSH-detection conditional to compare cleaned against the preserved original (use
the preserved var in the glob test instead of "$remote_url") so the
`${cleaned#*:}` branch executes for SSH URLs and get_repo_id returns owner/repo.
| run_gh_api_review() { | ||
| local output_file="$1" | ||
|
|
||
| if ! command -v gh &>/dev/null; then | ||
| print_error "Neither CodeRabbit CLI nor gh CLI available" | ||
| return 1 | ||
| fi | ||
|
|
||
| # Check if gh is authenticated | ||
| if ! gh auth status &>/dev/null 2>&1; then | ||
| print_error "gh CLI not authenticated" | ||
| return 1 | ||
| fi | ||
|
|
||
| print_info "Triggering CodeRabbit review via GitHub API..." | ||
|
|
||
| # Strategy: Find or create a tracking issue for daily reviews | ||
| local tracking_label="coderabbit-pulse" | ||
| local tracking_issue | ||
|
|
||
| # Look for existing open tracking issue | ||
| tracking_issue=$(gh issue list --repo "$repo_id" --label "$tracking_label" --state open --json number --jq '.[0].number // empty' 2>/dev/null || echo "") | ||
|
|
||
| if [[ -z "$tracking_issue" ]]; then | ||
| # Create the tracking label if it doesn't exist | ||
| gh label create "$tracking_label" --repo "$repo_id" --description "Daily CodeRabbit pulse review tracking" --color "7057ff" 2>/dev/null || true | ||
|
|
||
| # Create tracking issue (gh issue create returns URL, extract number) | ||
| local issue_url | ||
| issue_url=$(gh issue create --repo "$repo_id" \ | ||
| --title "Daily CodeRabbit Pulse Review" \ | ||
| --body "This issue tracks daily full codebase reviews by CodeRabbit. | ||
| local output_file="$1" | ||
|
|
There was a problem hiding this comment.
Bug: run_gh_api_review receives wrong arguments — output_file gets the repo path.
run_gh_api_review declares only one parameter (output_file="$1", line 140), but the call site on line 313 passes three arguments: "$repo_path" "$output_file" "$repo_id". This means $1 inside the function is the repo path, not the output file. The JSON written on line 205 will target the wrong path.
Additionally, $repo_id is used throughout the function body (lines 162, 166, 170, 176, 201) via dynamic scoping from the caller rather than as an explicit parameter — this is fragile.
🐛 Proposed fix — accept all needed parameters explicitly
run_gh_api_review() {
- local output_file="$1"
+ local repo_path="$1"
+ local output_file="$2"
+ local repo_id="$3"Or, fix the call site to match the current single-parameter signature:
- elif run_gh_api_review "$repo_path" "$output_file" "$repo_id"; then
+ elif run_gh_api_review "$output_file"; thenIf choosing the second option, document that $repo_id is expected from the caller's scope, though the first option (explicit params) is strongly preferred for maintainability.
As per coding guidelines, "Use local var="$1" pattern in shell scripts"
Also applies to: 313-313
🤖 Prompt for AI Agents
In @.agents/scripts/coderabbit-pulse-helper.sh around lines 139 - 141, The
function run_gh_api_review currently only reads local output_file="$1" but is
called with three args (repo_path, output_file, repo_id); change
run_gh_api_review to accept explicit parameters (e.g., local repo_path="$1"
local output_file="$2" local repo_id="$3") and update all references inside the
function that rely on caller scope (uses of repo_id in the body) to use the new
local repo_id variable; ensure the call site passes the three arguments in the
correct order ("$repo_path" "$output_file" "$repo_id") so the JSON write and
other operations target the intended output path and repo id.
2 participants
Summary
coderabbit-pulse-helper.shfailing every 2 minutes in cron because it relied solely ongh auth status(requires macOS keyring, unavailable in cron)GH_TOKENenv var before falling back togh auth status, matching the pattern already used bysupervisor-helper.shandissue-sync-helper.shRoot Cause
The supervisor's GH_TOKEN resolution chain (env > cache > gh auth token > gopass > credentials.sh) correctly populates
GH_TOKENand caches it. Butcoderabbit-pulse-helper.shwas the only script that didn't checkGH_TOKENbefore callinggh auth status, causing it to fail in cron where the macOS keyring is inaccessible.Summary by CodeRabbit
Refactor
New Features