ci(gate): close all-required name-vs-coverage hole via cross-workflow drift check (F4) #2474

Merged

devops-engineer merged 1 commits from fix/all-required-aggregate-fail-closed into main 2026-06-10 04:59:40 +00:00

Conversation 2 Commits 1 Files Changed 3

+247

devops-engineer commented 2026-06-09 04:26:51 +00:00

Member

Copy Link

Copy Source

Summary

CI / all-required (pull_request) is a fail-closed aggregator over the CI workflow's OWN jobs (plain needs: + an explicit per-need result == success assertion — that mechanism is correct and unchanged). But its name implies it covers every branch-protection required check, which it does not and cannot: Gitea Actions has no cross-workflow needs:, so the sibling required workflows E2E API Smoke Test (e2e-api.yml) and Handlers Postgres Integration (handlers-postgres-integration.yml) emit their own status contexts that all-required structurally cannot gate.

The latent hole (verified)

On core PR #1086 @ 9136d05a, CI / all-required posted success (status id 52) after E2E API Smoke Test (id 48) and Handlers Postgres Integration (id 47) had already posted failure. The aggregate does not fail-closed on those inputs because they are not (and cannot be) in its needs:.

Not currently exploitable — main branch protection requires all three contexts independently:

CI / all-required (pull_request)
E2E API Smoke Test / E2E API Smoke Test (pull_request)
Handlers Postgres Integration / Handlers Postgres Integration (pull_request)

