diff --git a/.github/required-contexts.txt b/.github/required-contexts.txt index 5c6f553a..b0e954f3 100644 --- a/.github/required-contexts.txt +++ b/.github/required-contexts.txt @@ -15,13 +15,14 @@ # "nothing pending" would have reported green without ever waiting for a test leg, because with one # required context there was nothing to wait for. Count what SHOULD gate the merge, not what does. # -# The set then moved four more times the same day (1 -> 13 -> 12 -> 15 -> 12), and it moved again on -# 2026-08-31: `a reviewer has read this` was armed, and the two CodeQL matrix contexts were seen -# required earlier that day and are no longer. THE LIST BELOW IS SET-EQUAL TO THE SERVER -- fourteen -# contexts, nothing extra on either side -- read from the API at 2026-08-31 20:57 CDT. A set-equal -# read is the only one worth writing down: a count alone cannot tell a matching set from two -# offsetting errors. Two non-context settings also moved and are recorded because they change merge -# semantics: +# The set then moved four more times the same day (1 -> 13 -> 12 -> 15 -> 12); it moved on 2026-08-31, +# when `a reviewer has read this` was armed and the two CodeQL matrix contexts stopped being required; +# and it moved again on 2026-09-04, when the owner DE-REQUIRED `a reviewer has read this`. That last +# one is recorded with its cost in the review-gate.yml entry at the foot of this file. THE LIST BELOW +# IS SET-EQUAL TO THE SERVER -- thirteen contexts, nothing extra on either side -- read from the API at +# 2026-09-04 18:48 CDT. A set-equal read is the only one worth writing down: a count alone cannot tell +# a matching set from two offsetting errors. Two non-context settings also moved and are recorded +# because they change merge semantics: # # strict = TRUE -- a PR must be up to date with `main` before it can merge. This closes the # stale-green hole: previously a PR validated against an older base could merge @@ -30,7 +31,8 @@ # enforce_admins = TRUE -- an admin's direct push to `main` is refused by the server, not merely by # the local hook. It was ON from 2026-07-28, turned OFF on 2026-07-29 through the # escape hatch in scripts/hooks/push_guard.py (for when a permanently red required -# check blocks every merge), and READ BACK AS TRUE on 2026-08-31. +# check blocks every merge), READ BACK AS TRUE on 2026-08-31, and TRUE again in +# the 2026-09-04 read above. `strict` re-read TRUE in that same call. # # THE HOOK ITSELF IS ALREADY CORRECTED. scripts/hooks/push_guard.py's module # docstring and its user-facing warning were rewritten in this same change, and @@ -152,59 +154,45 @@ a PR that implements BACKLOG #N must update BACKLOG.md # prevent. Adding to protection is all-or-nothing: the REST endpoint 422s with `already_exists` if any # context in the request is already required, and then adds NONE of them, so send only the missing ones. -# --- review-gate.yml -------------------------------------------------------------------------------- -# ARMED 2026-08-31 (BACKLOG #1404), and this line is the second half of that arming -- protection moved -# first, this file followed, which is the order the paragraph directly above prescribes. -# -# THIS ONE CONTEXT IS THE REPOSITORY'S ENTIRE REVIEW REQUIREMENT, AND THERE IS NOTHING BEHIND IT. -# `required_approving_review_count` is 0 and has to stay 0: every session on this machine pushes as ONE -# GitHub identity (`gh` auth lives at AppData\Roaming\GitHub CLI\hosts.yml, machine-wide rather than per -# config root), CODEOWNERS assigns every path to that same account, and GitHub refuses to let an author -# approve their own pull request -- so raising the count to 1 wedges every pull request permanently -# instead of reviewing any of them. review-gate.yml's header carries that measurement. With approvals -# pinned at 0, dropping this context from protection does not weaken review, it REMOVES it: nothing -# else anywhere reports that a green pull request was never read. -# -# What it enforces is a STEP, not an identity. `gh pr edit --add-label reviewed` is the whole -# protocol; nothing automated ever adds the label, and a `synchronize` event REMOVES it, so commits -# nobody has read are unread again. It does not establish that an independent party looked, and no -# prose here or anywhere else may describe it as if it does. -# -# SAFE TO REQUIRE, which for this file means one thing: the required-but-absent trap. A required check -# that never reports blocks every pull request forever, so three separate ways it could fail to report -# were checked before arming, and all three came back clean: -# -# * IT ALWAYS DISPATCHES. The trigger is `pull_request` with NO paths filter, and the `reviewed` job -# carries no `if:` of its own. The `if:`s in that workflow sit on STEPS, inside a job that always -# runs -- and a skipped step still leaves the job reporting a conclusion, which is what branch -# protection reads. -# * IT REPORTS IN THE QUEUE. review-gate.yml declares `merge_group:`. Omitting that is the total -# failure codeql.yml's own header records under BACKLOG #340: a required context whose workflow has -# no merge_group trigger never reports on a queue entry, so NOTHING MERGES. Measured there on the -# first entry the queue ever held -- 32 check-runs green and three contexts simply absent. -# * IT REPORTS ON PULL REQUESTS OLDER THAN ITSELF. GitHub runs a `pull_request` workflow from the -# MERGE commit, not from the head, so a branch that predates the workflow still gets the check. -# VERIFIED on PR 713: its head 5cb51e46 returns 404 for .github/workflows/review-gate.yml, and the -# context reported on it anyway. -# -# SO ARMING IT STRANDED NOTHING, and the remedy is not a rebase. Every open pull request goes red on -# this one check until a reviewer labels it; `labeled` is in the trigger list, so the label alone -# re-runs the job and clears it. That distinction is the one worth keeping: a reader who assumes a -# stranded check needs a rebase will rebase, which on `synchronize` STRIPS the label and moves the pull -# request further from green. -# -# THAT SENTENCE IS ONLY TRUE WHILE `labeled` IS IN THE WORKFLOW'S `types:` LIST, so it is held by a -# test rather than by this paragraph: -# -# tests/test_merge_gate_controls.py::test_the_review_gate_reruns_when_a_reviewer_adds_the_label -# -# Deleting that action reddens nothing on its own and leaves a gate no reviewer action can clear: the -# label fires no run, and the only trigger that still re-runs the job is `synchronize`, whose first -# step REMOVES the label. Measured 2026-08-31 -- before the test above existed, dropping `labeled` -# passed 65 tests. -a reviewer has read this - # DELIBERATELY NOT REQUIRED (do not add without the owner's decision; each has a recorded reason): +# review-gate.yml -- DE-REQUIRED 2026-09-04 by the owner, having been armed on 2026-08-31 +# (BACKLOG #1404). The workflow is untouched: it still dispatches, and +# it still reports `a reviewer has read this`. It no longer BLOCKS, so +# a red one on a pull request is not a merge problem. +# +# WHAT THAT COSTS, recorded because nothing else on the merge path now +# records it. `required_approving_review_count` is 0 and has to stay 0 +# -- every session here pushes as ONE GitHub identity and GitHub +# refuses to let an author approve their own pull request, so raising +# it to 1 wedges every pull request instead of reviewing any; +# review-gate.yml's header carries that measurement. This context was +# therefore the repository's ENTIRE review requirement. With it +# de-required, NOTHING on the merge path reports that a green pull +# request was never read. That is the owner's call, recorded here +# rather than argued. +# +# The `reviewed` label protocol is correspondingly unenforced. `gh pr +# edit --add-label reviewed` still works and still means what it +# meant -- a STEP happened, never that an independent party looked -- +# but no gate reads it any more. +# +# THE WORKFLOW'S OWN PROPERTIES ARE STILL HELD BY TESTS, in +# tests/test_merge_gate_controls.py: it reports under this exact +# context string, it declares `merge_group:`, and `labeled` is in its +# `types:` so a reviewer's label re-runs the job. Those tests stay. +# What they are no longer is NEGATIVE CONTROLS: the registry at +# tests/negative_controls.toml indexes controls by REQUIRED context +# and its reconciliation fails on a control naming a context nobody +# requires, so that entry came out in this same change and the tests +# themselves were kept. +# +# RE-ARMING IS AN OWNER DECISION, and the three ways it could have hit +# the required-but-absent trap were measured clean before 2026-08-31 +# and are unchanged: no paths filter and no job-level `if:`, a +# `merge_group:` trigger (whose absence is the total failure +# codeql.yml's header records under BACKLOG #340), and it reports on +# branches older than itself -- verified on PR 713, whose head +# 5cb51e46 404s for the workflow file and got the check anyway. # codeql.yml, scorecard.yml -- their SARIF upload needs `security-events: write`, which fork-PR # tokens lack, so requiring them would block every fork PR. # `CodeQL (python)` and `CodeQL (javascript-typescript)` were read as diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 47ca7f36..4b8e1e1f 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -22560,6 +22560,7 @@ So `test_the_script_prefers_its_own_repo_over_an_earlier_path_entry` supplies th **Adjacent and NOT fixed here, named rather than numbered.** `docs/WEBCONSOLE-PACKAGE.md`'s seam-refresh procedure is stale in three steps left behind by #1220: it says to bump `ENGINE_UI_SEAM` by hand (`1` to `2`) when the value is a derived digest, it says to update curated lists in this script that #1220 retired, and its step 5 prescribes `python scripts/webconsole_seam_snapshot.py > tests/golden/...`, the shell redirect this script's own docstring forbids because PowerShell's `>` writes UTF-16LE with a BOM into a file the test reads as UTF-8. That is doc drift with its own cause and it wants its own item; folding a documentation rewrite into a `sys.path` fix would make both harder to review. + ## 1434. the tooling-partition gate names the file it rejects and not the remedy, so the Builder who trips it has already exited > 🚧 **Filed 2026-09-03. The failure message is fixed on this branch. Five open pull requests still carry unregistered test files, and only their own branches can fix them -- they are named below.** `tests/test_tooling_partition.py::test_every_non_engine_test_is_classified` fails when a file matching `tests/test_*.py` sits in neither `tests/tooling_manifest.txt` nor `_STAYS_WITHOUT_IMPORTING`. It is not deselected by `-m 'not tooling'`, so it reds **all three required `test` legs at once**. PR 774 hit exactly that, and its Builder's process had exited before any leg reported. @@ -22627,3 +22628,33 @@ All 137 listed entries were swept for a repo-rooted read of `messagefoundry/**`. 1. **The five pull requests above.** A Builder must not push to another seat's branch, so each is fixed by whoever next touches it: append `tests/.py` to `tests/tooling_manifest.txt`, keeping the list alphabetical. **The gate is passable and two pull requests that night passed it correctly -- nothing here asks for it to be weakened.** 2. **Three copies of the manifest parser, pinned against nothing.** This file, `_tooling_basenames` in `tests/conftest.py` (the copy that actually applies the marker), and `_manifest_paths` in `tests/test_ci_tooling_gate.py` all implement the same rule. Extracting one `tests/_tooling_manifest.py` is the real fix. Related: `test_every_manifest_entry_trips_its_own_gate` in `tests/test_ci_tooling_gate.py` feeds each manifest entry back as its own changed path, so on the manifest arm every entry matches itself unconditionally. + +## 1452. required-contexts.txt and its pin still claim the retired reviewer context is required + +> 🔢 **Filed 2026-09-04 - FIXED IN THIS PULL REQUEST, not yet landed.** The owner removed `a reviewer has read this` from `main`'s branch protection earlier the same day. The checked-in claim did not follow it, so the repository's own answer to "does this check block a merge" over-reported the required set by exactly one context. + +**Cluster:** repository tooling. **Priority:** P3. **Verdict:** build. +**Severity:** no engine effect, no PHI axis, and **no deployment axis (sec. 0)** -- none of these files ship in a wheel. The cost is a merge-path claim that was wrong, in the one file whose entire purpose is to be that answer readable from a clone. + +**MEASURED 2026-09-04 18:48 CDT** against `gh api repos/MEFORORG/MessageFoundry/branches/main/protection`: the server requires **13** contexts, `.github/required-contexts.txt` named **14**, and the difference is exactly `a reviewer has read this`. The other twelve matched, both directions. `strict`, `enforce_admins` and `required_approving_review_count` re-read TRUE, TRUE and 0 in the same call, unchanged. + +**THE DIRECTION OF THE ERROR IS THE UNUSUAL PART, AND IT IS THE SAFER ONE.** The failure this file was built against is the file naming FEWER contexts than the server, because a context the file omits reads as not-blocking -- the reassuring answer rather than the true one. This drift ran the other way: the file claimed a gate that nothing enforced. That still misdirects, and it does so about review specifically, which is the one gate that never tested the code. + +**ONE INSTRUMENT SAW IT, AND IT IS DELIBERATELY NOT A REQUIRED ONE.** `tests/test_required_contexts.py` compares the file against prose, workflow job names and a hand-written count, so all nine of its tests stayed green throughout while the claim was false. That is by construction rather than a defect in it: the `gh api` line beside its count pin is a COMMENT recording a past manual read, not a call. `scripts/ci/check_required_contexts_drift.py` does read the server and fails closed, and it was RED on `main` for this drift -- it is not in the required 13, so it reported and blocked nothing. Watched fail 2026-09-04: exit 1 against the pre-fix file, naming that context, and exit 0 against the corrected file at 13 against 13. + +**WHAT THE CORRECTION HAD TO TOUCH, which is wider than the two lines it looks like.** Removing one context from the canonical file moves five in-repo claims that are reconciled against it: + +| File | What moved | +|---|---| +| `.github/required-contexts.txt` | the context line, the set-equal header read, and a new DELIBERATELY NOT REQUIRED entry recording the de-requiring and its cost | +| `tests/test_required_contexts.py` | the pinned count, 14 to 13, plus the module docstring | +| `docs/CI.md` | the required-checks bullet and the review-gate paragraph | +| `tests/test_security_posture.py` | 13 contexts resolving to 11 distinct jobs, was 14 over 12 | +| `tests/negative_controls.toml` | the review-gate control entry, because the registry refuses a control naming a context nobody requires | + +**THE NEGATIVE CONTROLS WERE DE-REGISTERED, NOT DELETED.** `review-gate.yml` still exists and still runs, so the nine test nodes that entry named still hold its real behaviour inside the required `test` legs. What ended is their status as controls for a merge gate. One assertion did have to go: `test_the_review_gate_still_reports_under_the_required_context_string` asserted the context was required, which is now false; it kept its three structural assertions, was renamed to `..._under_its_declared_context_string`, and its docstring records that a re-arm is caught by the registry reconciliation rather than duplicated there. + +**RESIDUE, NAMED RATHER THAN NUMBERED, because retiring any of it is an owner decision this seat cannot take.** `CLAUDE.md` section 5 still describes the Reviewer seat and still states that the `reviewed` label is what blocks a merge. `.github/workflows/unread-signal.yml` and `scripts/ci/check_unread_prs.py` exist to notify a reviewer that a pull request is waiting on a gate that now blocks nothing. `review-gate.yml` itself still reddens an unlabelled pull request, which is expected and harmless but reads as a failure. Whether any of that is retired, and whether the context is re-armed, is the owner's call. + +**TWO NUMBERS WERE BURNED ON THIS SUBJECT BEFORE THIS ONE, AND NEITHER IS FILED.** A peer seat allocated 1449 for the same drift and left it unfiled. This seat then allocated 1451 from the PRIMARY checkout by mistake: ledger ownership keys on the allocating worktree or its branch, both non-transferable, so 1451 is claimed by `main` in a tree that must never commit it and is unusable here. 1452 was re-allocated from this worktree and is the only one of the three that names this work. Recorded so a later reader does not read three numbers as three defects; `scripts/coord/alloc_strand_sweep.py` is the tool that already exists for stranded allocations. + diff --git a/docs/CI.md b/docs/CI.md index 4eadadfd..16135210 100644 --- a/docs/CI.md +++ b/docs/CI.md @@ -51,7 +51,6 @@ The stable contexts required on `main` are — mirroring - `forbidden-content (customer/PHI leak guard)` - `a PR that implements BACKLOG #N must update BACKLOG.md` - `cla` -- `a reviewer has read this` `cla` is the **job key** in `cla.yml`, whose job declares no `name:`. Branch protection matches the job name, never the workflow name — so the context is `cla`, not "CLA Assistant". Every @@ -69,12 +68,13 @@ the same reason and additionally **does not run on PRs at all** (`scorecard.yml` trigger — it runs on push-to-main, a schedule, and branch-protection changes). Nightly / path-gated legs (service-smoke, load, SQL/Postgres store) are deliberately **not** required. -`a reviewer has read this` (`review-gate.yml`) is the required check that is **not a test of the code**, -and the one with nothing behind it. `required_approving_review_count` is 0 and stays 0 — every session -pushes as one GitHub identity, so a human-approval rule would wedge every PR rather than review any — -which makes this single context the repository's whole review requirement. A PR clears it with -`gh pr edit --add-label reviewed`; a new commit removes the label, so re-review is automatic. It -proves a **step happened**, not that an independent party looked. +`a reviewer has read this` (`review-gate.yml`) was **de-required by the owner on 2026-09-04**, having +been required since 2026-08-31. **The workflow still runs**, so a PR with no `reviewed` label still +shows that check red — it just does not block a merge any more, and it is not something to chase. +`required_approving_review_count` is 0 and stays 0, because every session pushes as one GitHub identity +and a human-approval rule would wedge every PR rather than review any. That made this one context the +repository's whole review requirement, so **nothing on the merge path now reports that a green PR was +never read**. `.github/required-contexts.txt` records that call and its cost; do not restate it here. The `quality-advisory.yml` jobs create **no code-scanning category** and **no _required_ check context** — they do report as ordinary advisory checks, and they **must never be added to the required list**. Two diff --git a/tests/negative_controls.toml b/tests/negative_controls.toml index b53c0459..b52a19df 100644 --- a/tests/negative_controls.toml +++ b/tests/negative_controls.toml @@ -403,72 +403,29 @@ deliberately hostile ambient (PYTHONIOENCODING=cp1252, PYTHONUTF8=0, LC_ALL=tr_T core.autocrlf, and PATH ordered so the WSL bash comes first). """ -# --- review-gate.yml ------------------------------------------------------------------------------- - -[[control]] -context = "a reviewer has read this" -plants = """ -Six label sets that are not a review -- none at all, unrelated ones, and four NEAR-MISSES -(`reviewed-by-bot`, `not-reviewed`, `Reviewed`, `re,viewed`) -- plus the stale payload: a `synchronize` -whose event still carries `reviewed` because the removal step ran one step ago. The gate's OWN shell is -lifted out of the workflow and run under the flags Actions uses, not re-implemented; a second copy of -that `case` would be free to agree with itself. Four structural plants sit beside them, because the -quieter death of this gate is the context never arriving or never clearing: a renamed job, a job-level -`if:`, a trigger set that cannot report on a pull request or in the merge queue, and a `types:` list -that does not re-run the job when the reviewer adds the label. -""" -red = [ - "tests/test_merge_gate_controls.py::test_the_review_gate_refuses_a_pull_request_nobody_has_marked_read", - "tests/test_merge_gate_controls.py::test_the_review_gate_refuses_a_synchronize_even_when_the_payload_shows_the_label", - "tests/test_merge_gate_controls.py::test_the_review_gate_still_reports_under_the_required_context_string", - "tests/test_merge_gate_controls.py::test_nothing_in_the_review_gate_adds_the_label_it_checks_for", - "tests/test_merge_gate_controls.py::test_the_review_gate_reruns_when_a_reviewer_adds_the_label", -] -holds = """ -A labelled pull request must PASS, in every position the token can occupy in the joined label list -- -first, last, middle and alone. A gate that refused uniformly would satisfy every planted case above -while wedging every pull request in the repository, permanently, because `strict = true` means a -blocked pull request cannot even be brought up to date past it. The merge queue is the other half: a -merge_group entry carries no pull request and therefore no labels, so the label-reading step must stay -confined to pull_request events -- a required context that can never go green in the queue means -NOTHING MERGES, which codeql.yml's header records happening here. The trigger list has the same shape -of asymmetry: every action the gate depends on must be named individually when it is dropped, while a -list that only ADDS actions must stay clean. -""" -green = [ - "tests/test_merge_gate_controls.py::test_the_review_gate_passes_a_pull_request_a_reviewer_has_marked_read", - "tests/test_merge_gate_controls.py::test_the_review_gate_lets_a_merge_queue_entry_through", - "tests/test_merge_gate_controls.py::test_the_absence_detector_fires_on_a_trigger_set_that_can_go_quiet", - "tests/test_merge_gate_controls.py::test_the_label_rerun_detector_fires_on_a_types_list_that_ignores_the_label", -] -observed = """ -RUN AGAINST A NEUTERED GATE, 2026-08-31, and the result is SPECIFIC rather than uniform, which is the -part that makes it evidence. Two separate neuterings of review-gate.yml, each reverted from the same -run: - - * the `*,reviewed,*)` case arm widened to `*)`, so the gate can never refuse -- 1 of the 11 selected - controls red, 10 green. - * the synchronize comparison changed to a value that never occurs - (`[ "$ACTION" = "never-happens" ]`), which leaves the gate reading a payload that is stale by one - step -- 1 red, 10 green, and the one that reddened is the stale-payload control specifically. - -A THIRD SPELLING OF THE SAME NEUTERING IS REPORTED AS A HARNESS FAULT, DELIBERATELY. Replacing the -test with `if false` removes `$ACTION` from the script, and the fixture refuses to run a script that -ignores the input it is being fed -- three ERRORS rather than three passes. A control that fed input to -a script which cannot read it would be green about nothing (BACKLOG #1216). - -The gate has also been seen red in ordinary use: arming the context on 2026-08-31 turned every open -pull request red on this one check until a reviewer labelled it. - -A THIRD NEUTERING WAS FOUND BY IT SURVIVING, 2026-08-31, which is the only reason it is written down. -Deleting `labeled` from review-gate.yml's `types:` list reddened NOTHING -- 65 tests passed with the -gate made permanently unclearable, because the absence detector reads `pull_request`, `merge_group` and -`paths` and never looked at `types`. With the trigger-list control added, the same edit reds exactly -ONE test by name -- test_the_review_gate_reruns_when_a_reviewer_adds_the_label -- and 62 pass. Exactly -one, not two: the detector's own control uses literal trigger lists rather than subtracting from the -shipped file, so it does not move with the workflow it judges and cannot bury the naming red under a -second one that is only bookkeeping. -""" +# --- review-gate.yml: NO ENTRY, BECAUSE ITS CONTEXT IS NO LONGER REQUIRED --------------------------- +# +# `a reviewer has read this` was armed on 2026-08-31 (BACKLOG #1404) and DE-REQUIRED by the owner on +# 2026-09-04. Its control block came out here in that same change, for the reason the codeql note at +# the foot of this file gives: this registry reconciles against .github/required-contexts.txt, and an +# entry for a context that file does not list fails `every registered context is required`. +# +# THE TESTS THEMSELVES WERE KEPT, and that distinction is the whole point. review-gate.yml still exists +# and still runs; what ended is its power to block a merge. So the nine nodes this entry used to name +# still live in tests/test_merge_gate_controls.py and still execute inside the required `test` legs -- +# they are ordinary tests of a workflow now, not negative controls for a merge gate. Deleting them to +# satisfy a bookkeeping rule would have thrown away watched-fail evidence. +# +# WHAT THAT EVIDENCE WAS, kept here because it is not reproducible from the tests alone and would +# otherwise leave with this entry. Two neuterings of review-gate.yml on 2026-08-31, each reverted from +# the same run, reddened 1 of 11 selected controls and left 10 green -- SPECIFIC rather than uniform, +# which is what made them evidence rather than flattery. A third was found by SURVIVING: deleting +# `labeled` from the workflow's `types:` reddened NOTHING, 65 tests passing with the gate made +# permanently unclearable, because the absence detector read `pull_request`, `merge_group` and `paths` +# and never `types`. The trigger-list control added in response reds exactly one test, by name. +# +# RE-ARMING THE CONTEXT MEANS RESTORING AN ENTRY HERE, and the reconciliation says so on its own: a +# required context with no control is a failure, so nothing returns to the merge path unproven. # --- codeql.yml: NO ENTRY, BECAUSE ITS CONTEXTS ARE NOT REQUIRED ----------------------------------- # diff --git a/tests/test_merge_gate_controls.py b/tests/test_merge_gate_controls.py index bef3435a..3e06598b 100644 --- a/tests/test_merge_gate_controls.py +++ b/tests/test_merge_gate_controls.py @@ -2,17 +2,24 @@ # Copyright (C) 2026 MessageFoundry Organization and contributors """Negative controls for the required merge contexts that had none (BACKLOG #1000). -A GATE NOBODY HAS WATCHED FAIL IS AN ASSUMPTION WEARING A GREEN TICK. Fourteen contexts are the entire -merge gate as of 2026-08-31, and several of them were guarded only by the property that they exist. +A GATE NOBODY HAS WATCHED FAIL IS AN ASSUMPTION WEARING A GREEN TICK. Thirteen contexts are the entire +merge gate as of 2026-09-04, and several of them were guarded only by the property that they exist. This file supplies the missing plant-and-observe controls; ``tests/negative_controls.toml`` records which control belongs to which context and ``tests/test_negative_controls.py`` fails when a context has none. THE COUNT IS RECORDED HERE, NEVER DERIVED HERE. ``.github/required-contexts.txt`` is the checked-in -claim and the only thing this file reads. It said thirteen until ``a reviewer has read this`` was -reconciled onto it (BACKLOG #1404), and that is exactly how a context arrives here unproven: the -reconciliation is driven by the LIVE required set, so a newly-required context lands with zero controls -and SAYS SO, rather than quietly not being looked at. +claim and the only thing this file reads. It said thirteen, then fourteen when ``a reviewer has read +this`` was reconciled onto it (BACKLOG #1404), and thirteen again when the owner de-required that +context on 2026-09-04. The churn is the point: the reconciliation is driven by the LIVE required set, +so a newly-required context lands with zero controls and SAYS SO, rather than quietly not being looked +at. + +NOT EVERY TEST HERE BACKS A REQUIRED CONTEXT ANY MORE. The review-gate section at the foot is the case: +``review-gate.yml`` still exists and still runs, so those tests still hold its real behaviour, but they +stopped being NEGATIVE CONTROLS the moment its context left the required set -- the registry indexes by +required context and refuses an entry naming one nobody requires. They were kept rather than deleted, +because watched-fail evidence is not bookkeeping. EVERY CONTROL HERE IS ASYMMETRIC, and that is the part most likely to be skipped. It is not enough that neutering a rule turns a control red -- the control must fail for exactly the shapes that rule covers @@ -48,7 +55,7 @@ import pytest from _bash_resolver import CANNOT_RUN_CODES, bash_sees, explain_returncode, require_bash -from tests._workflow_contexts import ROOT, jobs_of, load_workflow, required_contexts +from tests._workflow_contexts import ROOT, jobs_of, load_workflow _TIMEOUT = 300 @@ -1019,11 +1026,16 @@ def test_the_absence_detector_fires_on_a_trigger_set_that_can_go_quiet() -> None # `a reviewer has read this` -- run as the SHIPPED SHELL against planted label sets. # =================================================================================================== # -# ARMED 2026-08-31 (BACKLOG #1404). With `required_approving_review_count` pinned at 0 -- every -# session on this machine pushes as one GitHub identity, so a human-approval rule would wedge every -# pull request rather than review any -- this single context is the repository's ENTIRE review -# requirement. Nothing else anywhere reports that a green pull request was never read, so a gate that -# could not go red here would not be a weakened control, it would be the absence of one. +# ARMED 2026-08-31 (BACKLOG #1404), DE-REQUIRED by the owner 2026-09-04. THE WORKFLOW IS UNCHANGED -- +# it still dispatches and still reports -- so everything below still describes live behaviour; what +# ended is the context's power to block a merge, which is why these are no longer registered in +# tests/negative_controls.toml. Expect a red `a reviewer has read this` on an unlabelled pull request: +# it gates nothing now. +# +# WHY THEY ARE KEPT ANYWAY. With `required_approving_review_count` pinned at 0 -- every session on this +# machine pushes as one GitHub identity, so a human-approval rule would wedge every pull request rather +# than review any -- this context WAS the repository's entire review requirement, and nothing replaced +# it. Re-arming is an owner decision; these tests are what would make it provable rather than hopeful. # # The gate's OWN shell is lifted out of the workflow and run, never re-implemented: a second copy of # that `case` would be free to agree with itself. @@ -1163,22 +1175,27 @@ def test_the_review_gate_passes_a_pull_request_a_reviewer_has_marked_read( ) -def test_the_review_gate_still_reports_under_the_required_context_string() -> None: +def test_the_review_gate_still_reports_under_its_declared_context_string() -> None: """The other way this gate dies, and the quieter one: the context stops arriving at all. Three surfaces, every one of them in-repo. The job NAME is the context string; a job-level `if:` - would let the job skip, and a skipped required check reports nothing; the trigger set decides - whether it reports on a pull request and in the merge queue. The `cla` control is this same shape - for the same reason -- where the context string is the surface, the string is what to pin. + would let the job skip, and a skipped check reports nothing; the trigger set decides whether it + reports on a pull request and in the merge queue. The `cla` control is this same shape for the + same reason -- where the context string is the surface, the string is what to pin. + + IT NO LONGER ASSERTS THAT THE CONTEXT IS REQUIRED, because since 2026-09-04 it is not. This test + kept its three structural assertions and dropped only that one, so what it now holds is that + re-arming would WORK: a job whose name drifted, or which could skip, or which could go absent in + the queue, is a context that cannot be put back on the merge path without wedging every pull + request. A re-arm is caught elsewhere and deliberately not duplicated here -- the registry + reconciliation in tests/test_negative_controls.py fails on a required context with no control, so + this gate cannot quietly return to blocking unproven. """ job = jobs_of(_REVIEW_GATE)[_REVIEW_GATE_JOB] - assert _REVIEW_GATE_CONTEXT in required_contexts(), ( - f"{_REVIEW_GATE_CONTEXT!r} left .github/required-contexts.txt. With approvals pinned at 0 " - "that does not weaken review, it removes it." - ) assert str(job.get("name")) == _REVIEW_GATE_CONTEXT, ( f"the job name is {job.get('name')!r}, so it reports under a different context string than " - f"branch protection requires. {_REVIEW_GATE_CONTEXT!r} would never report and every pull " + f"{_REVIEW_GATE_CONTEXT!r}. While that context is not required this reports nothing worse " + "than a renamed check; the moment it is re-armed, it would never report and every pull " "request would wedge." ) assert "if" not in job, ( diff --git a/tests/test_required_contexts.py b/tests/test_required_contexts.py index ce9bb045..568c6c68 100644 --- a/tests/test_required_contexts.py +++ b/tests/test_required_contexts.py @@ -12,19 +12,33 @@ and under a different string than its own line 18 gives tests/test_push_guard.py 12 -The live API said 12 when those five were counted. It says 14 at 2026-08-31 20:57 CDT. That question +The live API said 12 when those five were counted. It says 13 at 2026-09-04 18:48 CDT. That question is not trivia here: ``required_approving_review_count`` is 0 and auto-merge is armed, so required-set membership is the ONLY thing separating "reviewed" from "merged unread". A session reasoning from docs/CI.md would conclude that gitleaks, semgrep, npm-audit and crypto-inventory are advisory -- i.e. that four blocking security gates were safe to weaken. -THE SET NOW HOLDS A CONTEXT THAT DOES NOT TEST THE CODE, and it is the one with the least behind it. -``a reviewer has read this`` (review-gate.yml, armed 2026-08-31, BACKLOG #1404) is this repository's -ENTIRE review requirement. It is load-bearing BECAUSE approvals are pinned at 0 rather than in spite of -it: every session pushes as one GitHub identity, so a human-approval rule would wedge every pull -request instead of reviewing any. Drop that context from protection and review is not weakened, it is -gone -- nothing else anywhere reports that a green pull request was never read. The count below is what -makes removing it a deliberate edit rather than a quiet one. +EVERY CONTEXT IN THE SET NOW TESTS THE CODE. The one that did not, ``a reviewer has read this`` +(review-gate.yml, armed 2026-08-31 under BACKLOG #1404), was DE-REQUIRED by the owner on 2026-09-04. +With approvals pinned at 0 -- every session pushes as one GitHub identity, so a human-approval rule +would wedge every pull request instead of reviewing any -- that context was the repository's entire +review requirement, and removing it did not weaken review so much as end it: nothing on the merge path +now reports that a green pull request was never read. ``.github/required-contexts.txt`` records the +owner's call and its cost; this suite only holds the file honest about it. + +THIS SUITE CANNOT SEE THE SERVER, WHICH IS WHY THE DE-REQUIRING SAT HERE UNNOTICED. Everything in this +file reads the checked-in file, the workflows and the prose; the ``gh api`` line beside the count pin +is a COMMENT recording a manual read, not a call. So every test here stayed green while the file +overstated the required set by one context. A green run means the in-repo claims AGREE, never that they +are true. + +SOMETHING ELSE DOES READ THE SERVER, and it is the instrument to reach for when the answer matters: +``scripts/ci/check_required_contexts_drift.py`` reconciles the canonical file against live branch +protection and fails closed, run by the drift job in ``.github/workflows/required-workflow-state.yml``. +It is deliberately NOT one of the required contexts, so it reports without blocking -- it went red on +`main` for this very drift while this suite was green, which is the division of labour working, not a +gap. Watched fail 2026-09-04: against the pre-fix file it exited 1 naming ``a reviewer has read this``, +and against the corrected file it exited 0 on 13 versus 13. WHAT THIS PINS. ``.github/required-contexts.txt`` is the checked-in claim; these tests assert every in-repo statement agrees with it. The file is NOT the enforcement -- the server is -- so when branch @@ -107,16 +121,21 @@ def test_the_canonical_file_parses_and_names_the_live_set() -> None: ) # Pinned so that ADDING or REMOVING a required check is a deliberate, reviewed edit here rather # than a silent one. Verified against `gh api repos/MEFORORG/MessageFoundry/branches/main/protection` - # at 2026-08-31 20:57 CDT: 14 contexts, SET-EQUAL to the file with nothing extra on either side. + # at 2026-09-04 18:48 CDT: 13 contexts, SET-EQUAL to the file with nothing extra on either side. # Set-equal is the reading worth recording -- a count alone cannot tell a matching set from two # errors that cancel. # + # THE DATE IS NOT DECORATION AND MUST NOT BE COPIED FORWARD. Re-read the API and write down what + # THAT call returned; a stamp carried over from the last edit claims a measurement nobody took. + # # THE PIN GOES STALE IN THE DIRECTION THAT LOOKS FINE. It read 13 while the server held more, and # a context this file omits reads as NOT BLOCKING -- the reassuring answer rather than the true # one. A count that only ever fails when someone edits the FILE cannot notice the server moving - # underneath it, so reconcile against the API, never against this number. The server moved four - # times on 2026-07-29 and twice more on 2026-08-31. - assert len(contexts) == 14, ( + # underneath it, so reconcile against the API, never against this number -- or let + # scripts/ci/check_required_contexts_drift.py do it, per the module docstring. The server moved + # four times on 2026-07-29, twice more on 2026-08-31, and again on 2026-09-04 when `a reviewer has + # read this` was de-required, which this pin over-reported as required until it was re-measured. + assert len(contexts) == 13, ( f"the canonical required set changed to {len(contexts)} contexts. If branch protection really " "changed, update this count AND every claim this suite checks; if it did not, revert the file." ) diff --git a/tests/test_security_posture.py b/tests/test_security_posture.py index e0e408d9..0ce35482 100644 --- a/tests/test_security_posture.py +++ b/tests/test_security_posture.py @@ -263,23 +263,27 @@ def test_required_jobs_carry_no_continue_on_error() -> None: # change in the collapse — a matrix split, or a context that quietly stops resolving — forces a # look here instead of passing on a self-consistent count. # - # 14/12 since 2026-08-31 (BACKLOG #1404), when `a reviewer has read this` was recorded. One - # collapse: ci.yml's `test` matrix reports 3 contexts from 1 job, so 14 - 2 = 12. + # 13/11 since 2026-09-04, when the owner DE-REQUIRED `a reviewer has read this` and review-gate's + # job left this module's scope with it. One collapse: ci.yml's `test` matrix reports 3 contexts + # from 1 job, so 13 - 2 = 11. (It was 14/12 from 2026-08-31, when that context was armed, #1404.) # # THIS MODULE READS THE CANONICAL FILE, NOT THE SERVER, so a context that reaches protection and # not the file is not examined here. That file's own header states the ordering rule — protection # first, the file in the same pull request — and this pin is only as live as that discipline. Read - # at 2026-08-31 20:57 CDT the two were SET-EQUAL, so nothing is currently unexamined. + # at 2026-09-04 18:48 CDT the two were SET-EQUAL, so nothing is currently unexamined; the + # instrument that keeps them so is scripts/ci/check_required_contexts_drift.py, not this file. # - # Recording a context drags its job under every rule in this module for the first time, exactly as - # backlog-hygiene did on 2026-07-29 — and it paid immediately here: the review-gate job's - # `|| true` surfaced on the first CI run after that context was recorded. + # DE-RECORDING A CONTEXT TAKES ITS JOB BACK OUT OF EVERY RULE HERE, which is the direction that + # loses coverage quietly: review-gate.yml still runs, and its steps are no longer read by this + # module at all. Recording one does the reverse, exactly as backlog-hygiene did on 2026-07-29 — + # and that paid immediately here, the review-gate job's `|| true` surfacing on the first CI run + # after its context was recorded. print( f"[security-posture] examined {examined} distinct jobs backing " f"{len(required_contexts())} required contexts" ) - assert examined == 12, ( - f"expected the 14 required contexts to resolve to 12 distinct jobs (the 3 `test` legs share " + assert examined == 11, ( + f"expected the 13 required contexts to resolve to 11 distinct jobs (the 3 `test` legs share " f"one matrix job); got {examined}. If the workflow layout genuinely changed, update this " "count." )