From 2554bfcbb9e8158db8b3989b07ff0afb61f54b10 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Wed, 29 Jul 2026 14:16:27 -0400 Subject: [PATCH] review round 2: arm the required-checks name-coupling trap, restore m6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P3 --- required status checks are name-coupled to job names, and a required context that no longer exists does NOT fail. It leaves every PR pinned on "Expected --- waiting for status" forever, which is `main` becoming unmergeable by policy rather than by a red run. Three of this lane's own deferrals will do exactly that: the macOS matrix trim removes two contexts outright, and nextest or the serial/parallel split rename or add jobs. The rule is now in the ledger entry --- any job rename, removal, or matrix change updates the branch-protection required-checks list in the same motion --- and it is recorded HERE deliberately, because this is the single entry that both enabled protection and named the lanes that will invalidate it. Arming the warning anywhere else would separate the trap from the thing that sets it. P4 --- the rewritten top comment said "everything else keeps 25 against a sub-4-minute observed max" and dropped the clause noting that `m6-perf-gates` keeps its own tighter 15. Restored. Worth the fixup in a change whose entire subject was comments matching reality. Beyond the PR, and taken here rather than deferred: `TEST_IMPROVEMENT.md` on `main` still said "no branch protection on `main` (verified via API: 404, so every job is advisory)" and listed §5.1 as open. Both went stale during this session, and THIS lane is what made them stale, so it carries the correction rather than leaving it for whoever touches the file next. Struck through in both places rather than rewritten: the 404 was a true reading at audit time, and the document is the arc's scoping record, so what changed is more useful than a clean-looking present tense. Note also that protection shipped wider than §5.1 proposed --- all 12 contexts required, not the cheap-jobs-only starter --- which the correction states. Verified: YAML parses; the seam check from update-protocol rule 6 finds no double blanks; `git diff --check` clean. --- .github/workflows/ci.yml | 3 ++- TEST_IMPROVEMENT.md | 12 +++++++++--- docs/active-work.md | 12 ++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4ab12a4..b2bb834 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,7 +49,8 @@ concurrency: # once, and a cold macOS debug build of this workspace plus the suite is # the plausible way a HEALTHY run exceeds its ceiling. The test job # therefore gets 35 rather than 25 — roughly 2x its observed max — while -# everything else keeps 25 against a sub-4-minute observed max. +# everything else keeps 25 against a sub-4-minute observed max, except +# `m6-perf-gates`, which keeps its own tighter 15. # # DIAGNOSIS, WRITTEN BEFORE IT HAPPENS: four test legs timing out # simultaneously, shortly after a Rust release, is a cold cache and not diff --git a/TEST_IMPROVEMENT.md b/TEST_IMPROVEMENT.md index cd8e8a1..3c332e9 100644 --- a/TEST_IMPROVEMENT.md +++ b/TEST_IMPROVEMENT.md @@ -42,6 +42,9 @@ hand before inclusion. on `push:main` + `pull_request` only. No coverage measurement, no scheduled runs, no branch protection on `main` (verified via API: 404, so every job is advisory). + **~~No branch protection~~ — CLOSED. Protection was enabled during + the arc; the API now reports 12 required contexts, `strict` off, + `enforce_admins` off. The 404 above was a reading at audit time.** The suite is unusually thoughtful in places — the daemon harness's connect-based readiness probe, the `PMACS_REQUIRE_GPU` hard-fail @@ -358,9 +361,12 @@ test) pass in CI and flake for whoever runs the documented local gate. (Findings that change what CI *certifies*; speedups are §6.) -1. **Branch protection is off** — every job is advisory; a red run - merges as easily as a green one. Turn on required checks for the - cheap deterministic jobs at minimum (fmt, clippy, ubuntu test legs). +1. ~~**Branch protection is off**~~ — **DONE.** Every job was + advisory; a red run merged as easily as a green one. All 12 contexts + are now required, rather than the cheap-jobs-only starter suggested + here. `strict` is off (a PR need not rebase every time `main` moves, + which this repository's ledger contention makes expensive) and + `enforce_admins` is off (the maintainer retains an override). 2. **No job timeouts except m6** (15 min). Everything else inherits 360 min. The day a runner image ships any of the PATH-gated tools (§1.2), the basedpyright-class hang burns 6 h × 4 matrix legs with diff --git a/docs/active-work.md b/docs/active-work.md index 8245bf6..5527ba7 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -890,6 +890,18 @@ has **no branch and no framing yet**. justifies exempting `main` pushes by appeal to "the branch-protection record" — that record now exists, so the justification is real rather than aspirational. +- **Required status checks are NAME-COUPLED to job names, and this + lane's own deferrals will break them.** A required context that no + longer exists does not fail — it leaves every PR pinned on + "Expected — waiting for status", indefinitely, which is + `main` becoming unmergeable by policy rather than by a red run. + Three deferrals above change job names or the matrix: the macOS trim + (§6.4) removes two contexts outright, and nextest (§6.3) or the + serial/parallel split (§6.2) rename or add them. + **Rule: any job rename, removal, or matrix change updates the + branch-protection required-checks list in the same motion.** Recorded + here because this is the entry that both enabled protection and named + the lanes that will invalidate it. - Recovery from a clean checkout: `git fetch githubsucks && git worktree add ../pmacs-ci3 -b ci-timeouts-concurrency githubsucks/ci-timeouts-concurrency`.