frankenbit/release-toolkit

Fork 0

decide: should base-divergence-check be a required context #1312

New issue

Closed

opened 2026-09-06 12:43:40 +02:00 by bosun · 3 comments

bosun commented

2026-09-06 12:43:40 +02:00

Owner

Should base-divergence-check become a required status context? It is a change to how the crew merges, not a gate flip, and the measurement says both answers cost something real.

Split from #1275, whose AC3 costed it. Measured by @surveyor, 2026-09-06.

The measurement

#1276 rc=1   #1290 rc=1   #1294 rc=1   #1300 rc=1   #1302 rc=1
#1303 rc=0   (the release cut — base == merge_base)

5 of 6 open PRs, 11–15 commits behind main

🔑 And the cause is CADENCE, not quality: main took 12 commits in about two hours that day, so a PR diverges within MINUTES of opening.

⚠️ The gate is not measuring whether a PR is good. It is measuring how long it has been open.

The options, and what each costs

REQUIRED    every PR must rebase immediately before every merge
   costs    a rebase per merge; and dismiss_stale_approvals fires on any
            CONTENT-changing rebase, so some merges cost a re-stamp too
   buys     the base-relative-green class cannot land at all
   note     #1209 measured that a CONTENT-PRESERVING rebase does NOT dismiss,
            which is what a freshness gate mostly forces — the ratchet is real
            but narrower than the original argument claimed

ADVISORY    the refusal is visible; the merger acts on it
   costs    depends on an actor reading it — which is the failure #1275 existed
            for: the verb was never invoked at all until 2026-09-06
   buys     no workflow change

The counterfactual is real, and it is the strongest argument for REQUIRED

Three composition failures in one day, all of the class this gate's precondition prevents:

#1270                  #1255 and #1263 each exported CanonicalFiles — main did not compile
#1251 under #1250      an additive-LOOKING signature change in a file the PR never touched
#1272 off broken main  a rebase onto a tree that did not build

📌 #1278 detects that class AFTER the merge. This gate prevents its precondition. Neither replaces the other.

Why it is not decidable from here

⚠️ Requiring it makes rebase-before-merge a crew rule. That is a working-practice change for every chamber, and the trade — a rebase per merge against a class of defect that hit three times in one day — is a judgement about how the crew should work rather than about what is true.

AC

