diff --git a/.github/workflows/apply-rulesets-tests.yml b/.github/workflows/apply-rulesets-tests.yml new file mode 100644 index 00000000..6bd573b6 --- /dev/null +++ b/.github/workflows/apply-rulesets-tests.yml @@ -0,0 +1,58 @@ +# Quality gates for the apply-rulesets script and its unit tests. +# +# Triggered on any PR that touches: +# - scripts/apply-rulesets.sh +# - test/scripts/apply-rulesets/** +# +# Gates (all must pass before merge): +# 1. shellcheck — static analysis for the applier script +# 2. bats — unit tests for detect_required_checks (guards the #580 +# Dev-Lead Agent divergence removal against regression) + +name: Apply Rulesets Tests + +on: + pull_request: + paths: + - 'scripts/apply-rulesets.sh' + - 'test/scripts/apply-rulesets/**' + - '.github/workflows/apply-rulesets-tests.yml' + push: + branches: + - main + paths: + - 'scripts/apply-rulesets.sh' + - 'test/scripts/apply-rulesets/**' + - '.github/workflows/apply-rulesets-tests.yml' + +permissions: + contents: read + +concurrency: + group: apply-rulesets-tests-${{ github.ref }} + cancel-in-progress: true + +jobs: + test: + name: Lint and bats + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + fetch-depth: 1 + + - name: Install bats, shellcheck, and jq + run: | + set -euo pipefail + sudo apt-get update -qq + sudo apt-get install -y --no-install-recommends bats shellcheck jq + + - name: shellcheck + run: | + set -euo pipefail + shellcheck --severity=warning -x scripts/apply-rulesets.sh + + - name: Run bats suite + run: bats --print-output-on-failure test/scripts/apply-rulesets/ diff --git a/scripts/apply-rulesets.sh b/scripts/apply-rulesets.sh index 790110f7..f0de7b6a 100755 --- a/scripts/apply-rulesets.sh +++ b/scripts/apply-rulesets.sh @@ -1,12 +1,31 @@ #!/usr/bin/env bash # apply-rulesets.sh — Apply standard repository rulesets to petry-projects repos # +# ───────────────────────────────────────────────────────────────────────────── +# LEGACY / DETECTION-BASED APPLIER — NOT the codified source of truth. +# +# This copy builds the `code-quality` required-status-check set *dynamically* by +# probing each repo's workflow files. The canonical source of truth for the org +# rulesets is the codified JSON under `standards/rulesets/` (code-quality.json, +# pr-quality.json), applied by the file-driven applier in `.github-private`, +# which reads those files and PUTs them verbatim. See AGENTS.md ("What lives +# where") and petry-projects/.github#575 / #580. +# +# When these two disagree, the codified `standards/rulesets/*.json` wins. Do NOT +# teach this script to require any context that is absent from those files — +# doing so recreates the divergence #580 removed (the `Dev-Lead Agent / dev-lead` +# injection). Prefer the codified applier; the clean end-state (#580 option 1) is +# to retire this detection path entirely once the cross-repo bootstrap story for +# `.github-private` is settled. +# ───────────────────────────────────────────────────────────────────────────── +# # Companion script to compliance-audit.sh. Creates or updates the rulesets defined in: # standards/github-settings.md#repository-rulesets +# standards/rulesets/*.json (codified source of truth) # # Rulesets managed: # pr-quality — pull request review requirements and merge policy -# code-quality — required status checks (CI, SonarCloud, CodeQL default setup, Dev-Lead Agent) +# code-quality — required status checks (CI, SonarCloud, CodeQL default setup) # # Usage: # # Apply to a specific repo: @@ -68,14 +87,13 @@ detect_required_checks() { } # --- SonarCloud --- + # SonarCloud registers its check context directly via the SonarCloud API as + # "SonarCloud" (the job name), independent of the workflow's top-level `name:`. + # Do NOT derive the context from the workflow name — that would emit + # " / SonarCloud" (e.g. "SonarCloud Analysis / SonarCloud"), which is + # wrong and diverges from both the actual check and the codified code-quality.json. if echo "$workflows" | grep -qx "sonarcloud.yml"; then - local sc_wf_name - sc_wf_name=$(workflow_name "sonarcloud.yml") - if [ -n "$sc_wf_name" ]; then - checks+=("$sc_wf_name / SonarCloud") - else - checks+=("SonarCloud") - fi + checks+=("SonarCloud") fi # --- CodeQL (GitHub-managed default setup) --- @@ -125,9 +143,11 @@ detect_required_checks() { # the per-ecosystem jobs. checks+=("dependency-audit / Detect ecosystems") fi - if echo "$workflows" | grep -qx "dev-lead.yml"; then - checks+=("Dev-Lead Agent / dev-lead") - fi + # NOTE: Dev-Lead Agent / dev-lead is intentionally NOT added as a required + # check, even when dev-lead.yml is present. Per #579 the Dev-Lead Agent is a + # per-PR review, not a merge gate: requiring it deadlocks every + # workflow-touching PR. The codified standards/rulesets/code-quality.json + # deliberately omits it, and this detection-based copy must agree (#580). # dependabot-automerge / dependabot-rebase are intentionally NOT # required: dependabot-automerge runs only on dependabot[bot] PRs and # dependabot-rebase runs only on push to main, neither of which are @@ -351,56 +371,62 @@ apply_rulesets() { } # --------------------------------------------------------------------------- -# Parse arguments +# Main — parse arguments and dispatch # --------------------------------------------------------------------------- -if [ $# -eq 0 ]; then - usage -fi +main() { + if [ $# -eq 0 ]; then + usage + fi -if [ -z "${GH_TOKEN:-}" ]; then - err "GH_TOKEN is required — provide a token with administration:write scope" - exit 1 -fi + if [ -z "${GH_TOKEN:-}" ]; then + err "GH_TOKEN is required — provide a token with administration:write scope" + exit 1 + fi + + export GH_TOKEN -export GH_TOKEN + local TARGET="" arg + for arg in "$@"; do + case "$arg" in + --dry-run) DRY_RUN=true ;; + --all) TARGET="--all" ;; + -*) err "Unknown flag: $arg"; usage ;; + *) TARGET="$arg" ;; + esac + done -TARGET="" -for arg in "$@"; do - case "$arg" in - --dry-run) DRY_RUN=true ;; - --all) TARGET="--all" ;; - -*) err "Unknown flag: $arg"; usage ;; - *) TARGET="$arg" ;; - esac -done + if [ -z "$TARGET" ]; then + usage + fi -if [ -z "$TARGET" ]; then - usage -fi + if [ "$TARGET" = "--all" ]; then + info "Fetching all non-archived repos in $ORG ..." + local repos + repos=$(gh repo list "$ORG" --no-archived --json name -q '.[].name' --limit 500) -# --------------------------------------------------------------------------- -# Main -# --------------------------------------------------------------------------- -if [ "$TARGET" = "--all" ]; then - info "Fetching all non-archived repos in $ORG ..." - repos=$(gh repo list "$ORG" --no-archived --json name -q '.[].name' --limit 500) + if [ -z "$repos" ]; then + err "No repositories found in $ORG — check GH_TOKEN permissions" + exit 1 + fi - if [ -z "$repos" ]; then - err "No repositories found in $ORG — check GH_TOKEN permissions" - exit 1 - fi + local failed=0 repo + for repo in $repos; do + apply_rulesets "$repo" || failed=$((failed + 1)) + done - failed=0 - for repo in $repos; do - apply_rulesets "$repo" || failed=$((failed + 1)) - done + if [ "$failed" -gt 0 ]; then + err "$failed repo(s) failed — check output above for details" + exit 1 + fi - if [ "$failed" -gt 0 ]; then - err "$failed repo(s) failed — check output above for details" - exit 1 + ok "All repos processed successfully" + else + apply_rulesets "$TARGET" fi +} - ok "All repos processed successfully" -else - apply_rulesets "$TARGET" +# Run main only when executed directly, not when sourced (e.g. by bats tests +# that exercise individual helper functions such as detect_required_checks). +if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then + main "$@" fi diff --git a/test/scripts/apply-rulesets/dev-lead-divergence.bats b/test/scripts/apply-rulesets/dev-lead-divergence.bats new file mode 100644 index 00000000..4e46db77 --- /dev/null +++ b/test/scripts/apply-rulesets/dev-lead-divergence.bats @@ -0,0 +1,130 @@ +#!/usr/bin/env bats +# Tests for detect_required_checks() in scripts/apply-rulesets.sh — specifically +# the #580 debt: the detection-based builder must NOT inject a non-codified +# `Dev-Lead Agent / dev-lead` required status check. +# +# Per #579, the Dev-Lead Agent is a per-PR review, not a merge gate — requiring +# it deadlocks workflow-touching PRs. The codified source of truth +# (standards/rulesets/code-quality.json) deliberately omits it, so the +# detection-based copy must agree and not add it back. +# +# The suite sources the script (guarded main block) and stubs `gh` so the pure +# workflow-detection logic can run without live API calls. It asserts that even +# when dev-lead.yml is present in a repo, the Dev-Lead context is not emitted, +# while the genuinely-codified centralized checks still are (no over-removal). + +bats_require_minimum_version 1.5.0 + +setup() { + SCRIPT="${BATS_TEST_DIRNAME}/../../../scripts/apply-rulesets.sh" + [ -f "$SCRIPT" ] || { echo "script not found: $SCRIPT" >&2; return 1; } + + # Sourcing the script must not run main / usage (requires the BASH_SOURCE + # guard added for #580). GH_TOKEN is unset in the test env; the guard keeps + # the top-level token check from aborting the source. + # shellcheck source=/dev/null + source "$SCRIPT" + + # Initialize mock variables to prevent unbound variable errors under set -u + MOCK_WORKFLOWS=() +} + +# --------------------------------------------------------------------------- +# gh stub — answers the three API shapes detect_required_checks() issues: +# 1. contents/.github/workflows → newline list of workflow names +# 2. contents/.github/workflows/ → base64 of the file's `.content` +# 3. code-scanning/default-setup → the default-setup state string +# +# MOCK_WORKFLOWS (array) and MOCK_CODEQL_STATE (string) parameterize each test. +# --------------------------------------------------------------------------- +gh() { + local path="${2:-}" + case "$path" in + */contents/.github/workflows) + [ "${#MOCK_WORKFLOWS[@]}" -gt 0 ] && printf '%s\n' "${MOCK_WORKFLOWS[@]}" || true + ;; + */contents/.github/workflows/sonarcloud.yml) + # Matches the org-standard standards/workflows/sonarcloud.yml (name: SonarCloud + # Analysis, job: SonarCloud). SonarCloud registers its check context directly + # as "SonarCloud" — detection must emit "SonarCloud", never the compound + # "SonarCloud Analysis / SonarCloud" that a naive workflow_name() derivation + # would produce. + printf 'name: SonarCloud Analysis\non: push\njobs:\n sonarcloud:\n name: SonarCloud\n' | base64 + ;; + */contents/.github/workflows/ci.yml) + printf 'name: CI\njobs:\n build:\n name: build\n' | base64 + ;; + */code-scanning/default-setup) + printf '%s\n' "${MOCK_CODEQL_STATE:-configured}" + ;; + *) + return 1 + ;; + esac +} + +# --------------------------------------------------------------------------- +# The divergence guard: dev-lead.yml present must NOT add the Dev-Lead context. +# --------------------------------------------------------------------------- + +@test "dev-lead.yml present does NOT inject 'Dev-Lead Agent / dev-lead'" { + MOCK_WORKFLOWS=("agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") + MOCK_CODEQL_STATE="configured" + run detect_required_checks "somerepo" + [ "$status" -eq 0 ] + [[ "$output" != *"Dev-Lead Agent / dev-lead"* ]] +} + +@test "dev-lead.yml as the only workflow yields no Dev-Lead context" { + MOCK_WORKFLOWS=("dev-lead.yml") + MOCK_CODEQL_STATE="configured" + run detect_required_checks "somerepo" + [ "$status" -eq 0 ] + [[ "$output" != *"Dev-Lead"* ]] +} + +# --------------------------------------------------------------------------- +# SonarCloud context: always the bare "SonarCloud", never the compound form. +# --------------------------------------------------------------------------- + +@test "sonarcloud.yml with 'name: SonarCloud Analysis' emits bare 'SonarCloud' context" { + # The org-standard sonarcloud.yml has name: SonarCloud Analysis. SonarCloud + # registers its check directly via the SonarCloud API as "SonarCloud", so + # the required-check context must never be "SonarCloud Analysis / SonarCloud". + MOCK_WORKFLOWS=("sonarcloud.yml") + MOCK_CODEQL_STATE="not-configured" + run detect_required_checks "somerepo" + [ "$status" -eq 0 ] + [[ "$output" == *"SonarCloud"* ]] + [[ "$output" != *"SonarCloud Analysis / SonarCloud"* ]] +} + +# --------------------------------------------------------------------------- +# No over-removal: the genuinely-codified centralized checks still appear. +# --------------------------------------------------------------------------- + +@test "codified centralized checks are still emitted" { + MOCK_WORKFLOWS=("agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") + MOCK_CODEQL_STATE="configured" + run detect_required_checks "somerepo" + [ "$status" -eq 0 ] + [[ "$output" == *"agent-shield / AgentShield"* ]] + [[ "$output" == *"dependency-audit / Detect ecosystems"* ]] + [[ "$output" == *"CodeQL"* ]] +} + +@test "detected set matches the codified code-quality contexts for these workflows" { + # With sonarcloud + agent-shield + dependency-audit + configured CodeQL, the + # detection output must equal the codified standards/rulesets/code-quality.json + # context set (order-independent) — no extra Dev-Lead injection. + MOCK_WORKFLOWS=("sonarcloud.yml" "agent-shield.yml" "dependency-audit.yml" "dev-lead.yml") + MOCK_CODEQL_STATE="configured" + run detect_required_checks "somerepo" + [ "$status" -eq 0 ] + + detected=$(printf '%s\n' "$output" | sort) + codified=$(jq -r '.rules[] | select(.type=="required_status_checks") + | .parameters?.required_status_checks?[]?.context?' \ + "${BATS_TEST_DIRNAME}/../../../standards/rulesets/code-quality.json" | sort) + [ "$detected" = "$codified" ] +}