Repository navigation
fix(pr-review-mention): break the ack self-comment recursion - #553
Conversation
The handle-mention job posts an acknowledgement comment that itself contains '@donpetry-bot', via a PAT (DON_PETRY_BOT_GH_PAT) — and PAT-authored comments DO re-trigger workflows (unlike GITHUB_TOKEN). With no guard, each ack re-satisfies the comment trigger and posts another ack, looping every ~8s until GitHub's 2500-comment cap (observed live on .github-private#959: ~2500 acks, then a failure when commenting got disabled). Add a recursion guard to the comment branch of the job `if:`: skip any comment carrying our own ack marker `pr-review-agent mention-ack`. Match on the marker rather than the bot actor because donpetry-bot is also a human account whose own @-mention review requests must still fire. Pre-existing in all versions (the trigger logic is identical across the channel releases); surfaced by the #870 soak when the candidate ran on .github-private.
|
@donpetry-bot please review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
|
Warning Review limit reached
More reviews will be available in 45 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 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 |
|
@don-petry I'm on it — starting a fresh review now. Results will appear in a few minutes. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: d8ef3a9d3283680b57e35e232a9a2746cb6fca1b
Review mode: triage-approved (single reviewer)
Summary
Adds a one-line recursion guard to the comment branch of the handle-mention job's if: condition in pr-review-mention-reusable.yml, skipping any comment that carries the bot's own ack marker ('pr-review-agent mention-ack'). This breaks the PAT-authored self-comment loop that could run to GitHub's 2500-comment cap. The change is purely additive and strictly more restrictive.
Linked issue analysis
No closing issue is linked (closingIssuesReferences empty). The PR references combsmith/.github-private#870 for context (the soak that surfaced the bug); no issue requires substantive closure here.
Findings
No blocking findings.
- Correctness: the new clause is AND-ed into the existing condition, so it can only make the job fire LESS often — it can never start the job (or expose its secrets) in a case where it previously would not. Legitimate '@donpetry-bot please review' requests do not contain the ack marker, so they still fire; only the bot's own ack (which carries the marker) is skipped. Matching on the marker rather than the bot actor is correct, since donpetry-bot is also a human account whose own mentions must still trigger.
- Security: github.event.comment.body is used only inside contains() within the workflow if: expression — never interpolated into shell — so there is no script-injection exposure. The author_association OWNER/MEMBER/COLLABORATOR gate is preserved. No new permissions, secrets, or actions introduced.
- Secret scan: the run_secret_scanning MCP tool is not available in this environment; noted and skipped. The diff contains no secret material (workflow YAML guard only); gitleaks CI check passed.
- Style: clear explanatory comment added with the rationale; commit message and PR body document the why well.
CI status
All checks green or expected SKIPPED; no failures. SUCCESS: Lint, ShellCheck, Agent Security Scan, Secret scan (gitleaks), CodeQL, SonarCloud, agent-shield/AgentShield, dev-lead, pr-auto-review, npm audit, CodeRabbit. Expected SKIPPED: dependabot-automerge, pr-review-mention/handle-mention, unused ecosystem audits. mergeStateStatus is BLOCKED only because the org-leads team review is still required (REVIEW_REQUIRED) — unrelated to code health.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: LOW
Reviewed commit: d8ef3a9d3283680b57e35e232a9a2746cb6fca1b
Review mode: triage-approved (single reviewer)
Summary
Adds a one-line recursion guard to the comment branch of the handle-mention job's if: condition in pr-review-mention-reusable.yml, skipping any comment that carries the bot's own ack marker ('pr-review-agent mention-ack'), plus an explanatory comment. This breaks the PAT-authored self-comment loop that would otherwise run to GitHub's 2500-comment cap. The change is purely additive (+8/-0) and strictly more restrictive.
Linked issue analysis
No closing issue is linked (closingIssuesReferences is empty). The PR references combsmith/.github-private#870 for context (the soak that surfaced the bug); nothing requires substantive closure here.
Findings
No blocking findings. Triage assessment confirmed.
- Correctness: the new '!contains(... "pr-review-agent mention-ack")' clause is AND-ed into the comment (non-pull_request) branch only, so it can only make the job fire LESS often — it never starts the job (or exposes its secrets) where it previously would not, and the pull_request review-request branch is untouched. Legitimate '@donpetry-bot please review' comments lack the marker and still fire; only the bot's own ack (which carries the '' marker) is skipped. Matching on the marker rather than the bot actor is correct, since donpetry-bot is also a human account whose own mentions must still trigger.
- Security: github.event.comment.body is referenced only inside contains() within the workflow if: expression — never interpolated into a shell step — so there is no script-injection exposure. The author_association OWNER/MEMBER/COLLABORATOR gate is preserved. No new permissions, secrets, or actions are introduced.
- Secret scan: the run_secret_scanning MCP tool is not available in this environment; noted and skipped. The diff is workflow YAML (guard only) with no secret material; the gitleaks CI check passed.
- Minor (non-blocking): a legitimate comment that both mentions @donpetry-bot and happens to contain the literal string 'pr-review-agent mention-ack' would be suppressed — an acceptable and highly unlikely tradeoff.
CI status
All checks green or expected SKIPPED; no failures. SUCCESS: Lint, ShellCheck, Agent Security Scan, Secret scan (gitleaks), CodeQL (Analyze actions), SonarCloud, agent-shield/AgentShield, add-to-project, dev-lead, pr-auto-review, npm audit, CodeRabbit. Expected SKIPPED: dependabot-automerge, pr-review-mention/handle-mention, dev-lead/ci-relay, and the unused-ecosystem audits (pnpm/cargo/pip/govulncheck). reviewDecision is APPROVED; any merge block is the required org-leads team review, unrelated to code health.
Reviewed automatically by the PR-review agent (single-reviewer mode). Reply if you need a human review.
2 participants
Summary
pr-review-mention'shandle-mentionjob posts an acknowledgement comment that itself contains@donpetry-bot, via a PAT (DON_PETRY_BOT_GH_PAT). PAT-authored comments do re-trigger workflows (unlikeGITHUB_TOKEN), so with no guard each ack re-satisfies the comment trigger and posts another ack — looping every ~8s until GitHub's 2500-comment cap.Observed live on
petry-projects/.github-private#959during the #870 soak: ~2500 self-replies, then a failed run when commenting got disabled. (It manifests wheredonpetry-bot's commentauthor_association∈ OWNER/MEMBER/COLLABORATOR — i.e..github-private; on.githubthe acks don't re-trigger, which is why review-requests here never looped.)Fix
Add a recursion guard to the comment branch of the job
if::!contains(github.event.comment.body, 'pr-review-agent mention-ack') &&The ack always carries the HTML marker
<!-- pr-review-agent mention-ack -->, so this skips the bot's own acks. Matched on the marker, not the bot actor, becausedonpetry-botis also a human account whose own@donpetry-botreview requests must still fire.Notes
.github-private. Not a regression introduced by the candidate — but it meanspr-review-mentionv2.1.0 is not safe to promote as-is; this fix will be re-cut as a fresh candidate and re-soaked, while the other five reusables continue.Refs combsmith/.github-private#870.
🤖 Generated with Claude Code