Skip to content

feat: implement issue #338 — Compliance: ruleset-drift-pr-quality-require_code_owner_review - #354

Closed
don-petry wants to merge 2 commits into
mainfrom
dev-lead/issue-338-20260717-1402
Closed

feat: implement issue #338 — Compliance: ruleset-drift-pr-quality-require_code_owner_review#354
don-petry wants to merge 2 commits into
mainfrom
dev-lead/issue-338-20260717-1402

Conversation

@don-petry

Copy link
Copy Markdown
Contributor

Closes #338

Implemented by dev-lead agent. Please review.

Copilot AI review requested due to automatic review settings July 17, 2026 14:06
@don-petry
don-petry requested a review from a team as a code owner July 17, 2026 14:06
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@don-petry, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 reviews.

How do review 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 refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 69f651d8-7a72-4b75-9fc8-1ae264754668

📥 Commits

Reviewing files that changed from the base of the PR and between f2ff479 and bcfeb91.

📒 Files selected for processing (2)
  • scripts/setup-rulesets.sh
  • scripts/tests/setup-rulesets.bats
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev-lead/issue-338-20260717-1402

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new pr-quality ruleset configuration in scripts/setup-rulesets.sh to enforce pull-request quality gates on the default branch, along with a comprehensive BATS test suite in scripts/tests/setup-rulesets.bats to validate the ruleset payloads. Feedback highlights critical API validation issues: the OrganizationAdmin bypass actor is invalid for repository-level rulesets and should be replaced with RepositoryRole (ID 5), and the allowed_merge_methods parameter is unsupported under the pull_request rule and must be removed. Additionally, the corresponding test assertions should be updated to reflect these changes and utilize safe JSON navigation to prevent unhandled exceptions.

Comment thread scripts/setup-rulesets.sh
Comment thread scripts/setup-rulesets.sh Outdated
Comment thread scripts/tests/setup-rulesets.bats Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (applied)

Changes committed and pushed.

@don-petry
don-petry enabled auto-merge (squash) July 17, 2026 14:47
@sonarqubecloud

Copy link
Copy Markdown

@don-petry
don-petry disabled auto-merge July 17, 2026 14:52
@don-petry

Copy link
Copy Markdown
Contributor Author

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning
Issues addressed: 0
- Quality Gate Passed: no new issues identified
Files changed: none
Skipped (informational): 0
No action required. All checks passing, zero Tier 1 blockers.
```
The SonarCloud quality gate has passed with no actionable issues. While `gemini-code-assist[bot]` provided feedback on the implementation (in COMMENTED state, not CHANGES_REQUESTED), it is neither from the specified bot nor a Tier 1 blocker, so no thread resolution is needed per the constraints.

@don-petry
don-petry enabled auto-merge (squash) July 17, 2026 14:53
@donpetry-bot

Copy link
Copy Markdown
Contributor

Review — fix requested (cycle 1/3)

The automated review identified the following issues. Please address each one:

Findings to fix

Automated review — NEEDS HUMAN REVIEW

Risk: MEDIUM
Reviewed commit: bcfeb915bdee284970b78aaa76897848159a8474
Review mode: triage-approved (single reviewer)

Summary

Core fix is correct (require_code_owner_review=true, addressing #338) with a solid BATS suite, but the fix-bot commit removed allowed_merge_methods based on an incorrect bot claim — the live repo-level ruleset carries that parameter, so applying this payload would silently drop the squash-only merge restriction and re-introduce drift from the codified standard.

Linked issue analysis

Issue #338 (compliance drift: pr-quality ruleset require_code_owner_review expected true, actual false) is substantively addressed — the new payload sets require_code_owner_review: true and the BATS tests pin it, plus the other standard parameters. However, the issue names standards/rulesets/pr-quality.json (petry-projects/.github) as the source of truth, and the payload in this PR deviates from it in two places (see findings), so the compliance audit may flag new drift after this script runs.

Findings

  1. [MEDIUM — blocking] allowed_merge_methods: ["squash"] was incorrectly removed. The fix commit (bcfeb91) dropped it in response to gemini-code-assist's claim that the parameter is invalid for the pull_request rule. That claim is wrong: the live repo-level pr-quality ruleset on this repo (id 14592515, source_type: Repository) currently contains "allowed_merge_methods": ["squash"] under its pull_request rule, proving the parameter is accepted by the repository rulesets API. Since the script PUTs the full payload over the existing ruleset, applying it would silently drop the squash-only merge restriction — weakening protection relative to both the current live state and the codified standard, and likely triggering a new parameter-drift finding in the next weekly audit. Restore "allowed_merge_methods": ["squash"] to the payload and re-add the corresponding test assertion.
  2. [LOW] bypass_actors deviate from the codified standard. The standard specifies OrganizationAdmin (no actor_id); the payload uses RepositoryRole with actor_id: 5 (repository admin). If OrganizationAdmin is genuinely rejected by the repo-level API, the deviation should be verified and documented; otherwise mirror the standard verbatim. Relatedly, the inline comment says the bypass mirrors "the org admin role", which no longer matches the code (repository admin role).
  3. [INFO] Resolved prior feedback. All three gemini-code-assist review threads are marked resolved, and the invalid-OrganizationAdmin fix (finding-adjacent) plus test updates were applied in bcfeb91. The secret-scanning MCP tool was unavailable in this run; gitleaks CI passed and manual inspection found no secrets in the diff.

CI status

All validation checks green: CI (Lint, Format, Test, Secret scan/gitleaks), CodeQL, SonarCloud quality gate (0 new issues, 0 hotspots), AgentShield, dependency-audit (ecosystem jobs skipped — none detected), CodeRabbit status SUCCESS. The two CANCELLED dev-lead check runs are agent-orchestration jobs cancelled by [skip ci-relay], not validation CI. Merge state is BLOCKED only on the required review.


Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

Additional tasks

  1. Resolve all unresolved review thread comments from other reviewers
  2. Ensure all CI checks pass after your changes
  3. Rebase on the target branch if behind
  4. Do NOT modify files unrelated to the findings above

The review cascade will automatically re-review after new commits are pushed.

@don-petry

Copy link
Copy Markdown
Contributor Author

Closing as part of the open-PR drain (50-PR automation cap). This branch has real merge conflicts with main and cannot be rebased automatically; the linked compliance issue remains OPEN so dev-lead can regenerate a clean PR once there is cap headroom. No work is lost.

@don-petry don-petry closed this Jul 21, 2026
auto-merge was automatically disabled July 21, 2026 03:14

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compliance: ruleset-drift-pr-quality-require_code_owner_review

3 participants