Skip to content

Commit 4f236a8

Browse files
committed
chore: stop claiming branch protection gates every CI job
The removal's justification said branch protection refuses the merge whatever the report claimed. That is true only for the required contexts, and main requires five of the roughly eleven jobs ci.yml defines. Every job established as a member of the no-local-counterpart class sits in the non-required set, so the sentence was false for exactly the jobs whose cost it was justifying. Nothing was actually weakened by this PR: the removed end-of-cycle read was an instruction, not a mechanism, and branch protection covered the same subset before and after. But the merge gate rests on the instruction to read every check, so the paragraph says that, and says outright that a green required set is not green CI.
1 parent ce437e0 commit 4f236a8

2 files changed

Lines changed: 8 additions & 1 deletion

File tree

.claude/skills/webjs-start-work/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ If you cannot honestly say the final review left nothing must-fix open, you cann
399399
400400
**Merge is gated on green CI, enforced at the branch level, not by trust.** A PR must not merge until all CI checks pass. `main` branch protection requires the five `ci.yml` checks (Conventions, Unit+integration, Browser, E2E, Build) before any merge; if `gh api repos/webjsdev/webjs/branches/main/protection` shows `required_status_checks: null`, run `bash scripts/protect-main.sh` once (needs repo admin) to restore it. Do not work around a red or pending check. Wait for green, and fix whatever is red before merging.
401401
402-
**This is the ONLY place CI is read, on purpose. Do not add one back to the end of the review cycle.** An end-of-cycle read was removed because it was redundant against this gate: branch protection refuses the merge whatever the report claimed, so a red check gets caught and fixed right here, and reading it earlier only parks the finished cycle on a multi-minute CI run. What that costs is worth stating plainly, because it looks like a gap. Every `ci.yml` job with no counterpart in the deferred local suites now fails for the first time at merge rather than before the ready signal. That is a CLASS, not a list. Its membership moves as jobs are added and as the local suites grow to cover them, so derive it when you need it, by reading `.github/workflows/ci.yml` against the deferred set named above. Do NOT write the membership down here.
402+
**This is the ONLY place CI is read, on purpose. Do not add one back to the end of the review cycle.** An end-of-cycle read was removed because it was redundant against this gate, which reads EVERY check and fixes what is red before merging, so a failure gets caught here whatever the ready-to-merge report claimed, and reading it earlier only parks the finished cycle on a multi-minute CI run. Be precise about what enforces that, because the two halves are not equally strong. Branch protection MECHANICALLY refuses the merge for the required contexts only, and `ci.yml` defines roughly twice as many jobs as `main` requires, so the rest are held by the INSTRUCTION above to read every check rather than by anything that can refuse a merge. Removing the end-of-cycle read did not weaken that, since the removed read was an instruction too and branch protection covered the same subset before and after. It does mean a green REQUIRED set is not the same as green CI, so read `gh pr checks` in full rather than trusting the merge button to have judged for you. What that costs is worth stating plainly, because it looks like a gap. Every `ci.yml` job with no counterpart in the deferred local suites now fails for the first time at merge rather than before the ready signal. That is a CLASS, not a list. Its membership moves as jobs are added and as the local suites grow to cover them, so derive it when you need it, by reading `.github/workflows/ci.yml` against the deferred set named above. Do NOT write the membership down here.
403403
404404
That instruction is load-bearing and was expensive to learn. Three attempts to write the membership into this paragraph were each wrong, some by naming a job the local suites already cover, some by reading complete while omitting one they do not, and every one of them was caught by review rather than by the author. The reason the error keeps recurring is structural: deciding membership means holding `ci.yml` and the deferred set side by side and checking a non-obvious local counterpart for each job, which is a derivation, and a derivation frozen into prose is wrong the moment either side moves. So derive it, and do not leave the answer here for the next reader to trust.
405405

test/hooks/review-loop-exit.test.mjs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,13 @@ test('CI is read only at the merge gate, never at the end of the cycle', () => {
188188
assert.ok(!re.test(skill), `${label} tells the cycle to read CI again`);
189189
}
190190

191+
// The gate leans on reading every check, NOT on branch protection alone,
192+
// which mechanically covers only the required subset. Overstating that was
193+
// the removal's original justification and it was false for exactly the
194+
// jobs it was justifying the cost of.
195+
assert.match(skill, /Branch protection MECHANICALLY refuses the merge for the required contexts only/);
196+
assert.match(skill, /a green REQUIRED set is not the same as green CI/);
197+
191198
// The gate the removal leans on has to stay strict, since it is now the
192199
// only CI checkpoint there is.
193200
assert.match(skill, /\*\*Merge is gated on green CI, enforced at the branch level, not by trust\.\*\*/);

0 commit comments

Comments
 (0)