Compliance: ruleset-drift-pr-quality-dismiss_stale_reviews_on_push #408

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: #408 — Compliance: ruleset-drift-pr-quality-dismiss_stale_reviews_on_push

   Scope

   The codified source of truth for this repo's pr-quality ruleset is scripts/apply-rulesets.sh, whose payload already sets dismiss_stale_reviews_on_push: true (matching the standard). The reported drift is in the live GitHub ruleset state, which is converged operationally by an admin running apply-rulesets.sh (requires an administration:write token and a live GitHub API call — outside a code PR's scope, and not a git operation). The in-scope, durable code deliverable is a regression test that locks the codified value so it cannot silently drift back to false.

   Implementation Checklist

   • Add tests/scripts/apply-rulesets.test.sh — runs scripts/apply-rulesets.sh --dry-run --force with a mocked gh (real jq) and asserts the codified pr-quality ruleset emits "dismiss_stale_reviews_on_push": true and never false. Follows the existing tests/scripts/apply-repo-settings.test.sh convention.

   Tests to Write

   • pr-quality payload sets dismiss_stale_reviews_on_push: true (the drifted parameter)
   • payload never sets dismiss_stale_reviews_on_push: false (guards against regression to the drifted state)
   • script exits 0 in --dry-run --force mode
   • Demonstrate the guard is real (red) by transiently flipping the script value to false and confirming the test fails, then restoring to true (green)

   Test Command

   bash tests/scripts/apply-rulesets.test.sh (plus bash .dev-lead/scripts/dev-lead-lint.sh and pnpm run test for the full suite)