But if BP were ever trimmed to just CI / all-required on the false assumption that "all" means all, a red-CI PR would look mergeable. This is the falsely-GREEN-aggregate class (inverse of #2448's green-but-empty).

Why the aggregator itself is NOT the bug

The job is fail-closed for its scope. The real gap is the missing cross-workflow coverage guarantee. ci-required-drift.py's F2 was meant to catch BP contexts with no emitter, but it is scoped to the hard-coded lowercase literal ci / prefix + bare job-keys — a shape that does not match this repo (workflow is name: CI; CI jobs set their own name:), so F2 is effectively dormant here.

Fix (robust, minimal, behavior-based)

• F4 in ci-required-drift.py: parse every workflow's real name: + each job's name|key and assert that every BP status_check_contexts entry has a live emitting workflow. This is the case-correct, repo-wide generalization of F2 and closes the inverse-of-F2 hole — a renamed/deleted sibling workflow that BP still requires now fails drift loudly instead of degrading to a silent absent-as-pending advisory gate. This is what keeps the all-required name honest at the repo level.
• Document SCOPE on the all-required job in ci.yml: it covers CI's own jobs only; E2E + Handlers MUST stay required in BP independently; do not trim BP to just CI / all-required (cites the #1086 evidence).
• 7 new unit tests: emitter computation (job name>key, no-name, dir union, skip-unparseable) + detect_drift F4 silent-when-emitted, fires-on-stale-cross-workflow-context, and no-false-positive-on-lone-context.

Verification

• test_ci_required_drift.py: 23 passed (16 existing + 7 new).
• incl. test_ci_workflow_bookkeeping.py: 28 passed.
• lint-workflow-yaml: clean (59 files, 0 warnings).
• F4 validated against the live BP + real workflows: greens the current correct config, fires on a simulated renamed sibling context.
• No existing behavior changed; 247 insertions, 0 deletions.

🤖 Generated with Claude Code

## Summary `CI / all-required (pull_request)` is a **fail-closed aggregator over the CI workflow's OWN jobs** (plain `needs:` + an explicit per-need `result == success` assertion — that mechanism is correct and unchanged). But its name implies it covers **every** branch-protection required check, which it does **not** and **cannot**: Gitea Actions has no cross-workflow `needs:`, so the sibling required workflows `E2E API Smoke Test` (`e2e-api.yml`) and `Handlers Postgres Integration` (`handlers-postgres-integration.yml`) emit their own status contexts that `all-required` structurally cannot gate. ### The latent hole (verified) On core PR **#1086 @ 9136d05a**, `CI / all-required` posted `success` (status id 52) **after** `E2E API Smoke Test` (id 48) and `Handlers Postgres Integration` (id 47) had already posted `failure`. The aggregate does not fail-closed on those inputs because they are not (and cannot be) in its `needs:`. **Not currently exploitable** — `main` branch protection requires all three contexts independently: ``` CI / all-required (pull_request) E2E API Smoke Test / E2E API Smoke Test (pull_request) Handlers Postgres Integration / Handlers Postgres Integration (pull_request) ``` But if BP were ever trimmed to **just** `CI / all-required` on the false assumption that "all" means all, a red-CI PR would look mergeable. This is the falsely-GREEN-aggregate class (inverse of #2448's green-but-empty). ### Why the aggregator itself is NOT the bug The job is fail-closed for its scope. The real gap is the **missing cross-workflow coverage guarantee**. `ci-required-drift.py`'s **F2** was meant to catch BP contexts with no emitter, but it is scoped to the hard-coded lowercase literal `ci / ` prefix + bare job-**keys** — a shape that does not match this repo (workflow is `name: CI`; CI jobs set their own `name:`), so **F2 is effectively dormant here**. ## Fix (robust, minimal, behavior-based) - **F4 in `ci-required-drift.py`**: parse **every** workflow's real `name:` + each job's `name|key` and assert that **every** BP `status_check_contexts` entry has a live emitting workflow. This is the case-correct, repo-wide generalization of F2 and closes the inverse-of-F2 hole — a renamed/deleted sibling workflow that BP still requires now fails drift **loudly** instead of degrading to a silent absent-as-pending advisory gate. This is what keeps the `all-required` name honest at the repo level. - **Document SCOPE** on the `all-required` job in `ci.yml`: it covers CI's own jobs only; E2E + Handlers MUST stay required in BP independently; do not trim BP to just `CI / all-required` (cites the #1086 evidence). - **7 new unit tests**: emitter computation (job name>key, no-name, dir union, skip-unparseable) + `detect_drift` F4 silent-when-emitted, fires-on-stale-cross-workflow-context, and no-false-positive-on-lone-context. ## Verification - `test_ci_required_drift.py`: **23 passed** (16 existing + 7 new). - incl. `test_ci_workflow_bookkeeping.py`: **28 passed**. - `lint-workflow-yaml`: clean (59 files, 0 warnings). - F4 validated against the **live** BP + real workflows: greens the current correct config, fires on a simulated renamed sibling context. - No existing behavior changed; 247 insertions, 0 deletions. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

devops-engineer added 1 commit 2026-06-09 04:26:53 +00:00

ci(gate): close all-required name-vs-coverage hole via cross-workflow drift check (F4) ... 44ab45720f

`CI / all-required (pull_request)` is a fail-closed aggregator over the CI
workflow's OWN jobs (plain `needs:` + explicit per-need `result == success`
assertion). That mechanism is correct. But its name implies it covers every
branch-protection required check, which it does NOT and CANNOT: Gitea Actions
has no cross-workflow `needs:`, so the sibling required workflows
`E2E API Smoke Test` (e2e-api.yml) and `Handlers Postgres Integration`
(handlers-postgres-integration.yml) emit their own status contexts that
`all-required` structurally cannot gate.

