feat: implement issue #538 — Title: pr-review-mention: kill the self-trigger — bot's own mention-ack re-fires the listener#544
Conversation
…trigger — bot's own mention-ack re-fires the listener
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 49 minutes and 11 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 (4)
✨ 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 |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
![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 Bats-based contract tests and helper functions to verify that the reusable workflow for PR review mentions does not self-trigger on its own comments or fire unexpected mention webhooks. The reviewer's feedback focuses on making the tests and helper functions more robust. Specifically, they suggest updating the awk parser in the setup helper to support alternative YAML block-scalar styles and single-line statements, and replacing strict string assertions in the Bats tests with regular expressions to prevent fragility against minor formatting or quoting changes.
| awk ' | ||
| /^[[:space:]]*if:[[:space:]]*\|/ { grab=1; next } | ||
| grab && /^[[:space:]]*steps:[[:space:]]*$/ { exit } | ||
| grab { print } | ||
| ' "$TT_REUSABLE" |
There was a problem hiding this comment.
The current awk parser is tightly coupled to the block-scalar syntax if: |. If the workflow is ever refactored to use a single-line if: statement or a different block-scalar indicator like >, this helper will fail to extract the guard condition.
We can make the parser more robust by:
- Supporting both
|and>block-scalar indicators. - Supporting single-line
if:statements by stripping theif:prefix and printing the condition directly.
awk '
/^[[:space:]]*if:[[:space:]]*[|>]/ { grab=1; next }
/^[[:space:]]*if:[[:space:]]+/ { sub(/^[[:space:]]*if:[[:space:]]*/, ""); print; next }
grab && /^[[:space:]]*steps:[[:space:]]*$/ { exit }
grab { print }
' "$TT_REUSABLE"
| @test "if-guard excludes comments authored by the bot itself" { | ||
| run prm_if_guard | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"github.event.comment.user.login != 'donpetry-bot'"* ]] |
There was a problem hiding this comment.
The assertion uses strict string matching with single quotes ('donpetry-bot'). If the workflow file is modified to use double quotes (e.g., "donpetry-bot") or if whitespace around != is adjusted, this test will fail.
Using a regular expression match (=~) makes the test much more resilient to minor formatting changes.
[[ "$output" =~ github\.event\.comment\.user\.login[[:space:]]*!=[[:space:]]*['"]donpetry-bot['"] ]]
| @test "if-guard excludes comments carrying a pr-review-agent marker" { | ||
| run prm_if_guard | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"!contains(github.event.comment.body, '<!-- pr-review-agent')"* ]] |
There was a problem hiding this comment.
Similar to the previous test, using strict string matching with single quotes makes the test fragile to quote style changes or whitespace adjustments. Using a regular expression match (=~) is more robust.
[[ "$output" =~ !contains\(github\.event\.comment\.body,[[:space:]]*['"]<!--[[:space:]]*pr-review-agent['"]\) ]]
| @test "if-guard still requires an @donpetry-bot mention to fire (trigger preserved)" { | ||
| run prm_if_guard | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *"contains(github.event.comment.body, '@donpetry-bot')"* ]] |
There was a problem hiding this comment.
Using a regular expression match (=~) instead of strict string matching prevents the test from breaking if the quote style or whitespace changes in the workflow file.
[[ "$output" =~ contains\(github\.event\.comment\.body,[[:space:]]*['"]@donpetry-bot['"]\) ]]
| @test "if-guard still gates on trusted author_association (trust gate preserved)" { | ||
| run prm_if_guard | ||
| [ "$status" -eq 0 ] | ||
| [[ "$output" == *'["OWNER","MEMBER","COLLABORATOR"]'* ]] |
There was a problem hiding this comment.
The strict string match for the JSON array ["OWNER","MEMBER","COLLABORATOR"] is fragile to formatting changes (such as adding spaces after commas or switching to single quotes). A regular expression match is more resilient.
[[ "$output" =~ \\[[[:space:]]*['"]OWNER['"][[:space:]]*,[[:space:]]*['"]MEMBER['"][[:space:]]*,[[:space:]]*['"]COLLABORATOR['"][[:space:]]*\\] ]]
2 participants
Closes #538
Implemented by dev-lead agent. Please review.