feat(rulesets): retire detection-based apply-rulesets.sh; codified applier canonical in .github (#580)#590
Conversation
…d file-driven applier canonical in .github (#580) Implements the user's #580 decision (Option 1 — retire, not annotate). Supersedes the Option-2 PR #583. scripts/apply-rulesets.sh — REPLACE the detection-based builder (which probed each repo's workflows and *generated* the ruleset in-code, diverging from the codified set — e.g. injecting `Dev-Lead Agent`) with the codified file-driven applier: - reads the local standards/rulesets/*.json source of truth (co-located in .github), - --repo owner/repo | <repo-name> (back-compat positional) | --all | --dry-run, optional [<name>...] ruleset filter, - find-or-create per ruleset (PUT by id / POST), idempotent convergence. Net -400 lines (the detect_required_checks / build_ruleset_json machinery is gone). "All detection items in the file-driven approach" (nothing org-wide lost): SonarCloud, CodeQL, agent-shield/AgentShield, dependency-audit/Detect ecosystems → carried statically in code-quality.json. Dev-Lead Agent → intentionally excluded (the retired divergence). CI Pipeline (build-and-test) → repo-specific → per-repo branch protection. Secret scan (gitleaks) + coverage → template-produced; staged fleet-wide via #581. Tests + CI (salvages #583's divergence-guard intent, against the codified JSON): test/scripts/apply-rulesets/apply-rulesets.bats (13 tests incl. the "no Dev-Lead Agent context" guard) + .github/workflows/apply-rulesets-tests.yml (shellcheck+bats). Reference reconciliation (detection language → codified model): - github-settings.md: intro + Required-Check preamble + SonarCloud row (was detection-derived `SonarCloud Analysis / SonarCloud`; codified is `SonarCloud`). - ci-standards.md §CodeQL, push-protection.md step 4, compliance-audit.sh remediation message, ruleset-remediation-runbook.md. - pull-request-limits-adr.md: dated correction note (the "no static JSON / generated in-code" premise no longer holds; §7.2/7.3 conclusion unaffected). Verified: shellcheck + yamllint + actionlint + markdownlint clean; a live --dry-run against petry-projects/.github-private resolves the LOCAL standards/rulesets/ and updates the existing rulesets in place (PUT by id, no recreate). Remaining follow-up (noted, not in scope): consolidate .github-private's bootstrap applier (it now reads the same codified source via fetch, so no divergence — the full single-physical-applier "cross-repo bootstrap story" is a separate step). Closes #580. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (9)
✨ 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 replaces the dynamic, detection-based ruleset builder in apply-rulesets.sh with a codified, file-driven approach that reads static JSON rulesets from standards/rulesets/ as the single source of truth. It also updates associated documentation and adds comprehensive BATS unit tests. The review feedback highlights critical error-handling improvements in the shell script, specifically recommending that exit statuses of gh CLI commands and ruleset applications are captured atomically and propagated correctly to prevent silent failures.
There was a problem hiding this comment.
Pull request overview
This PR implements the #580 “retire” decision by replacing the detection-based scripts/apply-rulesets.sh with a codified, file-driven ruleset applier that reads standards/rulesets/*.json as the source of truth, and reconciles org documentation/messaging to match the new model.
Changes:
- Replaced the legacy detection/generation logic in
scripts/apply-rulesets.shwith an idempotent “read JSON → create/update by name/id” applier supporting--repo,--all,--dry-run, and optional ruleset-name filtering. - Added CI + bats coverage for the applier, including a guard that
code-quality.jsondoes not require Dev-Lead Agent. - Updated standards docs and audit messaging to remove references to detection-based required-check derivation and reflect the codified contexts.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/apply-rulesets.sh |
Replaces detection-based ruleset generation with a codified JSON-driven create/update applier and new CLI interface. |
test/scripts/apply-rulesets/apply-rulesets.bats |
Adds bats tests for create/update/dry-run behavior and anti-divergence checks against codified rulesets. |
.github/workflows/apply-rulesets-tests.yml |
Adds CI workflow to run ShellCheck and bats for the applier/tests. |
standards/github-settings.md |
Updates required-checks documentation to reference codified code-quality.json instead of detection logic and reconciles check-name guidance. |
standards/ci-standards.md |
Updates CodeQL check-name description to reference codified ruleset contexts. |
standards/push-protection.md |
Updates secret-scan rollout instructions to modify codified ruleset JSON (sequenced after fleet backfill). |
standards/ruleset-remediation-runbook.md |
Clarifies that apply-rulesets full-replaces from codified JSON source of truth. |
scripts/compliance-audit.sh |
Updates audit finding text to reference codified ruleset JSON as the canonical context list. |
docs/initiatives/pull-request-limits-adr.md |
Adds a dated correction noting the ADR premise changed (rulesets are now codified and applied file-driven). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Dev-Lead — fix-bot-comment (applied)Changes committed and pushed. |
|
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: MEDIUM
Reviewed commit: 925021b072d3932d0d45fd77821893d143b6e3cc
Review mode: triage-approved (single reviewer)
Summary
Replaces the detection-based apply-rulesets.sh with a codified, file-driven applier reading standards/rulesets/*.json (net −40 lines, −398/+358 across 9 files), implementing the owner's #580 Option-1 (retire) decision and superseding #583. Adds 13 bats tests incl. an anti-divergence guard, a minimal-permission SHA-pinned test workflow, and reconciles 5 docs/scripts from detection language to the codified model.
Linked issue analysis
Linked issue #580 (closes): owner decided Option 1 — retire the detection-based applier, ensure all detection items are covered by the file-driven approach, and keep it in the .github repo. All three conditions are met: (1) detect_required_checks()/in-code generation is fully removed; (2) the applier header + github-settings.md document the detection→codified mapping — SonarCloud/CodeQL/AgentShield/dependency-audit are static in code-quality.json, Dev-Lead Agent is intentionally excluded (per #579, the divergence being retired, with a bats guard enforcing it), CI Pipeline stays per-repo, gitleaks+coverage staged via #581; (3) the applier remains at scripts/apply-rulesets.sh in .github.
Findings
No blocking findings.
- Script correctness verified under set -euo pipefail: command substitutions are rc-guarded, apply_repo accumulates per-file failures, --all accumulates per-repo failures and reports a count; fast-fail gh auth status added.
- New workflow apply-rulesets-tests.yml: top-level permissions: {}, job contents: read, actions/checkout pinned by SHA, concurrency + timeout set — no Actions security smells; it ran green on this PR.
- All 8 prior review threads (gemini-code-assist critical/high error-handling, copilot set -e/auth findings) are resolved and addressed in the code.
- Docs reconciliation (github-settings.md SonarCloud context, push-protection.md step 4, remediation runbook, compliance-audit.sh message, pr-limits ADR update note) is consistent with the codified model.
- Secret-scanning MCP tool unavailable in this run — relying on the green 'Secret scan (gitleaks)' CI check; no credentials or tokens appear in the diff.
CI status
All checks green: agent-shield, CodeQL, SonarCloud (+ quality gate passed, 0 new issues), ShellCheck, Secret scan (gitleaks), npm audit, Detect ecosystems, and the new 'Lint and bats' job (shellcheck + 13 bats tests). Skipped checks (pnpm/cargo/pip audits, govulncheck, dependabot-automerge, dev-lead ci-relay) are expected conditional jobs.
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: MEDIUM
Reviewed commit: 925021b072d3932d0d45fd77821893d143b6e3cc
Review mode: triage-approved (single reviewer)
Summary
Implements the #580 'retire' decision: replaces the detection-based apply-rulesets.sh (which probed repo workflows and generated the code-quality ruleset in-code, causing the Dev-Lead Agent divergence) with a codified, file-driven applier that reads standards/rulesets/*.json and idempotently PUTs/POSTs them. Adds 13 bats tests (incl. the anti-divergence guard) and a dedicated shellcheck+bats CI workflow, and reconciles five standards/docs references from detection language to the codified model.
Linked issue analysis
Linked issue #580 (closed by this PR) asked to retire or delineate the detection-based applier because it diverged from the codified standards/rulesets/code-quality.json (it injected 'Dev-Lead Agent / dev-lead' as a required context). This PR implements Option 1 (retire) as decided: the detection builder is removed, the codified JSON is the single source of truth, every detection item is accounted for in the PR's mapping table, and the referenced docs (compliance-audit.sh message, ci-standards.md, github-settings.md, push-protection.md, ruleset-remediation-runbook.md, pull-request-limits-adr.md) are reconciled. Substantively addressed.
Findings
No blocking findings.
- Triage-approved confirmation: the triage assessment is correct; nothing missed.
- All 8 review threads (gemini-code-assist, Copilot — set -e error-handling in the rewritten script) are resolved; the final script guards every gh call and command substitution and accumulates per-file/per-repo failures correctly.
- New workflow .github/workflows/apply-rulesets-tests.yml follows org standards: top-level 'permissions: {}', job-scoped 'contents: read', concurrency with cancel-in-progress, 10-min timeout, and actions/checkout pinned to 9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 — verified via the GitHub API as the genuine v7.0.0 tag.
- Bats tests stub gh on PATH and assert create (POST), update (PUT by id), dry-run (no writes), bare-name back-compat, name filtering, fleet mode, and the 'code-quality.json does NOT require Dev-Lead Agent' anti-divergence guard.
- standards/rulesets/{code-quality,pr-quality}.json exist at the head SHA, so the applier's source of truth is present.
- Secret scan: the run_secret_scanning MCP tool is unavailable in this environment; the gitleaks CI check passed and the diff contains no credential-like content.
- Prior cascade review at this same SHA also approved at MEDIUM risk; no changes since.
CI status
All required and informational checks green at 925021b: Apply Rulesets Tests (Lint and bats), agent-shield / AgentShield, CodeQL, SonarCloud (quality gate passed), Secret scan (gitleaks), ShellCheck, dependency-audit / Detect ecosystems, npm audit, Agent Security Scan. Ecosystem-gated audit jobs and dependabot-automerge skipped as expected.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Summary
Implements your #580 decision — Option 1 (retire), not the annotate that dev-lead delivered. Supersedes #583.
Replaces the detection-based
apply-rulesets.sh(which probed each repo's workflows and generated the ruleset in-code — the source of theDev-Lead Agentdivergence) with the codified, file-driven applier reading thestandards/rulesets/*.jsonsource of truth. Net −400 lines.The new applier
standards/rulesets/(co-located in.github).--repo owner/repo·<repo-name>(back-compat positional) ·--all·--dry-run· optional[<name>…]filter."Ensure all detection items are in the file-driven approach" — nothing org-wide is lost
code-quality.jsonbuild-and-test)Documented in the applier header +
github-settings.md.Tests + CI (salvages #583's guard)
test/scripts/apply-rulesets/apply-rulesets.bats— 13 tests, incl. a "code-quality.json does NOT require Dev-Lead Agent" guard (the anti-divergence check, now against the codified source rather than the retired script)..github/workflows/apply-rulesets-tests.yml— shellcheck + bats.Reference reconciliation (detection language → codified model)
github-settings.md(intro, preamble, SonarCloud row — was detection-derivedSonarCloud Analysis / SonarCloud, codified isSonarCloud),ci-standards.md §CodeQL,push-protection.mdstep 4,compliance-audit.shremediation message,ruleset-remediation-runbook.md, and a dated correction note inpull-request-limits-adr.md(its "no static JSON / generated in-code" premise no longer holds; the §7.2/7.3 conclusion is unaffected).Verified
--dry-runagainstpetry-projects/.github-privateresolves the localstandards/rulesets/and updates the existing rulesets in place (PUT by id, no recreate).Follow-up (noted, out of scope)
.github-privatestill has its own file-driven applier (#1013) for bootstrap +release-channel-tags. It reads the same codified source, so there's no divergence — but collapsing to a single physical applier (the cross-repo "bootstrap story" #583 punted on) is a separate step.Closes #580.
🤖 Generated with Claude Code