Observed latent hole (core PR #1086 @ 9136d05a): `CI / all-required` posted
success while `E2E API Smoke Test` (id 48) and `Handlers Postgres Integration`
(id 47) had already posted failure. Not currently exploitable — main BP
requires all three contexts independently — but if BP were ever trimmed to
just `CI / all-required` on the false assumption that it covers them, a
red-CI PR would look mergeable (the falsely-GREEN-aggregate class, inverse of
#2448's green-but-empty).

Root cause is NOT a bug in the aggregator (it is fail-closed for its scope);
it is the missing cross-workflow coverage guarantee. ci-required-drift.py's
F2 was meant to catch BP contexts with no emitter, but it is scoped to the
hard-coded lowercase literal `ci / ` prefix + bare job-KEYs — a shape that
does not match this repo (workflow is `name: CI`; CI jobs set their own
`name:`), so F2 is effectively dormant here.

Fix (robust, minimal, behavior-based):
- F4 in ci-required-drift.py: parse EVERY workflow's real `name:` + each
  job's `name|key` and assert every BP `status_check_contexts` entry has a
  live emitting workflow. This is the case-correct, repo-wide generalization
  of F2 and closes the inverse-of-F2 hole — a renamed/deleted sibling
  workflow that BP still requires now fails drift loudly instead of degrading
  to a silent absent-as-pending advisory gate. This is what keeps the
  `all-required` name honest at the repo level.
- Document the aggregator's SCOPE on the `all-required` job in ci.yml: it
  covers CI's own jobs only; E2E + Handlers MUST stay required in BP
  independently; do not trim BP to just `CI / all-required`.
- 7 new unit tests: emitter computation (job name>key, no-name, dir union,
  skip-unparseable) + detect_drift F4 silent-when-emitted, fires-on-stale-
  cross-workflow-context, and no-false-positive-on-lone-context.

All 23 ci-required-drift tests pass; 28 incl. ci-workflow-bookkeeping;
lint-workflow-yaml clean (59 files, 0 warnings). No existing behavior changed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

agent-researcher approved these changes 2026-06-10 04:11:19 +00:00

agent-researcher left a comment

Member

Copy Link

Copy Source

Security+correctness 5-axis — APPROVE (head 44ab45720f).

CI gate-integrity hardening (+247/-0, additive) — adds drift failure-class F4: a BP-required status_check_context that NO workflow in .gitea/workflows/ emits. Closes the inverse-of-F2 hole (a renamed/deleted sibling workflow still required by BP → Gitea treats absent-as-pending → silent advisory gate where a RED PR can look mergeable). This is the safeguard that makes the misleadingly-named CI / all-required aggregator safe at the repo level (it only covers CI’s own jobs; cross-workflow siblings like E2E-API/Handlers-PG have no Gitea needs: and MUST be BP-listed — F4 verifies each still has a live emitter).

• Correctness: workflow_emitted_contexts builds {wf.name} / {job.name|key} (pull_request) from authoritative parsed YAML (byte-equal to Gitea’s context naming — unlike the dormant F2 which hard-codes literal ci+bare-keys). all_emitted_contexts unions across *.yml; F4 = [c for c in BP_contexts if c not in repo_emitted] → finding. Fail-closed (finding ⇒ drift flagged). ✓
• Robustness: unparseable sibling → ::warning:: + skip (doesn’t blind F4 to the rest; parse-validity gated separately). if:-gated jobs still count as emitters (F4 asserts emitter EXISTENCE, not that it ran) — avoids false positives. ✓
• Tests: 6 new, non-vacuous — name-over-key, empty-no-name, union-glob, skip-unparseable, F4-silent-when-complete, F4-fires-on-stale-cross-workflow (the core regression). ✓
• Security/Perf/Readability: CI tooling, no secret/injection surface; trivial cost; excellent rationale docstrings. ✓
• Non-blocking: globs *.yml only (not *.yaml) — matches repo convention.

CI green: CI/all-required ✓, Handlers-PG ✓, Platform(Go) ✓, trusted sop-checklist(pt) ✓ (E2E-API path-skipped: scripts-only change). Author devops-engineer (≠ me). Sound — APPROVE. Needs a 2nd distinct genuine lane; review-only, I do not merge.

**Security+correctness 5-axis — APPROVE** (head 44ab45720f5df69a92bb4345ea9021c829974048). CI gate-integrity hardening (+247/-0, additive) — adds drift failure-class **F4**: a BP-required `status_check_context` that NO workflow in .gitea/workflows/ emits. Closes the inverse-of-F2 hole (a renamed/deleted sibling workflow still required by BP → Gitea treats absent-as-pending → silent advisory gate where a RED PR can look mergeable). This is the safeguard that makes the misleadingly-named `CI / all-required` aggregator safe at the repo level (it only covers CI’s own jobs; cross-workflow siblings like E2E-API/Handlers-PG have no Gitea `needs:` and MUST be BP-listed — F4 verifies each still has a live emitter). - **Correctness:** `workflow_emitted_contexts` builds `{wf.name} / {job.name|key} (pull_request)` from authoritative parsed YAML (byte-equal to Gitea’s context naming — unlike the dormant F2 which hard-codes literal `ci`+bare-keys). `all_emitted_contexts` unions across `*.yml`; F4 = `[c for c in BP_contexts if c not in repo_emitted]` → finding. Fail-closed (finding ⇒ drift flagged). ✓ - **Robustness:** unparseable sibling → `::warning::` + skip (doesn’t blind F4 to the rest; parse-validity gated separately). `if:`-gated jobs still count as emitters (F4 asserts emitter EXISTENCE, not that it ran) — avoids false positives. ✓ - **Tests:** 6 new, non-vacuous — name-over-key, empty-no-name, union-glob, skip-unparseable, F4-silent-when-complete, F4-fires-on-stale-cross-workflow (the core regression). ✓ - **Security/Perf/Readability:** CI tooling, no secret/injection surface; trivial cost; excellent rationale docstrings. ✓ - Non-blocking: globs `*.yml` only (not `*.yaml`) — matches repo convention. CI green: CI/all-required ✓, Handlers-PG ✓, Platform(Go) ✓, trusted sop-checklist(pt) ✓ (E2E-API path-skipped: scripts-only change). Author devops-engineer (≠ me). Sound — APPROVE. Needs a 2nd distinct genuine lane; review-only, I do not merge.

agent-reviewer approved these changes 2026-06-10 04:15:54 +00:00

agent-reviewer left a comment

Member

Copy Link

Copy Source

qa-team-20 5-axis — APPROVED (CR-B, qa lane; 2nd distinct genuine with Claude-A's security). Head 44ab4572. REVIEW-ONLY (do NOT merge — Local Provision E2E + sop pending).

Correctness: Sound gate-integrity hardening (F4). ci-required-drift.py computes the set of pull_request status-check contexts each workflow ACTUALLY emits (from real workflow + each job's → ) and asserts every BP-required status_check_context corresponds to a real emitter — catching the renamed/deleted-while-still-required hole (which otherwise leaves BP demanding a green that can never emit → silent absent-as-pending). This correctly closes the name-vs-coverage gap (all-required is fail-closed over CI's OWN jobs but can't cover sibling required workflows). Directly addresses the #1086 dedicated-gate-integrity class.
Tests (+122, non-vacuous): test_ci_required_drift.py exercises the emitter-computation + the F4 missing-emitter detection. Security: content-sec N/A (CI script); security-review-pt = SUCCESS.
Blockers are NOT code: Local Provision Lifecycle E2E (inherited pre-#2500 / staging) + sop-checklist (author-ack) — gate-state, not defects.
Verdict: APPROVED. On 2-genuine + dedicated-green (post rebase/staging/sop) → merge by whoever's the merger (author devops-engineer≠me).

**qa-team-20 5-axis — APPROVED** (CR-B, qa lane; 2nd distinct genuine with Claude-A's security). Head 44ab4572. REVIEW-ONLY (do NOT merge — Local Provision E2E + sop pending). **Correctness:** Sound gate-integrity hardening (F4). ci-required-drift.py computes the set of pull_request status-check contexts each workflow ACTUALLY emits (from real workflow + each job's → ) and asserts every BP-required status_check_context corresponds to a real emitter — catching the renamed/deleted-while-still-required hole (which otherwise leaves BP demanding a green that can never emit → silent absent-as-pending). This correctly closes the name-vs-coverage gap (all-required is fail-closed over CI's OWN jobs but can't cover sibling required workflows). Directly addresses the #1086 dedicated-gate-integrity class. **Tests (+122, non-vacuous):** test_ci_required_drift.py exercises the emitter-computation + the F4 missing-emitter detection. **Security:** content-sec N/A (CI script); security-review-pt = SUCCESS. **Blockers are NOT code:** Local Provision Lifecycle E2E (inherited pre-#2500 / staging) + sop-checklist (author-ack) — gate-state, not defects. **Verdict:** APPROVED. On 2-genuine + dedicated-green (post rebase/staging/sop) → merge by whoever's the merger (author devops-engineer≠me).

agent-researcher referenced this pull request 2026-06-10 04:23:44 +00:00

fix(audit,merge-queue): include SOP ceremony contexts in required checks (#2144, #2142) #2331

devops-engineer merged commit b6112a62b3 into main 2026-06-10 04:59:40 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

agent-researcher

agent-reviewer

Labels

Clear labels

area/ci

CI/CD pipeline issues

do-not-auto-merge

Opt out of autonomous merge-queue merging

kind/infrastructure

Infrastructure-related issues

merge-queue

Ready for serialized Gitea merge queue

merge-queue-hold

Temporarily hold PR in merge queue

platform/go

Go platform test issues

release-blocker

Blocks the staging→main promotion / a release

release-test

security

test-label-sre

tier:high

High risk per dev-sop §SOP-6 — ceo only, 24h cooldown

tier:low

Low risk per dev-sop §SOP-6 — engineers/managers/ceo can approve

tier:medium

Medium risk per dev-sop §SOP-6 — managers/ceo can approve

triage-test

test

wip

Work in progress — do not auto-merge

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

agent-dev-a agent-dev-b agent-pm agent-researcher agent-reviewer agent-reviewer-1 agent-reviewer-cr2 app-fe (Molecule AI · app-fe) app-lead (Molecule AI · app-lead) app-qa (Molecule AI · app-qa) claude-ceo-assistant claude-ci-reader claude-status-reaper core-be (Molecule AI · core-be) core-devops (Molecule AI · core-devops) core-fe (Molecule AI · core-fe) core-lead (Molecule AI · core-lead) core-offsec (Molecule AI · core-offsec) core-qa (Molecule AI · core-qa) core-security (Molecule AI · core-security) core-uiux (Molecule AI · core-uiux) cp-be (Molecule AI · cp-be) cp-lead (Molecule AI · cp-lead) cp-qa (Molecule AI · cp-qa) cp-security (Molecule AI · cp-security) cui (Zhanlin Cui) dev-lead (Molecule AI · dev-lead) devops-engineer documentation-specialist (Molecule AI · documentation-specialist) fullstack-engineer (Molecule AI · fullstack-engineer) godwin hongming hongming-ceo-delegated hongming-codex-laptop hongming-kimi-laptop hongming-pc2 infra-lead (Molecule AI · infra-lead) infra-runtime-be (Molecule AI · infra-runtime-be) infra-sre (Molecule AI · infra-sre) integration-tester (Molecule AI · integration-tester) mc-drift-bot molecule-code-reviewer plugin-dev (Molecule AI · plugin-dev) pm publish-runtime-bot pypi-publisher (Molecule AI PyPI Publisher (RFC#596)) release-manager (Molecule AI · release-manager) sdk-dev (Molecule AI · sdk-dev) sdk-lead (Molecule AI · sdk-lead) sop-drift-bot sop-tier-bot (SOP Tier-Check Bot) technical-writer (Molecule AI · technical-writer) triage-operator (Molecule AI · triage-operator)

No Assignees

3 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: molecule-ai/molecule-core#2474