t1388: fix pulse external-contributor guard fails closed on API errors#2804
t1388: fix pulse external-contributor guard fails closed on API errors#2804alex-solovyev wants to merge 0 commit intomainfrom
Conversation
|
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 ID: 📒 Files selected for processing (1)
✨ 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, 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 significantly enhances the robustness of the automated pulse script by addressing critical issues related to GitHub API failures. The primary goal is to prevent the script from posting redundant comments or taking incorrect actions when API calls encounter errors, ensuring more reliable and idempotent behavior for PR labeling and commenting. 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
|
There was a problem hiding this comment.
Code Review
The pull request effectively addresses the root cause of duplicate external-contributor comments by implementing a robust "fail closed" strategy for API calls. By fetching comments into variables and checking exit codes, the script now correctly handles API failures, preventing redundant comments. The change from 2>/dev/null to 2>&1 for gh api calls also significantly improves debuggability by ensuring that errors are visible in logs. This is a solid improvement in the reliability and maintainability of the pulse script.
Note: Security Review has been skipped due to the limited scope of the PR.
.agents/scripts/commands/pulse.md
Outdated
| elif gh pr view <number> --repo <slug> --json comments --jq '.comments[].body' 2>&1 | grep -qiF 'external contributor'; then | ||
| # Comment already exists but label is missing — re-add the label only | ||
| gh api "repos/<slug>/issues/<number>/labels" -X POST -f 'labels[]=external-contributor' 2>/dev/null || true | ||
| continue # or skip to next PR |
There was a problem hiding this comment.
The continue statement on line 94 is typically used within a loop (for, while, until) to skip the rest of the current iteration. In this code snippet, which appears to be a standalone example within the markdown documentation, continue would result in a syntax error or unexpected behavior if not enclosed in a loop.
If this snippet is intended to be part of a larger script that iterates over pull requests, consider showing the loop context for clarity in the documentation. Otherwise, if this is a standalone conditional block, continue should be replaced with an appropriate action like exit 0 or simply ending the block, or a comment indicating the intended flow.
| continue # or skip to next PR | |
| : # Already labelled -- skip | |
| # Continue to the next PR in a loop context, or exit if standalone. |
|
Reopened by pulse supervisor. This PR was closed without merging at 2026-03-04T03:31Z — likely by the pulse itself (a separate bug). All CI checks passed. The fix is correct and addresses the root cause of 18 duplicate comments on PR #2792. Closes #2802. Also addresses #2805 (duplicate issue filed before discovering this PR existed). |
179e055 to
16aaafe
Compare
Summary
gh pr view | grep) that silently returned "no match" on API failure, falling through to post another comment every 2-minute pulse cycle (18 duplicates on PR feat: add memory pressure monitor with launchd integration #2792)2>/dev/nullwith2>&1on label-add operations so failures are visible in logsRoot Cause
The prior fix (#2796) addressed
2>/dev/nullon the label check but left the comment check vulnerable. Theelifbranch used:When
gh pr viewfails (rate limit, network error), the pipe sends the error message togrep, which doesn't match "external contributor", so the code falls through to theelsebranch and posts yet another duplicate comment.Fix
$comment_exit -ne 0), skip posting — assume already flagged. The next pulse cycle (2 min later) retries when the API recoversCloses #2802