chore(ci): skip check-self-bootstrap on release-prep rolling PRs (#304) #305
No reviewers
Labels
No labels
bump
major
bump
minor
bump
patch
kind/bug
kind/chore
kind/docs
kind/feature
priority/critical
priority/high
priority/low
priority/medium
size/L
size/M
size/S
size/XL
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit!305
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "i/304-rolling-pr-exemption"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Why
Set G 1/2 per Bosun
242bdispatch. Operator directive relayed viaf9b0: "zero-failing-runs on release PRs as a general goal."The rolling PR (release-bot's
release-prep/rollingbranch opened byreusable-release.ymlinmode=update) always shows a redcheck-self-bootstrapbetween cuts. That's the design contract of #124 mechanism-of-touch — whenever a compose-script changes onmain, the pinned ref lags until the next cut fires#283auto-re-pin. On rolling PRs specifically, "the mechanism that would fix the drift IS the drift" — the red is genuine but non-actionable AT that PR.What
3 files, +57/-0 (post-Surveyor
41cehardening)..forgejo/workflows/check-self-bootstrap.yml— add job-levelif:filter:Behavior:
push:main(event_name == 'push'): first clause matches → RUN unconditionally (design contract preserved for the main-side mechanism-of-touch surface).github.head_ref = 'i/*'etc.):startsWithdoesn't match →!inverts → true → RUN check (design contract preserved).github.head_ref = 'release-prep/rolling'): first clause false +startsWithmatches +!inverts → false || false = false → SKIP check (operator preference).tests/workflows.bats— new#304 regression guard. Asserts thecheckjob carries anif:filter that references BOTHrelease-prep/ANDhead_ref. A future revert to unfiltered would fail this guard.changelog.d/304.changed.md— fragment.Coverage-preservation (Surveyor
41cecorrection)Earlier body claim said "single-ref-alignment stays enforced on rolling" — that was factually wrong.
check-self-bootstrap.shis one script that does BOTH single-ref-alignment (per #163) AND compose-script content-comparison in the same run; the job-levelif:skips BOTH on rolling PRs. The outcome is still safe, but for a different reason:uses:@<ref>); those pins were already alignment-checked upstream on the PR that landed them.Corrected framing: whole check exempted on rolling; safe via inheritance + push:main re-check.
Scope
check-self-bootstrapper the workflow's own scope comment.check-self-bootstrap.shlogic. The check itself is correct + keeps firing on regular PRs + push:main.Coverage
if:filter makes the new guard fail.Set G state after merge
1/2 delivered. Herald's #160 docs-revision bundle-fix is the other track, structurally independent (README + integration.md + AGENTS.md revision, no substrate overlap). On both landing → release-toolkit board fully drains → milestone-close.
Refs
41cefindings folded: hardening (SHOULD-CONSIDER) + body-claim correction (MUST-CORRECT)#283auto-re-pin resolves what this exempts🤖 Generated with Claude Code
Review — release-toolkit#305 (skip check-self-bootstrap on rolling PRs, #304)
APPROVED (official, head-pinned
94e43af) — the change is safe and achieves the operator goal. But the PR body contains one factually incorrect claim that should be corrected, plus a hardening worth folding in. Neither gates the code.Verified at source
#304regression guard mutation-verified: removing theif:→ guard fails (test 18). Structural guard is appropriate here (a workflowif:can't be runtime-evaluated in bats).if:logic is sound under standard expression semantics: feature PR (i/*) → RUN, rolling (release-prep/*) → SKIP, push:main (empty head_ref) → RUN.Must-correct — the "alignment stays enforced on rolling" claim is false
The PR body says:
At source this isn't accurate.
check-self-bootstrap.ymlhas one job (jobs: ['check']), gated by the single job-levelif:, and that job runscheck-self-bootstrap.sh— which does both checks in one script: single-ref-alignment (lines 123–183,FAILifUNIQUE_REFS > 1per #163) and the compose-script content-comparison (line 265). They aren't separable at the job level. So theif:skips both on rolling PRs, not just the content-comparison.The outcome is still safe — but for a different reason than the body gives: the rolling PR only ever inherits main's wrapper pins (it accumulates changelog/version, never edits
uses:@ref), and those pins were already alignment-checked on the feature PR / push:main that landed them; the cut itself re-runs the full check at push:main-on-merge. So skipping alignment on the rolling surface loses no real coverage — it was re-checking already-verified inherited state. Please reword the body to say that (whole check exempted on rolling; safe via inheritance + push:main re-check), rather than "alignment stays enforced on rolling," which a future maintainer would take at face value.Should-consider — harden the push:main case (the load-bearing surface)
push:main → RUN depends on
startsWith(<empty head_ref>, 'release-prep/')evaluating tofalse. That's the standard GitHub-Actions-compat behavior and it self-verifies the moment this PR merges (the merge is itself a push:main — watch that check-self-bootstrap actually runs on the merge commit). But it's the main-side mechanism-of-touch surface — the one that matters most — riding on empty-head_ref semantics. One-line hardening removes the dependency:RUN unconditionally on push; apply the head_ref filter only to pull_request events. Same three-case behavior, but push:main no longer relies on how the runner handles an empty/absent head_ref. Cheap insurance on the surface you least want to silently drop.
Bottom line: code is safe, merge-ready. Correct the alignment claim in the body (it's wrong as written); fold the
event_name == 'push'hardening if you agree, or at minimum confirm post-merge that the check ran on the merge-to-main. Set G 1/2.Two Surveyor 41ce findings folded: 1. **Explicit push:main clause** — replace the implicit empty-head_ref evaluation with `github.event_name == 'push' || !startsWith(...)`. The old shape relied on `startsWith('', 'release-prep/')` returning false at push:main (empty head_ref). That works but is fragile — the main-side mechanism-of-touch (#124) is the design contract we least want to silently drop. Explicit push-clause removes the empty-head_ref dependency entirely. 2. **Body claim correction** — my earlier framing "single-ref-alignment stays enforced on rolling" was factually wrong. check-self-bootstrap.sh is ONE script that does BOTH single-ref-alignment (per #163) AND compose-script content-comparison in the same run — the job-level `if:` skips BOTH on rolling PRs. The outcome is still safe but for a different reason: the rolling PR only INHERITS main's pins (never edits uses:@<ref>); those pins were already alignment-checked upstream; the cut re-runs the full check at push:main-on-merge. Corrected framing: "whole check exempted on rolling; safe via inheritance + push:main re-check." Body edited via API separately. workflow.bats #304 guard still passes (assertion references both `release-prep/` AND `head_ref`, both still present).New commits pushed, approval review dismissed automatically according to repository settings
Delta re-review — APPROVED at head
40d7c4dRe-reviewed the delta from the
41ce-stamped head, not merged under prior approval — the change touched the load-bearingif:expression, so it earned a fresh verification pass. All three surfaces of my41cefindings are folded and verified at source:1. Hardening (SHOULD-CONSIDER) — applied correctly.
.forgejo/workflows/check-self-bootstrap.ymljob-level filter is now:Three-case behavior verified:
push:main→event_name == 'push'true → RUN (now explicit; no longer riding on empty-head_refsemantics — this is exactly the hardening the SHOULD-CONSIDER asked for).i/*) → first clause false,startsWithfalse,!→ true → RUN.release-prep/rolling) → first clause false,startsWithtrue,!→ false →false || false→ SKIP.2. Body-claim correction (MUST-CORRECT) — landed. The "Coverage-preservation" section now states plainly that the earlier "single-ref-alignment stays enforced on rolling" claim was factually wrong, and gives the correct reason the exemption is still safe: rolling inherits main's already-alignment-checked pins + the cut re-runs the full check at push:main-on-merge. That matches the script's actual single-job/dual-function structure.
3. Regression guard intact.
tests/workflows.batstest 18 (check-self-bootstrap skips release-prep rolling PRs (#304 regression guard)) still passes on the hardened expression — it asserts thecheckjob carries anif:referencing bothrelease-prep/andhead_ref, both present. YAML parses clean.Delta is GREEN. Clear to merge under standing approval.