3. don-petry commented on Jul 17, 2026

   

   don-petry

   on Jul 17, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Plan Execution

   • Added tests/scripts/apply-rulesets.test.sh — regression test locking the codified pr-quality ruleset value dismiss_stale_reviews_on_push: true
   • Demonstrated the guard is real (TDD red): transiently flipped the script value to false → test FAILED; restored to true → test PASSED (script restored exactly, no diff)
   • Ran full verification: new test, .dev-lead/scripts/dev-lead-lint.sh (shellcheck + CODEOWNERS), shellcheck on the new file, and pnpm run test

   Root-cause note

   The codified source of truth for this repo's pr-quality ruleset is scripts/apply-rulesets.sh, and its payload already sets dismiss_stale_reviews_on_push: true — matching the standard. The reported drift is in the live GitHub ruleset state, which converges when an admin runs apply-rulesets.sh (requires an administration:write token + a live GitHub API call). That operational convergence is outside the scope of a code PR and is not a git operation, so no source change to the payload was needed. The durable code contribution is a regression test that prevents the codified value from silently drifting back to false.

   Test Results

   ok - codifies the pr-quality ruleset
   ok - pr-quality sets dismiss_stale_reviews_on_push to true (issue #408)
   ok - pr-quality never sets dismiss_stale_reviews_on_push to false
   ---
   PASS — 3 assertion(s) passed
   

   Full suite (pnpm run test): 4 suites, 36 tests passed. Lint (dev-lead-lint.sh): all checks passed. Shellcheck on new file: clean.

   Files Changed

   • tests/scripts/apply-rulesets.test.sh (new): runs apply-rulesets.sh --dry-run --force with a stubbed gh (real jq) and asserts the pr-quality payload emits "dismiss_stale_reviews_on_push": true and never false. Follows the existing tests/scripts/apply-repo-settings.test.sh convention.

   Notes

   • Live convergence still required: to clear the audit finding on GitHub, an admin must run GH_TOKEN=<admin-token> ./scripts/apply-rulesets.sh (or the org-central apply-rulesets.sh --repo petry-projects/broodly). This PR guards the codified value; it cannot mutate live ruleset state.
   • No implementation code changed — the codified payload was already compliant, so per the TDD exception this is a test-only, behavior-preserving change (red was still demonstrated via a transient flip to prove the guard works).
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: #408 — Compliance: ruleset-drift-pr-quality-dismiss_stale_reviews_on_push

   Findings

   • The codified remediation script scripts/apply-rulesets.sh already sets the pr-quality pull_request rule with dismiss_stale_reviews_on_push: true (line 102) — the in-repo source of truth matches the standard.
   • The drift is in live GitHub ruleset state (pr-quality, id 14592511): the live rule currently has dismiss_stale_reviews_on_push: false. Converging live state requires running scripts/apply-rulesets.sh with an administration:write token — an operator/CI step outside a code PR (the dev-lead token here has only repo, workflow, read:org).
   • Gap: there is no regression guard for the pr-quality payload, and unlike its sibling scripts/apply-repo-settings.sh, apply-rulesets.sh --dry-run still hits the live API (fetches existing rulesets before the dry-run guard), so it cannot be validated offline in CI.

   Scope

   Lock the dismiss_stale_reviews_on_push: true standard into an offline, CI-runnable regression test, and make apply-rulesets.sh --dry-run truly offline (mirroring apply-repo-settings.sh) so the codified ruleset can never silently drift.

   Implementation Checklist

   • scripts/apply-rulesets.sh — skip the live gh api repos/.../rulesets fetch when --dry-run is set so dry-run emits payloads without admin API access
   • tests/scripts/apply-rulesets.test.sh — new regression test (mirrors existing apply-repo-settings.test.sh pattern)

   Tests to Write

   • Assert apply-rulesets.sh --dry-run --force (dummy token) exits 0 and its pr-quality payload contains dismiss_stale_reviews_on_push: true
   • Assert the other codified pr-quality params (required_approving_review_count, require_code_owner_review, require_last_push_approval, required_review_thread_resolution) and that the code-quality ruleset is emitted

   Test Command

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

   Note

   This PR codifies and guards the standard. Converging the live pr-quality ruleset still requires an operator to run GH_TOKEN=<admin> ./scripts/apply-rulesets.sh (administration:write), which is outside the scope/credentials of this code PR.
6. don-petry commented on Jul 21, 2026

   

   don-petry

   on Jul 21, 2026

   ContributorAuthor

   More actions

   Dev-Lead: Implementation Complete

   Summary

   The in-repo source of truth (scripts/apply-rulesets.sh) already codifies dismiss_stale_reviews_on_push: true for the pr-quality ruleset — so no value was wrong in code. The gaps were: (1) no regression guard protecting that value, and (2) the script's --dry-run was not offline (it fetched live rulesets first), so it couldn't be validated in CI. This PR closes both gaps.

   Plan Execution

   • Made scripts/apply-rulesets.sh --dry-run fully offline (skips the live gh api .../rulesets fetch), mirroring scripts/apply-repo-settings.sh
   • Added tests/scripts/apply-rulesets.test.sh regression guard asserting dismiss_stale_reviews_on_push: true and the other standard pr-quality params
   • Verified new + existing script tests pass and dev-lead-lint.sh (shellcheck) is clean

   Test Results

   === TDD red (before fix) ===
   not ok - script exited non-zero (1) in --dry-run --force mode
   gh: Bad credentials (HTTP 401)   # dry-run was hitting the live API
   FAIL — see assertions above
   
   === TDD green (after fix) ===
   ok - emits pr-quality ruleset
   ok - pr-quality sets dismiss_stale_reviews_on_push = true
   ok - pr-quality requires 1 approving review
   ok - pr-quality requires code owner review
   ok - pr-quality requires last push approval
   ok - pr-quality requires review thread resolution
   ok - emits code-quality ruleset
   PASS — 7 assertion(s) passed
   
   === full verification ===
   PASS apply-repo-settings.test.sh   (existing, unaffected)
   PASS apply-rulesets.test.sh        (new)
   dev-lead-lint (shellcheck on 2 files + CODEOWNERS): all checks passed
   

   Files Changed

   • scripts/apply-rulesets.sh: skip live ruleset fetch when --dry-run is set so dry-run runs offline (non-dry-run apply path unchanged — real applies still fetch and UPDATE existing rulesets)
   • tests/scripts/apply-rulesets.test.sh: new regression test locking in the pr-quality standard

   Notes / Known Limitation

   • Live-state convergence is out of scope for this code PR. The live pr-quality ruleset (id 14592511) currently reports dismiss_stale_reviews_on_push: false (and require_code_owner_review/require_last_push_approval also false). Converging it requires an operator/CI run of GH_TOKEN=<admin> ./scripts/apply-rulesets.sh with administration:write scope. The dev-lead token available here has only repo, workflow, read:org, so it cannot apply the change. Once an admin runs the (already-correct) script, the weekly compliance audit will clear this finding.
   • No test was skipped or weakened. TDD red→green cycle documented above; not a doc/config-only change, so the Phase 2 exception did not apply.
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 dismiss_stale_reviews_on_push 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 dismiss_stale_reviews_on_push 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

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

   feat: implement issue #408 — Compliance: ruleset-drift-pr-quality-dis…

   ...

   5186d2e
10. don-petry

    closed this as completedin #434on Aug 2, 2026