Compliance: ruleset-drift-pr-quality-require_code_owner_review #407

Activity

1. don-petry

   added

   compliance-auditAutomated compliance audit findingAutomated compliance audit finding

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jul 17, 2026
2. don-petry commented on Jul 17, 2026

   

   don-petry

   on Jul 17, 2026

   ContributorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #407 — Compliance: ruleset-drift-pr-quality-require_code_owner_review

   Scope

   Lock the codified pr-quality ruleset standard in scripts/apply-rulesets.sh (which already sets require_code_owner_review: true) with an offline regression test, so the drifted parameter cannot silently regress in-repo. Live GitHub-state convergence still requires an org admin token via scripts/apply-rulesets.sh and is out of scope for this repo-file change.

   Findings

   • The committed scripts/apply-rulesets.sh already codifies "require_code_owner_review": true (pr-quality payload). There is no drift in the repo file — the drift is in the live GitHub ruleset state, which converges by running the script with an administration:write token.
   • The sibling script scripts/apply-repo-settings.sh has an offline --dry-run --force regression test (tests/scripts/apply-repo-settings.test.sh). apply-rulesets.sh has no equivalent guard, and its --dry-run path currently makes a live gh api call (fails offline), so the codified ruleset parameters are untested.

   Implementation Checklist

   • scripts/apply-rulesets.sh — make --dry-run fully offline by skipping the live rulesets fetch in dry-run mode (behavior-preserving for real runs)
   • tests/scripts/apply-rulesets.test.sh — new offline test asserting the codified pr-quality ruleset sets require_code_owner_review: true (plus core pr-quality review params)

   Tests to Write

   • apply-rulesets.sh --dry-run --force exits 0 without network/admin token
   • dry-run output codifies "require_code_owner_review": true
   • dry-run output codifies the other required pr-quality review params (approving review count, last-push approval, stale-review dismissal, thread resolution)

   Test Command

   bash tests/scripts/apply-rulesets.test.sh
3. don-petry commented on Jul 17, 2026

   

   don-petry

   on Jul 17, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Plan Execution

   • Confirmed scripts/apply-rulesets.sh already codifies "require_code_owner_review": true in the pr-quality payload — no drift in the repo file; the drift is in live GitHub state (which converges only with an administration:write token, out of scope for a repo-file change).
   • Made apply-rulesets.sh --dry-run offline (best-effort fetch, falls back to empty ruleset set when unauthenticated) so the codified payloads can be regression-tested without an admin token. Real (non-dry-run) behavior is unchanged.
   • Added tests/scripts/apply-rulesets.test.sh locking the required pr-quality review parameters, mirroring the existing apply-repo-settings.test.sh pattern.

   Test Results

   ok - codifies the pr-quality ruleset
   ok - enforces require_code_owner_review=true
   ok - enforces required_approving_review_count=1
   ok - enforces require_last_push_approval=true
   ok - enforces dismiss_stale_reviews_on_push=true
   ok - enforces required_review_thread_resolution=true
   ---
   PASS — 6 assertion(s) passed
   

   Verification also run:

   • shellcheck scripts/apply-rulesets.sh tests/scripts/apply-rulesets.test.sh — clean
   • .dev-lead/scripts/dev-lead-lint.sh — passed
   • tests/scripts/apply-repo-settings.test.sh (sibling) — still passes

   Files Changed

   • scripts/apply-rulesets.sh: dry-run no longer hard-fails on a failed rulesets fetch — it falls back to an empty set so payloads can be previewed/tested offline. Non-dry-run runs still fetch and fail hard as before.
   • tests/scripts/apply-rulesets.test.sh: new offline regression test asserting the codified pr-quality ruleset enforces require_code_owner_review=true, required_approving_review_count=1, require_last_push_approval=true, dismiss_stale_reviews_on_push=true, and required_review_thread_resolution=true.

   Notes

   • Live GitHub ruleset convergence is not performed by this change. Bringing the live pr-quality ruleset back to require_code_owner_review=true requires running scripts/apply-rulesets.sh with a token that has administration:write on the repo — a privileged, live-infrastructure operation outside the dev-lead's repo-file scope. This change codifies + regression-guards the correct value so it cannot silently drift in-repo, and provides the tested script to converge live state.
   • The JS/Go package test suites were not run: no node_modules are installed in this environment and the change is shell-only (does not touch any TS/Go package). The relevant shell test suite passes.
4. don-petry

   added

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   and removed

   dev-leadFor dev-lead agent pickupFor dev-lead agent pickup

   on Jul 21, 2026
5. don-petry commented on Jul 21, 2026

   

   don-petry

   on Jul 21, 2026

   ContributorAuthor

   More actions

   Dev-Lead Implementation Plan

   Issue: #407 — Compliance: ruleset-drift-pr-quality-require_code_owner_review

   Scope

   Guard the codified pr-quality ruleset standard (require_code_owner_review: true) with a regression test, since the drift is in live GitHub state while the checked-in convergence script scripts/apply-rulesets.sh is already correct.

   Findings

   • Live pr-quality ruleset (id 14592511) reports require_code_owner_review: false — confirmed via gh api.
   • Checked-in scripts/apply-rulesets.sh already codifies require_code_owner_review: true (plus dismiss_stale_reviews_on_push / require_last_push_approval: true), so the repo source is already compliant with the standard.
   • Converging live state requires running apply-rulesets.sh with an administration:write token. The dev-lead token only has repo/workflow scope and live-ruleset changes mutate shared infrastructure — that is handled by the privileged apply workflow, not a code PR.
   • scripts/apply-rulesets.sh has no regression test (its sibling apply-repo-settings.sh does), which is how the standard could silently drift in-repo.

   Implementation Checklist

   • Add tests/scripts/apply-rulesets.test.sh mirroring the existing tests/scripts/apply-repo-settings.test.sh pattern (self-contained gh mock, --dry-run --force).

   Tests to Write

   • Assert the pr-quality dry-run payload contains "require_code_owner_review": true.
   • Assert the script exits 0 in --dry-run --force mode with a mocked gh.

   Test Command

   bash tests/scripts/apply-rulesets.test.sh

   Note (red-first exception)

   The codified script is already compliant, so the new guard passes on first run — this is a behavior-preserving compliance guard (Phase 2 exception). Live-state convergence is out of scope for a code PR and is noted for the privileged apply workflow.
