From 35f535beb819327d09cec1db0b9328e22a1f7c09 Mon Sep 17 00:00:00 2001 From: DJ Date: Wed, 8 Apr 2026 09:48:12 -0700 Subject: [PATCH 1/3] feat(compliance-audit): detect stale required-check names in rulesets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #92. Adds `check_centralized_check_names` to compliance-audit.sh. For every non-`.github` repo in the org, fetches the active required-status-check contexts from BOTH the new ruleset system (gh api .../rules/branches/main) and classic branch protection (gh api .../branches/main/protection), then flags two distinct problems: 1. Stale pre-centralization names (`claude`, `claude-issue`, `AgentShield`, `Detect ecosystems`) — emits `stale-required-check-` with the canonical replacement in the message. 2. `claude-code / claude` listed as required — emits `required-claude-code-check-broken` because that check is structurally incompatible with workflow-modifying PRs: claude-code-action's GitHub App refuses to mint a token whenever the PR diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. This was the exact root cause of the markets/bmad-bgreat-suite stuck PRs from #87 sweep. Tested locally with stub fixtures for all four cases (stale claude, stale AgentShield, broken claude-code/claude, clean ruleset). Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/compliance-audit.sh | 73 +++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/scripts/compliance-audit.sh b/scripts/compliance-audit.sh index 82466e5..0362ada 100755 --- a/scripts/compliance-audit.sh +++ b/scripts/compliance-audit.sh @@ -549,6 +549,78 @@ check_centralized_workflow_stubs() { done } +# --------------------------------------------------------------------------- +# Check: required-status-check rulesets reference current names +# +# After centralizing workflows into reusables (#87, #88), GitHub composes +# check names as ` / `. Repos +# that updated their workflow files but didn't update their rulesets +# are silently broken — the merge gate references a name that no +# longer exists, so it can never be satisfied. +# +# Inspects both the new ruleset system and classic branch protection. +# Flags two distinct problems: +# 1. Stale pre-centralization name (e.g. `claude`, `AgentShield`) +# → emit "stale-required-check-" +# 2. `claude-code / claude` listed as required +# → emit "required-claude-code-check-broken" because that check +# is structurally incompatible with workflow-modifying PRs +# (claude-code-action's app-token validation refuses to mint +# a token whenever the PR diff includes any workflow file) +# --------------------------------------------------------------------------- +check_centralized_check_names() { + local repo="$1" + + # The .github repo owns the reusables; its own ruleset is allowed to + # reference whatever check names it likes. + [ "$repo" = ".github" ] && return + + # Map from stale name → current canonical name. Order matters for + # readability of the emitted finding only. + local renames=( + "claude:claude-code / claude" + "claude-issue:claude-code / claude-issue" + "AgentShield:agent-shield / AgentShield" + "Detect ecosystems:dependency-audit / Detect ecosystems" + ) + + # Collect every required-status-check context from every source. + # Sources: (1) every active ruleset, (2) classic branch protection. + local contexts="" + + # Source 1: rulesets that apply to main + local ruleset_contexts + ruleset_contexts=$(gh_api "repos/$ORG/$repo/rules/branches/main" \ + --jq '.[] | select(.type == "required_status_checks") | .parameters.required_status_checks[].context' 2>/dev/null || echo "") + contexts+="$ruleset_contexts"$'\n' + + # Source 2: classic branch protection (may not exist) + local classic_contexts + classic_contexts=$(gh_api "repos/$ORG/$repo/branches/main/protection/required_status_checks" \ + --jq '.contexts[]' 2>/dev/null || echo "") + contexts+="$classic_contexts" + + [ -z "$(echo "$contexts" | tr -d '[:space:]')" ] && return + + # Check 1: stale pre-centralization names + local entry old new + for entry in "${renames[@]}"; do + IFS=':' read -r old new <<< "$entry" + if echo "$contexts" | grep -qxF "$old"; then + add_finding "$repo" "rulesets" "stale-required-check-${old// /-}" "error" \ + "Required-status-check ruleset references the stale check name \`$old\`. After workflow centralization (petry-projects/.github#87) this check is published as \`$new\`. Update the ruleset (and any classic branch protection) to use the new name." \ + "standards/ci-standards.md#centralization-tiers" + fi + done + + # Check 2: claude-code / claude as required is structurally broken + if echo "$contexts" | grep -qxF "claude-code / claude"; then + add_finding "$repo" "rulesets" "required-claude-code-check-broken" "error" \ + "Required-status-check ruleset includes \`claude-code / claude\`, which is incompatible with workflow-modifying PRs. claude-code-action's GitHub App refuses to mint an OAuth token for any PR whose diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. Remove \`claude-code / claude\` from required status checks; the check still runs and surfaces review feedback on normal PRs without being a merge gate." \ + "standards/ci-standards.md#centralization-tiers" + fi +} + # --------------------------------------------------------------------------- # Check: CLAUDE.md exists and references AGENTS.md # --------------------------------------------------------------------------- @@ -1026,6 +1098,7 @@ main() { check_workflow_permissions "$repo" check_claude_workflow_checkout "$repo" check_centralized_workflow_stubs "$repo" + check_centralized_check_names "$repo" check_claude_md "$repo" check_agents_md "$repo" From 7f9c32d838570740e0a707344f7186f4d4eb06f1 Mon Sep 17 00:00:00 2001 From: DJ Date: Wed, 8 Apr 2026 10:03:49 -0700 Subject: [PATCH 2/3] =?UTF-8?q?fix(compliance-audit):=20never=20recommend?= =?UTF-8?q?=20renaming=20claude=20=E2=86=92=20claude-code/claude?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit on #96: the rename map said `claude` → `claude-code / claude`, but a separate check correctly flags `claude-code / claude` as a forbidden required check (it deadlocks workflow PRs). Following the rename recommendation would have moved a repo from one broken state to another. Fix: split the logic into two distinct sets. 1. `renames[]` — only contains checks where the new name is safe to require (AgentShield, Detect ecosystems). These get a "rename to X" message. 2. `forbidden_required[]` — contains every claude variant (legacy and post-centralization). Any of them as a required check emits a stable per-name finding telling the maintainer to REMOVE it from required checks, not rename it. The Claude review check still runs and surfaces feedback on normal PRs without being a merge gate; only the required-status-checks pin is removed. Each forbidden_required entry maps to a stable check id so findings don't churn across audit runs from slashes in canonical names. Verified locally with stub fixtures for all five cases: stale `claude` -> required-claude-check-broken stale `claude-issue` -> required-claude-issue-check-broken `claude-code / claude` -> required-claude-code-check-broken stale `AgentShield` -> stale-required-check-AgentShield (rename) clean ruleset -> 0 findings Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/compliance-audit.sh | 59 ++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/scripts/compliance-audit.sh b/scripts/compliance-audit.sh index 0362ada..d104039 100755 --- a/scripts/compliance-audit.sh +++ b/scripts/compliance-audit.sh @@ -575,15 +575,36 @@ check_centralized_check_names() { # reference whatever check names it likes. [ "$repo" = ".github" ] && return - # Map from stale name → current canonical name. Order matters for - # readability of the emitted finding only. + # Map from stale name → current canonical name. Used for the rename + # remediation message. The remediation here is "rename in the + # ruleset" because both the old and new names refer to a check that + # CAN be required (it runs on PRs and reports a definitive result). + # + # NOTE: `claude` and `claude-issue` are deliberately NOT in this map. + # The post-centralization equivalents are `claude-code / claude` and + # `claude-code / claude-issue`, but those checks are themselves + # incompatible with workflow-modifying PRs (claude-code-action's app + # token validation refuses to mint a token for any PR whose diff + # includes a workflow file, so the check fails on every workflow PR + # and the merge gate becomes a deadlock). The remediation for the + # `claude*` cases is therefore "remove from required checks", not + # "rename" — handled below as a separate finding so the message + # never recommends a name that creates a new deadlock. local renames=( - "claude:claude-code / claude" - "claude-issue:claude-code / claude-issue" "AgentShield:agent-shield / AgentShield" "Detect ecosystems:dependency-audit / Detect ecosystems" ) + # Names that should never be required checks regardless of how they + # got there (legacy or post-centralization). Removing them from the + # required-checks list is the only valid remediation. + local forbidden_required=( + "claude" + "claude-issue" + "claude-code / claude" + "claude-code / claude-issue" + ) + # Collect every required-status-check context from every source. # Sources: (1) every active ruleset, (2) classic branch protection. local contexts="" @@ -602,7 +623,7 @@ check_centralized_check_names() { [ -z "$(echo "$contexts" | tr -d '[:space:]')" ] && return - # Check 1: stale pre-centralization names + # Check 1: stale pre-centralization names that have a safe rename local entry old new for entry in "${renames[@]}"; do IFS=':' read -r old new <<< "$entry" @@ -613,12 +634,28 @@ check_centralized_check_names() { fi done - # Check 2: claude-code / claude as required is structurally broken - if echo "$contexts" | grep -qxF "claude-code / claude"; then - add_finding "$repo" "rulesets" "required-claude-code-check-broken" "error" \ - "Required-status-check ruleset includes \`claude-code / claude\`, which is incompatible with workflow-modifying PRs. claude-code-action's GitHub App refuses to mint an OAuth token for any PR whose diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. Remove \`claude-code / claude\` from required status checks; the check still runs and surfaces review feedback on normal PRs without being a merge gate." \ - "standards/ci-standards.md#centralization-tiers" - fi + # Check 2: claude-* checks (legacy or post-centralization) listed as + # required. These cannot be made compliant by renaming because the + # post-centralization name is itself broken — the only safe action + # is to remove the check from required-status-checks entirely. + local forbidden + for forbidden in "${forbidden_required[@]}"; do + if echo "$contexts" | grep -qxF "$forbidden"; then + # Use a stable check id so this finding can be tracked across + # audit runs without churn from the slash in the canonical name. + local check_id + case "$forbidden" in + "claude") check_id="required-claude-check-broken" ;; + "claude-issue") check_id="required-claude-issue-check-broken" ;; + "claude-code / claude") check_id="required-claude-code-check-broken" ;; + "claude-code / claude-issue") check_id="required-claude-code-issue-check-broken" ;; + *) check_id="required-claude-broken" ;; + esac + add_finding "$repo" "rulesets" "$check_id" "error" \ + "Required-status-check ruleset includes \`$forbidden\`, which is incompatible with workflow-modifying PRs. claude-code-action's GitHub App refuses to mint an OAuth token for any PR whose diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. **Remove \`$forbidden\` from required status checks** — do NOT rename it. The Claude review check still runs on normal PRs and surfaces feedback without being a merge gate. See \`scripts/apply-rulesets.sh\` (post petry-projects/.github#94) for the canonical required-checks list." \ + "standards/ci-standards.md#centralization-tiers" + fi + done } # --------------------------------------------------------------------------- From 13c8024553854f4ccf075ddf91b66827d24b2630 Mon Sep 17 00:00:00 2001 From: DJ Date: Wed, 8 Apr 2026 10:16:02 -0700 Subject: [PATCH 3/3] fix(compliance-audit): suffix-match forbidden Claude required checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address remaining CodeRabbit feedback on #96: the previous exact-match list (`claude-code / claude`, `claude-code / claude-issue`) only caught the canonical caller-job-id. Repos with a custom caller — e.g. a ruleset that pins `Claude Code / claude` (workflow display name) or `review-claude / claude` (custom job id) — would slip past the audit even though they're equally broken. Fix: classify each context line by suffix: - bare `claude` / `claude-issue` → match - ` / claude` → match - ` / claude-issue` → match Then map to a stable check id (claude vs claude-issue) so findings don't churn across audit runs from prefix variation. The full forbidden context string is still echoed in the finding message so maintainers see exactly what to remove. Verified locally with stub fixtures for 8 cases: bare claude / claude-issue canonical claude-code / claude{,-issue} custom-prefix Claude Code / claude weird-prefix review-claude / claude stale AgentShield (rename, unaffected) clean ruleset (no findings) Co-Authored-By: Claude Opus 4.6 (1M context) --- scripts/compliance-audit.sh | 63 ++++++++++++++++++++++--------------- 1 file changed, 37 insertions(+), 26 deletions(-) diff --git a/scripts/compliance-audit.sh b/scripts/compliance-audit.sh index d104039..50f3fe2 100755 --- a/scripts/compliance-audit.sh +++ b/scripts/compliance-audit.sh @@ -595,15 +595,15 @@ check_centralized_check_names() { "Detect ecosystems:dependency-audit / Detect ecosystems" ) - # Names that should never be required checks regardless of how they - # got there (legacy or post-centralization). Removing them from the - # required-checks list is the only valid remediation. - local forbidden_required=( - "claude" - "claude-issue" - "claude-code / claude" - "claude-code / claude-issue" - ) + # Patterns for required checks that are structurally broken and + # should be removed (not renamed). Matched as either: + # - the bare legacy name ("claude" / "claude-issue"), or + # - any reusable-workflow check whose suffix is "/ claude" or + # "/ claude-issue", regardless of caller-job-id prefix + # (so a custom caller named e.g. "Claude Code / claude" is + # also caught, not just the canonical "claude-code / claude"). + # + # The match is computed against each context line below. # Collect every required-status-check context from every source. # Sources: (1) every active ruleset, (2) classic branch protection. @@ -638,24 +638,35 @@ check_centralized_check_names() { # required. These cannot be made compliant by renaming because the # post-centralization name is itself broken — the only safe action # is to remove the check from required-status-checks entirely. - local forbidden - for forbidden in "${forbidden_required[@]}"; do - if echo "$contexts" | grep -qxF "$forbidden"; then - # Use a stable check id so this finding can be tracked across - # audit runs without churn from the slash in the canonical name. - local check_id - case "$forbidden" in - "claude") check_id="required-claude-check-broken" ;; - "claude-issue") check_id="required-claude-issue-check-broken" ;; - "claude-code / claude") check_id="required-claude-code-check-broken" ;; - "claude-code / claude-issue") check_id="required-claude-code-issue-check-broken" ;; - *) check_id="required-claude-broken" ;; - esac - add_finding "$repo" "rulesets" "$check_id" "error" \ - "Required-status-check ruleset includes \`$forbidden\`, which is incompatible with workflow-modifying PRs. claude-code-action's GitHub App refuses to mint an OAuth token for any PR whose diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. **Remove \`$forbidden\` from required status checks** — do NOT rename it. The Claude review check still runs on normal PRs and surfaces feedback without being a merge gate. See \`scripts/apply-rulesets.sh\` (post petry-projects/.github#94) for the canonical required-checks list." \ - "standards/ci-standards.md#centralization-tiers" + # + # We classify each context line by suffix so any caller-job-id prefix + # is caught (e.g. "claude-code / claude", "Claude Code / claude", + # "review-claude / claude" all match). + local context match_type + while IFS= read -r context; do + [ -z "$context" ] && continue + match_type="" + case "$context" in + "claude") match_type="claude" ;; + "claude-issue") match_type="claude-issue" ;; + *"/ claude") match_type="claude" ;; + *"/ claude-issue") match_type="claude-issue" ;; + *) continue ;; + esac + + # Stable check id per match type so findings don't churn across + # audit runs from variations in caller-job-id prefixes. + local check_id + if [ "$match_type" = "claude-issue" ]; then + check_id="required-claude-issue-check-broken" + else + check_id="required-claude-check-broken" fi - done + + add_finding "$repo" "rulesets" "$check_id" "error" \ + "Required-status-check ruleset includes \`$context\`, which is incompatible with workflow-modifying PRs. claude-code-action's GitHub App refuses to mint an OAuth token for any PR whose diff includes a workflow file, so the check fails on every workflow PR and the merge gate becomes a deadlock. **Remove \`$context\` from required status checks** — do NOT rename it. The Claude review check still runs on normal PRs and surfaces feedback without being a merge gate. See \`scripts/apply-rulesets.sh\` (post petry-projects/.github#94) for the canonical required-checks list." \ + "standards/ci-standards.md#centralization-tiers" + done <<< "$contexts" } # ---------------------------------------------------------------------------