feat: implement issue #453 — Compliance: ruleset-drift-pr-quality-dismiss_stale_reviews_on_push - #455
feat: implement issue #453 — Compliance: ruleset-drift-pr-quality-dismiss_stale_reviews_on_push#455don-petry wants to merge 2 commits into
Conversation
…miss_stale_reviews_on_push
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
🤖 CodeAnt AI — Review Status
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Review limit reached
Next review available in: 54 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR documents ChangesRuleset reconciliation
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change requires fresh approvals after updates while preserving existing repository settings; no actionable merge-blocking risk remains at the current head. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
codeant-ai
Bot
added
the
size:M
![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 refactors the repository settings reconciliation script by extracting the payload generation logic into a standalone, testable function pr_quality_reconcile_payload and adding corresponding unit tests. The review feedback highlights two main improvements: first, avoiding hardcoded values in the new function by utilizing the script's existing constants to prevent potential infinite reconciliation loops; second, defensively wrapping command substitutions in the test script within conditional blocks to ensure robust error handling under strict shell execution modes.
| pr_quality_reconcile_payload() { | ||
| local json="${1:-}" | ||
| printf '%s' "$json" | jq \ | ||
| --arg method "$PR_QUALITY_MERGE_METHOD" ' | ||
| { | ||
| name: .name, | ||
| target: .target, | ||
| enforcement: .enforcement, | ||
| bypass_actors: (.bypass_actors // []), | ||
| conditions: .conditions, | ||
| rules: [ | ||
| (.rules // [])[] | ||
| | if .type == "pull_request" | ||
| then .parameters.allowed_merge_methods = [$method] | ||
| | .parameters = ((.parameters // {}) + {require_last_push_approval: true, dismiss_stale_reviews_on_push: true}) | ||
| else . end | ||
| ] | ||
| }' | ||
| } |
There was a problem hiding this comment.
The pr_quality_reconcile_payload function currently hardcodes require_last_push_approval: true and dismiss_stale_reviews_on_push: true inside the jq filter. This bypasses the PR_QUALITY_REQUIRE_LAST_PUSH_APPROVAL and PR_QUALITY_DISMISS_STALE_REVIEWS constants defined at the top of the script. If those constants are ever modified, the drift detection will trigger an update, but the generated payload will still apply the hardcoded true values, causing an infinite reconciliation loop.
Additionally, we should add a defensive check to ensure the function returns an error if the input JSON is empty, and use defensive defaults in jq to handle potentially null or missing fields.
pr_quality_reconcile_payload() {
local json="${1:-}"
if [[ -z "$json" ]]; then
return 1
fi
printf '%s' "$json" | jq \
--arg method "$PR_QUALITY_MERGE_METHOD" \
--argjson rlpa "$PR_QUALITY_REQUIRE_LAST_PUSH_APPROVAL" \
--argjson ds "$PR_QUALITY_DISMISS_STALE_REVIEWS" '
{
name: .name,
target: .target,
enforcement: .enforcement,
bypass_actors: (.bypass_actors // []),
conditions: .conditions,
rules: [
(.rules // [])[]
| if .type == "pull_request"
then .parameters.allowed_merge_methods = [$method]
| .parameters = ((.parameters // {}) + {require_last_push_approval: $rlpa, dismiss_stale_reviews_on_push: $ds})
else . end
]
}'
}References
- When using
jqto modify or iterate over JSON fields that might be null or missing, use defensive defaults (e.g.,// []or// {}) to preventjqfrom throwing errors like 'Cannot use null as object'.
There was a problem hiding this comment.
Fixed in scripts/apply-repo-settings.sh: replaced the two hardcoded true literals in pr_quality_reconcile_payload's jq filter with --argjson rlpa "$PR_QUALITY_REQUIRE_LAST_PUSH_APPROVAL" and --argjson ds "$PR_QUALITY_DISMISS_STALE_REVIEWS", so the payload always reflects the constants and cannot drift from them. Also added an empty-input guard ([[ -z "$json" ]] && return 1) and a corresponding test in the test script.
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
|
CI checks on this PR are still running. Once they complete, re-mention Posted by the donpetry-bot PR-review cascade. |
|
Advisory bots were rate-limited; auto-approval is withheld until they recover. pr-review-sweep will re-review this PR after 2026-08-21T14:24:36Z. |
Dev-Lead — fix-bot-comment (no-changes)Agent reasoning |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 97fef1b979bb4f6a12cd8edb1bfce296cdeac185
Cascade: triage → deep (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5)
Summary
Clean refactor of scripts/apply-repo-settings.sh extracting the pr-quality ruleset PUT-payload builder into a pure, unit-tested pr_quality_reconcile_payload function, plus adding dismiss_stale_reviews_on_push to the reconciled settings (a branch-protection hardening). The triage-escalating Gemini high-priority flag ('hardcodes require_last_push_approval/dismiss_stale_reviews_on_push, risking an infinite reconciliation loop') is a resolved false positive: at the current head 97fef1b the payload is parameterized via --argjson from the same PR_QUALITY_* constants the drift detector reads, so there is no value divergence and no loop; Gemini reviewed an earlier state. Downstream impact: (none).
Findings
- INFO: Gemini's high-priority 'infinite reconciliation loop' concern is not present at head 97fef1b. pr_quality_reconcile_payload (apply-repo-settings.sh:205-212) applies require_last_push_approval and dismiss_stale_reviews_on_push via --argjson rlpa/$ds bound to PR_QUALITY_REQUIRE_LAST_PUSH_APPROVAL and PR_QUALITY_DISMISS_STALE_REVIEWS ('true'), the exact constants the drift-status functions compare against (lines 283-293). Detector and writer share one source of truth, so no drift/apply divergence.
- INFO: 10 new unit tests cover drift on every reconciled parameter, preservation of name/target/enforcement/conditions/bypass_actors and non-pull_request rules, absent-bypass_actors default, and non-zero exit on empty input. shellcheck clean; suite passes in a clean env and the CI 'review' check is green. (A local resolve_repo test 'failure' was env contamination from the reviewer runner's GITHUB_REPOSITORY, unrelated to this PR.)
- INFO: Change is security-adjacent (reconciles pr-quality branch-protection ruleset) but strengthens posture: dismiss_stale_reviews_on_push=true drops approvals on new pushes. jq is fed GitHub-API ruleset JSON via stdin with typed --argjson bindings; no shell/jq injection, no secrets. run_secret_scanning MCP tool not available in this environment; gitleaks CI passed and diff contains no secret material.
Reviewed by the PR-review cascade (triage: haiku 4.5 → deep: opus 4.8 + duck: o4-mini → audit: fable 5). Reply if you need a human review.
2 participants
User description
Closes #453
Implemented by dev-lead agent. Please review.
CodeAnt-AI Description
Require fresh approvals after code changes and preserve repository rules during reconciliation
What Changed
Impact
✅ Fresh approvals after every code update✅ Fewer merges of unreviewed changes✅ Preserved repository rules during settings updates💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Documentation
Improvements
Tests