ci(branch-protection): check-name parity gate (#144) #56
Ghost
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "fix/144-branch-protection-check-name-parity-audit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Adds a regression-guard against the path-filter / matrix-refactor / workflow-rename class of bugs called out in saved memory
feedback_branch_protection_check_name_parity.Audit finding
Every currently-required check name on molecule-core's branch protection (per
apply.sh) ALREADY uses the safe always-runs-with-conditional-steps shape:Platform (Go),Canvas (Next.js),Python Lint & Test,Shellcheck (E2E scripts),Detect changes—ci.yml(single-job-with-per-step-if)Canvas tabs E2E—e2e-staging-canvas.ymlE2E API Smoke Test—e2e-api.ymlPR-built wheel + import smoke—runtime-prbuild-compat.ymlAnalyze (go|javascript-typescript|python)—codeql.ymlmatrixScan diff for credential-shaped strings—secret-scan.yml(no path filter)No production drift to fix today. The new gate prevents future drift.
What it catches
The script classifies each owning workflow into one of five categories:
paths:filter (workflow always fires).if: needs.<x>.outputs.<y>gates without top-levelpaths:.paths:filter without per-step gates (silent block).paths:AND per-step gates (workflow may still skip entirely; gates lay dormant).Test plan
test_check_name_parity.shcovering each classification.paths:+ namedCanvas (Next.js)job → script flags UNSAFE-PATH-FILTER..github/workflows/→ exit 0, all safe.shellcheckclean on both scripts.python3 -c "yaml.safe_load(...)"validates the workflow YAML.Hostile self-review
apply.shusesEOF, but a future contributor could change it toEOF1/MAINCHECKS/ similar. The script extracts the marker dynamically viagrep + sedrather than hardcodingEOF, so it stays correct across renames.if:regex — initially anchored on^[[:space:]]+if:which misses every real workflow's- if:shape (the-step-marker breaks the leading-space anchor). Loosened to matchif:[[:space:]]+needs\.<x>\.outputs\.anywhere on the line; comment explains the rationale.molecule-core, so the gap is non-load-bearing. Adding aggregator detection is straightforward when a workflow lands in that shape (look forif: always()on a job whosename:matches the required check +needs:listing matrix children).Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com
Audit finding: every workflow that emits a required-status-check name on molecule-core's branch protection (apply.sh's STAGING_CHECKS + MAIN_CHECKS) ALREADY uses the safe always-runs-with-conditional-steps shape — Platform/Canvas/Python/Shellcheck in ci.yml, Canvas tabs E2E in e2e-staging-canvas.yml, E2E API Smoke in e2e-api.yml, PR-built wheel in runtime-prbuild-compat.yml, the codeql Analyze matrix, and the always-on Secret scan + Detect changes. No production drift to fix today. Adds a regression-guard so the next path-filter / matrix refactor / workflow rename can't silently re-introduce the bug shape called out in saved memory feedback_branch_protection_check_name_parity: "Path filters … silently break branch protection because no job emits the protected sentinel status when path-filter returns false." New tools: - tools/branch-protection/check_name_parity.sh — extracts every required check name from apply.sh's heredocs, then for each name classifies the owning workflow as safe (no top-level paths:) / safe (per-step if-gates without top-level paths:) / unsafe (top-level paths: without per-step if-gates) / unsafe-mix (top-level paths: WITH per-step if-gates — the workflow may still skip entirely on path exclusion, leaving the gates dormant) / missing (no emitter at all). Special-cases codeql.yml's matrix- expanded `Analyze (${{ matrix.language }})`. - tools/branch-protection/test_check_name_parity.sh — 6 unit tests covering each classification: safe, unsafe-path-filter, missing, safe-with-per-step-gates, unsafe-mix, matrix-expansion. Each test builds a synthetic apply.sh + workflow file in a tmpdir, invokes the script, and asserts on exit code + stderr substring. Per feedback_assert_exact_not_substring the assertions pin specific classifications, not just non-zero exit. Wired into branch-protection-drift.yml so every PR touching .github/workflows/** runs the parity check; the existing daily schedule covers between-PR drift. The check is cheap (~1s) and runs without the admin token — only reads files in the checkout. Self- test step runs the unit tests on every invocation, so a regression in the script can't false-pass on production. Per BSD-vs-GNU portability hygiene: heredoc-marker extraction stays in plain awk + sed (no gawk-only `match()` array form), grep regex avoids `^` anchor for `if:` lines because real workflows use ` - if:` with the `-` step-marker between leading spaces and `if:` (the original anchor missed every workflow's per-step gates). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Branch-protection check-name parity regression guard. Pure tooling addition (3 files: check_name_parity.sh + test + workflow wire-up). Production audit run exit 0 — no current drift. Per feedback_long_term_robust_automated principle (proper + robust + eliminate human error). 6 unit tests covering safe / unsafe-path-filter / matrix-expansion shapes; shellcheck clean. Only red is intentional pr-guards/disable-auto-merge-on-push. Ready to merge.