A decision is recorded: ADVISORY, by the operator, 2026-09-06. His reason: "While it may break release-toolkit, it should not break the adopters. At least the ones pinned to a release." Costed in comment 108940; my measured recommendation and its caveat in 109752.
~~If required, status_check_contexts gains the context and the rebase-before-merge expectation is stated~~ — RETIRED: not required. The branch stays at block_on_outdated_branch=false and the context stays out of the 23.
The reason is written where the next person proposing promotion will read it — cmd/rt/base_divergence_check.go's own doc comment. It carries the operator's decision and reason, the three measurements behind it (redundant with the required set via #1195's run-time replay; required-and-stale against 131 commits/day; 59 of 78 merges behind, max 24, against dismiss_stale_approvals=true), and the counterweight that divergence HAS bitten but that this verb measures distance rather than compatibility and would not have caught #1141/#1143.

#1275 (the tracker that costed it), #1302 (the workflow), #1307 (the PASS line's tense), #1195 / #1278 (the same class from the other side), #1149

Anchor

@surveyor, 2026-09-06 — she ran the verb against every live PR rather than reasoning about the frequency, and identified cadence as the driver.

Should `base-divergence-check` become a required status context? It is a change to how the crew merges, not a gate flip, and the measurement says both answers cost something real. Split from `#1275`, whose AC3 costed it. Measured by @surveyor, 2026-09-06. ## The measurement ``` #1276 rc=1 #1290 rc=1 #1294 rc=1 #1300 rc=1 #1302 rc=1 #1303 rc=0 (the release cut — base == merge_base) 5 of 6 open PRs, 11–15 commits behind main ``` 🔑 **And the cause is CADENCE, not quality: `main` took 12 commits in about two hours that day, so a PR diverges within MINUTES of opening.** > ⚠️ ***The gate is not measuring whether a PR is good. It is measuring how long it has been open.*** ## The options, and what each costs ``` REQUIRED every PR must rebase immediately before every merge costs a rebase per merge; and dismiss_stale_approvals fires on any CONTENT-changing rebase, so some merges cost a re-stamp too buys the base-relative-green class cannot land at all note #1209 measured that a CONTENT-PRESERVING rebase does NOT dismiss, which is what a freshness gate mostly forces — the ratchet is real but narrower than the original argument claimed ADVISORY the refusal is visible; the merger acts on it costs depends on an actor reading it — which is the failure #1275 existed for: the verb was never invoked at all until 2026-09-06 buys no workflow change ``` ## The counterfactual is real, and it is the strongest argument for REQUIRED **Three composition failures in one day, all of the class this gate's precondition prevents:** ``` #1270 #1255 and #1263 each exported CanonicalFiles — main did not compile #1251 under #1250 an additive-LOOKING signature change in a file the PR never touched #1272 off broken main a rebase onto a tree that did not build ``` 📌 **`#1278` detects that class AFTER the merge. This gate prevents its precondition. Neither replaces the other.** ## Why it is not decidable from here ⚠️ **Requiring it makes rebase-before-merge a crew rule.** **That is a working-practice change for every chamber, and the trade — a rebase per merge against a class of defect that hit three times in one day — is a judgement about how the crew should work rather than about what is true.** ## AC - [x] A decision is recorded: **ADVISORY**, by the operator, 2026-09-06. His reason: *"While it may break release-toolkit, it should not break the adopters. At least the ones pinned to a release."* Costed in comment 108940; my measured recommendation and its caveat in 109752. - [x] ~~If required, `status_check_contexts` gains the context and the rebase-before-merge expectation is stated~~ — **RETIRED: not required.** The branch stays at `block_on_outdated_branch=false` and the context stays out of the 23. - [x] The reason is written where the next person proposing promotion will read it — **`cmd/rt/base_divergence_check.go`'s own doc comment.** It carries the operator's decision and reason, the three measurements behind it (redundant with the required set via #1195's run-time replay; required-and-stale against 131 commits/day; 59 of 78 merges behind, max 24, against `dismiss_stale_approvals=true`), and the counterweight that divergence HAS bitten but that this verb measures distance rather than compatibility and would not have caught #1141/#1143. ## Related `#1275` (the tracker that costed it), `#1302` (the workflow), `#1307` (the PASS line's tense), `#1195` / `#1278` (the same class from the other side), `#1149` ## Anchor @surveyor, 2026-09-06 — she ran the verb against every live PR rather than reasoning about the frequency, and identified cadence as the driver.

bosun added the

priority/high

size/S

kind/feature

labels

2026-09-06 12:43:40 +02:00

bosun commented

2026-09-06 13:48:34 +02:00

Author

Owner

OPERATOR DECISION — the options, what each costs, and why none dominates. Measured 2026-09-06 by @bosun; this tracker had no such comment before now.

State today

base-divergence-check required?   NO — 0 of 23 status_check_contexts
posts advisorily on               pull_request, since #1302

The corpus is SMALL and I am stating it rather than implying more

Only five heads have ever posted this context — it is days old:

#1318 success · #1316 success · #1314 success · #1319 success · #1302 FAILURE

⚠️ I have previously cited "5 of 6 open PRs were rc=1". That was measured mid-campaign on OPEN PRs, all of which were behind a moving main; the merged corpus reads 4/5 green because each was rebased before landing. 🔑 The two numbers measure different populations and the honest statement is that the failure rate is a function of WHEN you look, not a property of the check. That is itself an argument the decision turns on.

📌 #1302 merged at 12:39 with this check RED — and #1302 is the PR that ADDED the check. It was allowed to because the context is advisory. The check's own PR would have been blocked by the check.

The cost that decides it

merges onto main today        43
open PRs at any moment        2-6
re-run cost per rebase        ~184s wall-clock, 347s summed, 23 contexts
release-prep/rolling as head  3 of the last 50 PRs

🔴 At 43 base moves in one day, every open PR goes stale several times an hour. Required means each one rebases and re-runs the full suite — and the rebases serialise against the thing causing them: while one PR rebases, another merge invalidates it. That is the failure mode where a gate is switched off rather than fixed.

⚠️ And release-prep/rolling becomes a PR head. Nobody can push a rebase to a bot-regenerated branch (crew-doctrine#135); the bot force-pushes on base moves, so it probably self-heals. I have not verified that it does, and it is the arm that would hurt most.

The three options

A — REQUIRE IT. No PR ever lands on a stale base again. Costs: continuous forced rebasing at current cadence, a re-run storm, and an unverified interaction with the rolling branch. Cheap at normal cadence; expensive now.

B — LEAVE IT ADVISORY (status quo). No forced rebases. Costs: the signal can be ignored, and #1302 is the instance proving it will be. Merging on a stale base is what produced the v0.61.1 fragment miss (#1305).

C — REQUIRE IT WITH A CARVE-OUT (bot branches exempt, or required only outside a declared crunch). Costs: a conditional gate is a gate with a hidden scope, and nothing forces the carve-out to be removed later. #1177 is the local precedent for a required context whose scope nobody re-read.

🔑 Why none dominates

The cost of A is entirely cadence-dependent and the benefit is not. At 43 merges/day it is expensive and safe; at 3 merges/day it is nearly free and equally safe. So the right answer depends on how long this crunch phase lasts — which is an operator fact, not one I can measure from the board.

⚠️ I am not asking to be told what to pick because I would rather not choose. If the crunch ends soon, A dominates and I will take it without asking. The decision is genuinely yours only while the cadence is this high.

📌 #1305's cut-fragment-coverage-check (@surveyor, #1319) is deliberately NOT a required context for the same reason, on her own reasoning — a status posted at PR time answers about a base that moves afterwards, which is the failure it exists to catch. Whatever is decided here should be consistent with that, or one of the two is wrong.

**OPERATOR DECISION — the options, what each costs, and why none dominates. Measured 2026-09-06 by @bosun; this tracker had no such comment before now.** ## State today ``` base-divergence-check required? NO — 0 of 23 status_check_contexts posts advisorily on pull_request, since #1302 ``` ## The corpus is SMALL and I am stating it rather than implying more **Only five heads have ever posted this context** — it is days old: ``` #1318 success · #1316 success · #1314 success · #1319 success · #1302 FAILURE ``` ⚠️ **I have previously cited *"5 of 6 open PRs were rc=1"*. That was measured mid-campaign on OPEN PRs, all of which were behind a moving main; the merged corpus reads 4/5 green because each was rebased before landing.** 🔑 **The two numbers measure different populations and the honest statement is that the failure rate is a function of WHEN you look, not a property of the check.** That is itself an argument the decision turns on. 📌 **`#1302` merged at 12:39 with this check RED — and `#1302` is the PR that ADDED the check.** It was allowed to because the context is advisory. *The check's own PR would have been blocked by the check.* ## The cost that decides it ``` merges onto main today 43 open PRs at any moment 2-6 re-run cost per rebase ~184s wall-clock, 347s summed, 23 contexts release-prep/rolling as head 3 of the last 50 PRs ``` 🔴 **At 43 base moves in one day, every open PR goes stale several times an hour.** Required means each one rebases and re-runs the full suite — **and the rebases serialise against the thing causing them: while one PR rebases, another merge invalidates it.** That is the failure mode where a gate is switched off rather than fixed. ⚠️ **And `release-prep/rolling` becomes a PR head.** Nobody can push a rebase to a bot-regenerated branch (`crew-doctrine#135`); the bot force-pushes on base moves, so it *probably* self-heals. **I have not verified that it does, and it is the arm that would hurt most.** ## The three options **A — REQUIRE IT.** No PR ever lands on a stale base again. **Costs: continuous forced rebasing at current cadence, a re-run storm, and an unverified interaction with the rolling branch.** Cheap at normal cadence; expensive now. **B — LEAVE IT ADVISORY (status quo).** No forced rebases. **Costs: the signal can be ignored, and `#1302` is the instance proving it will be.** Merging on a stale base is what produced the v0.61.1 fragment miss (`#1305`). **C — REQUIRE IT WITH A CARVE-OUT** (bot branches exempt, or required only outside a declared crunch). **Costs: a conditional gate is a gate with a hidden scope**, and nothing forces the carve-out to be removed later. `#1177` is the local precedent for a required context whose scope nobody re-read. ## 🔑 Why none dominates **The cost of A is entirely cadence-dependent and the benefit is not.** At 43 merges/day it is expensive and safe; at 3 merges/day it is nearly free and equally safe. **So the right answer depends on how long this crunch phase lasts — which is an operator fact, not one I can measure from the board.** ⚠️ **I am not asking to be told what to pick because I would rather not choose.** If the crunch ends soon, **A dominates and I will take it without asking.** The decision is genuinely yours only while the cadence is this high. 📌 **`#1305`'s `cut-fragment-coverage-check` (@surveyor, `#1319`) is deliberately NOT a required context for the same reason, on her own reasoning** — a status posted at PR time answers about a base that moves afterwards, which is the failure it exists to catch. **Whatever is decided here should be consistent with that, or one of the two is wrong.**

quartermaster referenced this issue from a commit

2026-09-06 15:01:43 +02:00

fix(pre-push): grade the declaration list too, so a dead entry cannot hide a gap

bosun commented

2026-09-06 15:16:42 +02:00

Author

Owner

📌 A DATA POINT THAT QUALIFIES THE COST ESTIMATE IN THE OPTIONS COMMENT ABOVE. @quartermaster, 2026-09-06, on #1334.

base-divergence-check / check went RED and then GREEN with NO rebase and no push — the head never moved from 0a735e2c.

"I was going to leave it red as honest noise; it resolved itself, which means the divergence it reported had already cleared."

⚠️ The options comment costs Option A (REQUIRE IT) as continuous forced rebasing at 43 merges/day. 🔑 If a divergence red can expire without anyone touching the branch, that cost is over-stated — some fraction of reds clear on their own when the next scheduled run finds the base is an ancestor again.

🔴 BUT IT CUTS BOTH WAYS, AND THAT IS THE PART TO KEEP: a red that expires by itself is not a reliable standing signal that a rebase is owed. As a REQUIRED context that is fine — the merge simply waits. As an ADVISORY one it is worse than it looks: a merger who sees red, does nothing, and finds it green later learns that the signal can be ignored.

📌 The mechanism is NOT established and I am not asserting one. It could be the base becoming an ancestor after an unrelated merge, or a re-run against a moved tip. Establishing it needs the status history for that context on that head, which nobody has read.

✅ What this does to the decision: it makes Option A cheaper than costed and Option B weaker than costed — both in the same direction. ⚠️ It does not settle the decision, because the deciding fact is still how long crunch cadence lasts, and that remains the operator's.

(@quartermaster explicitly did NOT rebase pre-emptively, which is why this observation exists: a rebase would have destroyed the evidence that it self-cleared.)

📌 **A DATA POINT THAT QUALIFIES THE COST ESTIMATE IN THE OPTIONS COMMENT ABOVE. @quartermaster, 2026-09-06, on `#1334`.** **`base-divergence-check / check` went RED and then GREEN with NO rebase and no push — the head never moved from `0a735e2c`.** > ***"I was going to leave it red as honest noise; it resolved itself, which means the divergence it reported had already cleared."*** ⚠️ **The options comment costs Option A (REQUIRE IT) as *continuous forced rebasing at 43 merges/day*.** 🔑 **If a divergence red can expire without anyone touching the branch, that cost is over-stated** — some fraction of reds clear on their own when the next scheduled run finds the base is an ancestor again. 🔴 **BUT IT CUTS BOTH WAYS, AND THAT IS THE PART TO KEEP: a red that expires by itself is not a reliable standing signal that a rebase is owed.** **As a REQUIRED context that is fine — the merge simply waits.** **As an ADVISORY one it is worse than it looks: a merger who sees red, does nothing, and finds it green later learns that the signal can be ignored.** 📌 **The mechanism is NOT established and I am not asserting one.** *It could be the base becoming an ancestor after an unrelated merge, or a re-run against a moved tip. Establishing it needs the status history for that context on that head, which nobody has read.* ✅ **What this does to the decision: it makes Option A cheaper than costed and Option B weaker than costed — both in the same direction.** ⚠️ **It does not settle the decision, because the deciding fact is still how long crunch cadence lasts, and that remains the operator's.** *(@quartermaster explicitly did NOT rebase pre-emptively, which is why this observation exists: a rebase would have destroyed the evidence that it self-cleared.)*

bosun referenced this issue from a commit

2026-09-06 16:01:37 +02:00

fix(pre-push): grade the declaration list too, so a dead entry cannot hide a gap

bosun commented

2026-09-06 17:12:43 +02:00

Author

Owner

Operator asked for my opinion, and I agree with his lean: leave it advisory. My reason is stronger than the cost argument, and it changed today.

It is now redundant with the required set

#1195 landed today. Measured on main:

workflows carrying the landing-tree replay step   19 of 35
the required contexts                             23, all in those workflows
how the replay resolves its base                  BASE_REF: ${{ github.base_ref }}
                                                  git fetch --no-tags origin "$BASE_REF"
                                                  -> resolved AT RUN TIME

So every required context already grades head replayed onto the base as of the moment it ran. base-divergence-check reports the branch's STORED merge-base — a different object and a weaker claim.

Making it required would add a gate that answers a worse question than the 23 already answer. That is the decisive point, and it was not true a week ago.

And a required base-divergence-check would be required-and-stale

Its own PASS line says so: "this status does NOT re-run when the base moves; compare that SHA against the base tip before merging."

commits on main today   131

A required status graded at push time, on a branch that the base outruns 131 times a day, is expired for most of its life. Requiring it enforces a claim that has an expiry — which is worse than not requiring it, because a red that means "stale by one commit" and a red that means "genuinely diverged" are the same red.

The cost is measured, not hypothetical

Every PR merged today, with how far behind it was at merge:

merges today     78
behind = 0       19
behind >= 1      59
behind >= 10     23
max behind       24

With block_on_outdated_branch=true and dismiss_stale_approvals=true, 59 of those 78 need a rebase, each rebase destroys the only approval, and main moves again while the re-review is pending. That is #1183's deadlock at the scale this repo actually runs at: the cut cadence is shorter than a review round.

The honest counterweight

Divergence HAS bitten. #1141/#1143 and the 25-minute main breakage recorded on #1195 are real, so "it does not happen" is false and should not be the argument.

But those were semantic collisions between changes to different files. base-divergence-check measures DISTANCE, not compatibility, and would not have caught either. The gate is not the remedy for the failure it is associated with.

The one real composition collision today — #1346 against #1195, same file, same location — was caught by @pilot rebasing voluntarily and resolving by hand, then cross-checking the .gitea twin with rt gitea-twin --write. Not by a gate.

What I do instead, and would rather mechanise

I replay each PR onto live main at the moment of merge, with a control that deliberately conflicts:

control (deliberate collision)   rc=1     <- the harness CAN fail
PR replay onto live main         rc=0

That does not race, because it runs at the moment of the merge rather than at push time. Every merge today was verified this way and none failed.

One caveat against my own position

@shipwright and @surveyor both argued #1351 needed no rebase because "the gates already graded the landing tree" — and that argument expired within three minutes, because main moved twice more while the decision was being made. It merged cleanly anyway.

Both reasons were correct when measured and stale when used. So "the required gates grade the landing tree" is true at run time and decays exactly like everything else here. It is still strictly better than a stored merge-base, which is what base-divergence-check reports — but it is not a guarantee, and I would rather state that than let this comment read as one.

Recommendation

Leave base-divergence-check advisory. Keep it: its red is a useful prompt to a merger. Do not promote it to required — it would cost 59 rebases a day to enforce a weaker claim than the required set already checks, on a status that cannot re-run when the thing it measures changes.

Operator asked for my opinion, and I agree with his lean: leave it advisory. My reason is stronger than the cost argument, and it changed today. ## It is now redundant with the required set `#1195` landed today. Measured on `main`: ``` workflows carrying the landing-tree replay step 19 of 35 the required contexts 23, all in those workflows how the replay resolves its base BASE_REF: ${{ github.base_ref }} git fetch --no-tags origin "$BASE_REF" -> resolved AT RUN TIME ``` So every required context already grades `head` replayed onto the base **as of the moment it ran**. `base-divergence-check` reports the branch's STORED merge-base — a different object and a weaker claim. Making it required would add a gate that answers a worse question than the 23 already answer. That is the decisive point, and it was not true a week ago. ## And a required base-divergence-check would be required-and-stale Its own PASS line says so: *"this status does NOT re-run when the base moves; compare that SHA against the base tip before merging."* ``` commits on main today 131 ``` A required status graded at push time, on a branch that the base outruns 131 times a day, is expired for most of its life. Requiring it enforces a claim that has an expiry — which is worse than not requiring it, because a red that means "stale by one commit" and a red that means "genuinely diverged" are the same red. ## The cost is measured, not hypothetical Every PR merged today, with how far behind it was at merge: ``` merges today 78 behind = 0 19 behind >= 1 59 behind >= 10 23 max behind 24 ``` With `block_on_outdated_branch=true` and `dismiss_stale_approvals=true`, 59 of those 78 need a rebase, each rebase destroys the only approval, and main moves again while the re-review is pending. That is `#1183`'s deadlock at the scale this repo actually runs at: the cut cadence is shorter than a review round. ## The honest counterweight Divergence HAS bitten. `#1141`/`#1143` and the 25-minute main breakage recorded on `#1195` are real, so "it does not happen" is false and should not be the argument. But those were semantic collisions between changes to different files. `base-divergence-check` measures DISTANCE, not compatibility, and would not have caught either. The gate is not the remedy for the failure it is associated with. The one real composition collision today — `#1346` against `#1195`, same file, same location — was caught by @pilot rebasing voluntarily and resolving by hand, then cross-checking the `.gitea` twin with `rt gitea-twin --write`. Not by a gate. ## What I do instead, and would rather mechanise I replay each PR onto live `main` at the moment of merge, with a control that deliberately conflicts: ``` control (deliberate collision) rc=1 <- the harness CAN fail PR replay onto live main rc=0 ``` That does not race, because it runs at the moment of the merge rather than at push time. Every merge today was verified this way and none failed. ## One caveat against my own position @shipwright and @surveyor both argued `#1351` needed no rebase because "the gates already graded the landing tree" — and that argument expired within three minutes, because main moved twice more while the decision was being made. It merged cleanly anyway. Both reasons were correct when measured and stale when used. So "the required gates grade the landing tree" is true at run time and decays exactly like everything else here. It is still strictly better than a stored merge-base, which is what `base-divergence-check` reports — but it is not a guarantee, and I would rather state that than let this comment read as one. ## Recommendation Leave `base-divergence-check` advisory. Keep it: its red is a useful prompt to a merger. Do not promote it to required — it would cost 59 rebases a day to enforce a weaker claim than the required set already checks, on a status that cannot re-run when the thing it measures changes.

bosun referenced this issue from a commit

2026-09-06 17:23:40 +02:00

docs(gates): record why base-divergence-check stays advisory

bosun referenced this issue from a pull request that will close it,

2026-09-06 17:23:57 +02:00

docs(gates): record why base-divergence-check stays advisory #1358

bosun closed this issue

2026-09-06 17:43:56 +02:00

bosun referenced this issue from a commit

2026-09-06 17:43:57 +02:00

docs(gates): record why base-divergence-check stays advisory