From e6a18c12731ed59895d1b93473ca21a64839fd41 Mon Sep 17 00:00:00 2001 From: core-devops Date: Sun, 21 Jun 2026 20:11:48 -0700 Subject: [PATCH] ci(governance): make qa-review + security-review + reserved-path-review merge-blocking (SSOT + directives) (#3141) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CTO-authorized: the three review-team gates post commit statuses today but are NOT in branch_protections/main.status_check_contexts, so a PR merges once its 4 BP-required contexts are green regardless of the review verdicts (proven: #3131 merged with qa-review red). MECHANISM — Path B (owner BP flip) is the ONLY option. Path A (wire into the `CI / all-required` sentinel) is structurally impossible: all-required is a plain `needs:` aggregator over CI's OWN jobs (ci.yml); Gitea has no cross-workflow `needs:`, and the prior poll-the-status-API design was removed (runner-squat RCA 2026-06-01). These three contexts are emitted by separate workflow files on pull_request_target / pull_request_review events, so the sentinel cannot gate them — which is exactly why ci-required-drift.py F4 keeps cross-workflow required contexts honest by requiring a live emitter in BP. This PR does the lint-correct in-repo half (owner BP flip follows merge): - .gitea/required-contexts.txt: add the 3 review contexts (bare/event-stripped form) + the REMAINING OWNER ACTION block (mirrors the #48 precedent). Allowlist-superset-of-BP is lint-clean; BP-superset-of-allowlist trips lint_no_coe_on_required's live-BP drift check — so the allowlist MUST land first, BP flip second. - qa-review.yml / security-review.yml: flip the stale `# bp-exempt:` directive (whose text falsely claimed "enforced by CI / all-required") to `# bp-required: pending #3141`. - reserved-path-review.yml: re-point `# bp-required: pending #673` → `#3141` (same owner BP flip, now CTO-authorized). BP variant to require = (pull_request_target): fires on every PR open/sync and starts RED when there is no APPROVE (review-check.sh exits 1 → job conclusion publishes `failure`) — fail-closed, no false-green, no deadlock. The pull_request_review path explicitly RE-POSTS the (pull_request_target) context to flip green on a genuine non-author APPROVE (same shape as sop-checklist). Requiring (pull_request_review) instead would deadlock (absent-until-review). Lints verified locally green: lint_no_coe_on_required (OK, 11 required, no CoE), lint_required_context_exists_in_bp (skipped — directive edits create no new emissions), lint-required-no-paths (OK), lint-workflow-yaml (OK), lint_bp_context_emit_match (OK). Owner action tracked in #3141. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitea/required-contexts.txt | 30 +++++++++++++++++++++++ .gitea/workflows/qa-review.yml | 10 +++++++- .gitea/workflows/reserved-path-review.yml | 20 +++++++-------- .gitea/workflows/security-review.yml | 10 +++++++- 4 files changed, 58 insertions(+), 12 deletions(-) diff --git a/.gitea/required-contexts.txt b/.gitea/required-contexts.txt index 4b2902d0b..0b1a164a4 100644 --- a/.gitea/required-contexts.txt +++ b/.gitea/required-contexts.txt @@ -21,3 +21,33 @@ E2E Staging SaaS (full lifecycle) / E2E Staging Concierge Creates Workspace # (pull_request)" to branch_protections/main.status_check_contexts AFTER this PR # merges (allowlist-superset-of-BP is lint-clean; BP-superset-of-allowlist is not). E2E Staging SaaS (full lifecycle) / E2E Staging Platform Boot +# #3141 (CTO-authorized): make the three review-team gates TRULY merge-blocking. +# Today they post commit statuses but are NOT in BP, so a PR merges once its 4 +# BP-required contexts are green regardless of the review verdicts (proven: #3131 +# merged with qa-review red). +# +# MECHANISM — Path B (owner BP flip) is the ONLY option; Path A (sentinel) is +# structurally impossible: `CI / all-required` is a plain `needs:` aggregator +# over CI's OWN jobs (ci.yml). Gitea has no cross-workflow `needs:`, and the +# poll-the-status-API design was deliberately removed (runner-squat RCA +# 2026-06-01). These three contexts are emitted by SEPARATE workflow files on +# pull_request_target / pull_request_review events, so the sentinel cannot gate +# them — which is exactly why ci-required-drift.py F4 enforces cross-workflow +# required contexts in BP directly. +# +# BP variant = (pull_request_target): it fires on every PR open/synchronize and +# starts RED when there is no APPROVE (review-check.sh exits 1 → job conclusion +# publishes `failure`) — fail-closed, no false-green, no deadlock. The +# pull_request_review path explicitly RE-POSTS the (pull_request_target) context +# to flip green on a genuine non-author APPROVE (same shape as sop-checklist). +# Requiring (pull_request_review) instead would deadlock (absent-until-review = +# pending forever). +# +# REMAINING OWNER ACTION: after THIS PR merges, PATCH +# branch_protections/main.status_check_contexts to ADD the three +# `(pull_request_target)` variants of the contexts below (keep the existing 4). +# Allowlist-superset-of-BP is lint-clean; BP-superset-of-allowlist is not — so +# the allowlist (this file) MUST land first, BP flip second. See #3141. +qa-review / approved +security-review / approved +reserved-path-review / reserved-path-review diff --git a/.gitea/workflows/qa-review.yml b/.gitea/workflows/qa-review.yml index fded9518f..e100dab54 100644 --- a/.gitea/workflows/qa-review.yml +++ b/.gitea/workflows/qa-review.yml @@ -113,7 +113,15 @@ permissions: statuses: write jobs: - # bp-exempt: PR review bot signal; required merge state is enforced by CI / all-required. + # bp-required: pending #3141 — CTO-authorized: this gate is becoming TRULY + # merge-blocking. NB: `CI / all-required` does NOT enforce this context — it is + # a plain `needs:` aggregator over CI's OWN jobs (ci.yml) and Gitea has no + # cross-workflow `needs:`; the prior poll-the-status-API design was removed + # (runner-squat RCA 2026-06-01). Enforcement requires the BP context + # `qa-review / approved (pull_request_target)` to be in + # branch_protections/main.status_check_contexts (owner action, tracked in + # #3141; allowlist landed in .gitea/required-contexts.txt this PR, BP flip + # follows merge). `pending #NNN` per lint_required_context_exists_in_bp grammar. approved: # Gate the job: # - On pull_request_target events: always run. diff --git a/.gitea/workflows/reserved-path-review.yml b/.gitea/workflows/reserved-path-review.yml index 2992b75e7..05c78c14c 100644 --- a/.gitea/workflows/reserved-path-review.yml +++ b/.gitea/workflows/reserved-path-review.yml @@ -58,16 +58,16 @@ permissions: jobs: # IS intended to be branch-protection-enforced (that's its purpose — - # closes cp#673). But main BP currently has 0 status_check_contexts - # (see .gitea/required-contexts.txt, only CI / all-required / - # E2E API Smoke / Handlers PG are pinned) so `bp-required: yes` - # would fail the in-BP verification. `bp-required: pending #673` - # therefore: commit to enforcement, defer the actual - # BP-status-check-add to a separate operator follow-up tracked via - # cp#673. (The actual addition of `reserved-path-review` to main - # BP status_check_contexts is an OPERATOR action — not in scope for - # the lint or this PR.) - # bp-required: pending #673 — the reserved-path-review self-merge guard + # closes cp#673). The BP context add (`reserved-path-review / + # reserved-path-review (pull_request_target)` → main + # status_check_contexts) is an OPERATOR action; it is now CTO-authorized + # and tracked in #3141 alongside qa-review + security-review (the same + # owner BP flip). The allowlist (.gitea/required-contexts.txt) lands this + # PR; the BP flip follows merge (allowlist-superset-of-BP is lint-clean, + # the reverse is not). `bp-required: yes` would fail the in-BP verification + # until BP is flipped, so this stays `pending #NNN` per + # lint_required_context_exists_in_bp grammar. + # bp-required: pending #3141 — the reserved-path-review self-merge guard reserved-path-review: runs-on: ubuntu-latest # Draft PRs can't merge anyway; skip to save a runner. Still runs on diff --git a/.gitea/workflows/security-review.yml b/.gitea/workflows/security-review.yml index 0c7d2f9f0..8f7c99c4f 100644 --- a/.gitea/workflows/security-review.yml +++ b/.gitea/workflows/security-review.yml @@ -36,7 +36,15 @@ permissions: statuses: write jobs: - # bp-exempt: PR security review bot signal; required merge state is enforced by CI / all-required. + # bp-required: pending #3141 — CTO-authorized: this gate is becoming TRULY + # merge-blocking. NB: `CI / all-required` does NOT enforce this context — it is + # a plain `needs:` aggregator over CI's OWN jobs (ci.yml) and Gitea has no + # cross-workflow `needs:`; the prior poll-the-status-API design was removed + # (runner-squat RCA 2026-06-01). Enforcement requires the BP context + # `security-review / approved (pull_request_target)` to be in + # branch_protections/main.status_check_contexts (owner action, tracked in + # #3141; allowlist landed in .gitea/required-contexts.txt this PR, BP flip + # follows merge). `pending #NNN` per lint_required_context_exists_in_bp grammar. approved: # Gate the job: # - On pull_request_target events: always run. -- 2.52.0