6. don-petry commented on Jul 21, 2026

   

   don-petry

   on Jul 21, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Summary

   The compliance finding is a live GitHub ruleset drift, not a source defect: the pr-quality ruleset (id 14592511) currently reports require_code_owner_review: false, but the checked-in convergence script scripts/apply-rulesets.sh already codifies require_code_owner_review: true (verified via gh api and a dry-run of the script). The repository source is therefore already aligned with the standard.

   Live convergence is out of scope for a code PR. Applying the ruleset to live GitHub requires an administration:write token; the dev-lead token only carries repo/workflow scope, and mutating a live ruleset changes shared infrastructure. That step is handled by the privileged apply workflow — run:
   GH_TOKEN=<admin-token> ./scripts/apply-rulesets.sh (or the org-level apply-rulesets.sh) to converge.

   To make the repo self-defending against future in-repo drift of this exact parameter, I added the regression guard that scripts/apply-rulesets.sh was missing (its sibling apply-repo-settings.sh already had one).

   Plan Execution

   • Confirmed live drift (require_code_owner_review: false) and that the codified script already sets it true
   • Added tests/scripts/apply-rulesets.test.sh (self-contained gh mock, --dry-run --force, no network)
   • Proved the guard catches drift via a mutation test (flip payload to false → test fails; restore → passes)
   • bash .dev-lead/scripts/dev-lead-lint.sh passes; shellcheck --severity=warning -x on the new file passes
   • Sibling tests/scripts/apply-repo-settings.test.sh still passes (no regression)

   Test Results

   $ bash tests/scripts/apply-rulesets.test.sh
   ok - pr-quality requires code owner review
   ok - emits the pr-quality ruleset
   ok - pr-quality requires one approving review
   ok - pr-quality requires last push approval
   ok - pr-quality requires review thread resolution
   ---
   PASS — 5 assertion(s) passed
   
   $ bash .dev-lead/scripts/dev-lead-lint.sh
     [lint] shellcheck on 2 file(s)...
     [lint] validating CODEOWNERS...
   OK: CODEOWNERS
     [lint] all checks passed
   
   # Mutation check (payload forced to false, then restored):
   not ok - pr-quality requires code owner review (output missing: "require_code_owner_review": true)
   FAIL — see assertions above   # <- guard correctly goes red on drift
   

   Files Changed

   • tests/scripts/apply-rulesets.test.sh (new): regression guard asserting the codified pr-quality payload keeps require_code_owner_review: true alongside its companion review parameters. Runs offline via a stub gh on PATH; mirrors the existing apply-repo-settings.test.sh convention.

   Notes

   • Red-first exception (Phase 2): the codified script already satisfies the standard, so the new guard passes on first run. Genuineness of the guard is demonstrated by the mutation test above rather than by shipping a broken script.
   • The JS (pnpm -r run test) and Go (go test ./...) suites were not run — dependencies are not installed in this environment and the change is a standalone shell test outside both build graphs, so it cannot affect them.
   • Action still required to close the finding: run the apply-rulesets convergence with an admin-scoped token to flip the live ruleset require_code_owner_review from false to true. This code change guards the standard but cannot mutate live GitHub state.
7. don-petry commented on Jul 24, 2026

   

   don-petry

   on Jul 24, 2026

   ContributorAuthor

   More actions

   Weekly Compliance Audit (2026-07-24)

   This finding is still open.

   Detail: Ruleset pr-quality parameter require_code_owner_review has drifted from the codified standard: expected true, actual false. The codified JSON (standards/rulesets/pr-quality.json) is the source of truth (#575/#580); run scripts/apply-rulesets.sh --repo petry-projects/broodly to converge.

   Standard: standards/github-settings.md#pr-quality--standard-ruleset-all-repositories
8. don-petry commented on Jul 31, 2026

   

   don-petry

   on Jul 31, 2026

   ContributorAuthor

   More actions

   Weekly Compliance Audit (2026-07-31)

   This finding is still open.

   Detail: Ruleset pr-quality parameter require_code_owner_review has drifted from the codified standard: expected true, actual false. The codified JSON (standards/rulesets/pr-quality.json) is the source of truth (#575/#580); run scripts/apply-rulesets.sh --repo petry-projects/broodly to converge.

   Standard: standards/github-settings.md#pr-quality--standard-ruleset-all-repositories
9. don-petry

   closed this as completedin #433on Aug 4, 2026
10. don-petry

    added a commit that references this issue on Aug 4, 2026

    feat: implement issue #407 — Compliance: ruleset-drift-pr-quality-req…

    ...

    ff2c20f