Skip to content

fix(ci): follow the server after the reviewer context was de-required (BACKLOG #1452) - #887

Open
wshallwshall wants to merge 2 commits into
mainfrom
claude/required-contexts-drift
Open

fix(ci): follow the server after the reviewer context was de-required (BACKLOG #1452)#887
wshallwshall wants to merge 2 commits into
mainfrom
claude/required-contexts-drift

Conversation

@wshallwshall

Copy link
Copy Markdown
Collaborator

What

The owner removed a reviewer has read this from main's branch protection on 2026-09-04. The checked-in claim did not follow, so .github/required-contexts.txt over-reported the required set by exactly one context. This makes the file, its pinned count, and the four other in-repo claims reconciled against it agree with the server again.

Filed as BACKLOG #1452, whose row is the last commit here.

The server, re-measured rather than taken on trust

gh api repos/MEFORORG/MessageFoundry/branches/main/protection, read at 2026-09-04 18:48 CDT:

Reading Value
required contexts 13
the file named 14
the difference exactly a reviewer has read this; the other twelve matched both ways
strict TRUE
enforce_admins TRUE
required_approving_review_count 0

Why this is wider than two lines

Removing one context from the canonical file moves five in-repo claims reconciled against it. I found them by running the suite and reading its failures, not by guessing.

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, since the registry refuses a control naming a context nobody requires
tests/test_merge_gate_controls.py header prose, and one assertion that the context is required

The negative controls were de-registered, not deleted. review-gate.yml still runs, so the nine nodes that entry named still hold its real behaviour inside the required test legs. test_the_review_gate_still_reports_under_the_required_context_string did assert the context was required, which is now false; it kept its three structural assertions and was renamed to ..._under_its_declared_context_string. A re-arm is still caught, by the registry reconciliation failing on a required context with no control.

An unrequired gate caught this, and the required suite could not

tests/test_required_contexts.py compares in-repo text to in-repo text, so all nine tests stayed green while the claim was false. scripts/ci/check_required_contexts_drift.py reads the server and fails closed, and it was red on main for this drift. It is deliberately not one of the 13, so it reported and blocked nothing.

Watched fail rather than argued:

  • pre-fix file: exit 1, naming a reviewer has read this under NAMED IN THE FILE, NOT REQUIRED ON THE SERVER
  • corrected file: exit 0, 13 against 13

Checks

Check Result
ruff check . passed
ruff format --check . 1241 files already formatted
mypy messagefoundry no issues in 267 source files
pytest tests/test_required_contexts.py 9 passed
pytest tests/test_push_guard.py passed
the seven coupled suites together 143 passed
check_required_contexts_drift.py exit 0, and exit 1 on the pre-fix file
backlog_status_check.py OK, 666 items each declaring one status
backlog_citation_check.py --base origin/main OK, 0 citations in scope
pre-commit, both commits all hooks passed, no --no-verify

Not run locally: the hosted-runner-only legs. windows-service-smoke and the server-DB legs need reading after CI reports.

Expect a red a reviewer has read this on this PR

The workflow still exists and still reddens an unlabelled pull request. It blocks nothing now. Do not label this PR to silence it, and do not treat it as a failure of this change.

CI is queue-deep at the time of writing, so every check will sit pending for roughly forty minutes. Pending is not nearly-green.

Deliberately not done

  • Branch protection is untouched. That is the Lander's.
  • review-gate.yml is untouched. Whether it is retired is an open owner decision.
  • CLAUDE.md is not rewritten here. It is now stale on this subject: section 5 still describes the Reviewer seat and still states that the reviewed label gates a merge. No test I had to satisfy forced the edit, so it stays a named follow-on rather than an unreviewed rewrite folded into a drift fix. .github/workflows/unread-signal.yml and scripts/ci/check_unread_prs.py are the same shape: they notify a reviewer about a gate that no longer blocks. The #1452 row records all three.

Two burned numbers, named so nobody reads three defects

  • 1449 was allocated by a peer seat for this same drift and left unfiled.
  • 1451 this seat allocated from the primary checkout by mistake. Ledger ownership keys on the allocating worktree or its branch and is non-transferable, so it is claimed by main in a tree that must never commit it. Unusable, and recorded rather than hidden.
  • 1452 was re-allocated from this worktree and is the only one naming this work. scripts/coord/alloc_strand_sweep.py already exists for stranded allocations.

🤖 Generated with Claude Code

wshallwshall and others added 2 commits September 4, 2026 19:06
The owner removed `a reviewer has read this` from main's branch protection on
2026-09-04. The checked-in claim did not follow, so it overstated the required
set by one context and read as blocking when nothing was.

Re-measured the server at 2026-09-04 18:48 CDT: 13 contexts, strict true,
enforce_admins true, approvals 0.

- .github/required-contexts.txt: drop the context, and record the de-requiring
  and its cost under DELIBERATELY NOT REQUIRED.
- tests/test_required_contexts.py: pin 13, not 14.
- docs/CI.md: drop the bullet, rewrite the review-gate paragraph.
- tests/test_security_posture.py: 13 contexts over 11 distinct jobs.
- tests/negative_controls.toml: remove the entry, since the registry refuses a
  control for a context nobody requires. The tests it named are kept, because
  review-gate.yml still exists and still runs.

review-gate.yml and branch protection are untouched.

Watched fail: scripts/ci/check_required_contexts_drift.py exits 1 on the pre-fix
file naming that context, and 0 on the corrected file, 13 against 13.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d here

Records the measurement, the five in-repo claims that had to move with the
canonical file, and why the review-gate negative controls were de-registered
rather than deleted.

Also records the residue nobody has ruled on: CLAUDE.md still describes the
Reviewer seat and the reviewed-label gate, and unread-signal.yml plus
check_unread_prs.py exist to notify a reviewer about a gate that now blocks
nothing.

Two earlier allocations on this subject are named so a later reader does not
read three numbers as three defects: 1449, allocated by a peer seat and left
unfiled, and 1451, which this seat allocated from the primary checkout by
mistake and cannot commit, because ledger ownership keys on the allocating
worktree or its branch.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the unread Green and mergeable, but not marked read. Read it, then add: reviewed label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

@wshallwshall

This pull request is green and nobody has marked it read.

Every check that has reported is passing and the reviewed label is absent, so
a reviewer has read this is the only thing between it and a merge. Nothing else reports that, which is
why this comment exists (BACKLOG #1413).

To clear it, IN THIS ORDER:

  1. If the branch is behind main, update it FIRST: gh pr update-branch --repo MEFORORG/MessageFoundry 887.
    That push fires synchronize, and synchronize REMOVES the reviewed label. Labelling
    before updating throws the label away and costs a round trip.
  2. Read the diff.
  3. gh pr edit 887 --repo MEFORORG/MessageFoundry --add-label reviewed

DO NOT TRIAGE THIS FROM mergeStateStatus. It returns one value with precedence, so BEHIND,
DIRTY and UNSTABLE each mask the missing check -- the requirement is invisible until you act on
something else. This check never reads that field; see scripts/ci/check_unread_prs.py.

A seat can find every pull request in this state in one call: gh pr list --label unread.
The label is withdrawn automatically once this pull request leaves the state.

The reviewed label is a PROCESS gate. It records that a step happened. It does not
establish that an independent party looked, and nothing here should be read as saying it does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unread Green and mergeable, but not marked read. Read it, then add: reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant