Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 37 minutes. View limit details
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
left a comment
There was a problem hiding this comment.
Reviewed the full diff and ran the suite locally (69/69 bats passing, shellcheck --severity=warning clean). The core approach — codifying deletion/non_fast_forward, checking parameterless rules, GitHub CODEOWNERS precedence, and inline-comment stripping — is sound. I left 5 line comments: 2 false-positive risks in the new CODEOWNERS tokenization (CRLF, escaped-space patterns), 1 reachability gap in the new parameterless-ruleset check, 1 standard/doc drift for the new catch-all rule, and 1 nit on team-slug validation. None block the ruleset JSON change itself.
…ty, and standard docs
|
don-petry
left a comment
There was a problem hiding this comment.
Re-reviewed the full diff at 59c22904 across all 7 files and verified locally (71/71 bats tests passing, shellcheck --severity=warning -x clean):
scripts/compliance-audit.sh(_ruleset_contents_one): Parameterless rule drift check (deletion,non_fast_forward) now runs immediately after live ruleset fetch/validation and before thehas_rule != trueearly return, ensuring all missing codified rules are reported in a single run.scripts/compliance-audit.sh(check_codeowners): Probes.github/CODEOWNERS→CODEOWNERS→docs/CODEOWNERSin GitHub precedence order, strips\r(CRLF) and inline#comments, normalizes\escaped spaces in path patterns beforeawkfield splitting, validates owner tokens against^@${ORG}/[a-z0-9-]+$, and rejects extra teams on the*catch-all line.standards/codeowners-standard.md,standards/rulesets/pr-quality.json,standards/rulesets/README.md: Documentation and codified ruleset JSON are aligned with the audit checks.- Tests (
apply-rulesets.bats,codeowners-catchall.bats,ruleset-contents-drift.bats): All 5 review items have dedicated regression test coverage (71/71 passing).
LGTM — ready for merge.



Summary
Centralizes the
CODEOWNERSandpr-qualityruleset compliance checks incombsmith/.githubso downstream repos (such ascombsmith/bmad-bgreat-suite) can retire divergent repo-local ruleset test scripts and local ruleset forks:standards/rulesets/pr-quality.json&standards/rulesets/README.md:{"type": "deletion"}and{"type": "non_fast_forward"}instandards/rulesets/pr-quality.json, matchingstandards/github-settings.md(Allow force pushes: No,Allow deletions: No) and OpenSSF ScorecardBranch-Protection._ruleset_contents_one()inscripts/compliance-audit.sh:standards/rulesets/<name>.json(deletionandnon_fast_forwardinpr-quality.json) exist on the live ruleset, emittingruleset-drift-<name>-missing-rule-<type>when absent (even if the primary$rtyperule is also missing).check_codeowners()inscripts/compliance-audit.sh&standards/codeowners-standard.md:CODEOWNERSin GitHub's actual evaluation order (.github/CODEOWNERS→CODEOWNERS→docs/CODEOWNERS).\r) and inline#comments on owner lines, and normalizes escaped spaces (\) in path patterns before tokenizing owners.$2..$NF) matches^@${ORG}/[a-z0-9-]+$(codeowners-individual-users), catching individual@usernames, bare usernames/emails without@, malformed team slugs, and teams outside@combsmith/.standards/codeowners-standard.mdthat the default*catch-all line lists ONLY@combsmith/org-leads(codeowners-catchall-extra-owners), preventing extra teams on*from satisfyingrequire_code_owner_reviewrepo-wide.Verification
shellcheck --severity=warning -x scripts/compliance-audit.sh scripts/apply-rulesets.shbats test/scripts/apply-rulesets/ test/scripts/compliance-audit/codeowners-*.bats test/scripts/compliance-audit/ruleset-contents-drift.bats(71/71 passing)Refs combsmith/bmad-bgreat-suite#520, combsmith/bmad-bgreat-suite#336