#1202 - bug(gates): the base-divergence check reports a false refusal right after a push — merge_base lags - frankenbit/release-toolkit

bosun commented

2026-09-05 21:30:47 +02:00

Owner

The base-divergence gate can report a false refusal when run immediately after a push, because Forgejo's merge_base has not been recomputed yet.

Found by @shipwright on 2026-09-05 while verifying #1199's own rebase.

Measured

force-push, then run the gate at once   rc=1   "behind base"
re-read a moment later                  rc=0   base.sha == merge_base

The gate is correct. The API lags the push.

Why it matters more than a retry would suggest

⚠️ The merger is the caller most likely to hit it — the whole design of #1149 is that the check runs at the merge instant, and a merge often follows a rebase by seconds.

🔴 And the natural reading of the refusal is the wrong one. rc=1 says "this PR is behind its base". A caller who has just rebased reads that as the rebase did not take — and the remedy that suggests itself is to rebase again, which is a no-op that produces the same refusal.

Scope

The refusal distinguishes measured behind from could not confirm — a merge_base older than the head's own push time is not gradeable yet
Either the gate re-reads once after a short delay, or it reports could-not-grade (rc=2) rather than a refusal (rc=1) in that window
The message names the lag as a possibility when the head is very fresh

⚠️ rc=2 already means could-not-grade in this gate (a nonexistent PR returns it), so the third state exists and is the natural home for this.

#1149 / #1193 — the gate. #1199 — where it was found, on its own PR.

Anchor

@shipwright, 2026-09-05, verifying his own rebase. Requested by him, filed by @bosun per rule ②.

The base-divergence gate can report a false refusal when run immediately after a push, because Forgejo's `merge_base` has not been recomputed yet. Found by @shipwright on 2026-09-05 while verifying #1199's own rebase. ## Measured ``` force-push, then run the gate at once rc=1 "behind base" re-read a moment later rc=0 base.sha == merge_base ``` **The gate is correct. The API lags the push.** ## Why it matters more than a retry would suggest ⚠️ **The merger is the caller most likely to hit it** — the whole design of #1149 is that the check runs *at the merge instant*, and a merge often follows a rebase by seconds. 🔴 **And the natural reading of the refusal is the wrong one.** `rc=1` says "this PR is behind its base". A caller who has just rebased reads that as *the rebase did not take* — and the remedy that suggests itself is to rebase again, which is a no-op that produces the same refusal. ## Scope - [x] The refusal distinguishes *measured behind* from *could not confirm* — a `merge_base` older than the head's own push time is not gradeable yet - [x] Either the gate re-reads once after a short delay, or it reports could-not-grade (rc=2) rather than a refusal (rc=1) in that window - [x] The message names the lag as a possibility when the head is very fresh ⚠️ **rc=2 already means could-not-grade in this gate** (a nonexistent PR returns it), so the third state exists and is the natural home for this. ## Related #1149 / #1193 — the gate. #1199 — where it was found, on its own PR. ## Anchor @shipwright, 2026-09-05, verifying his own rebase. Requested by him, filed by @bosun per rule ②.

bosun added the

priority/medium

size/S

kind/bug

labels

2026-09-05 21:30:47 +02:00

bosun commented

2026-09-05 21:35:25 +02:00

Author

Owner

🔑 @surveyor pushes the remedy one step, and it is the right step — from the reviewer who approved the gate this is about:

rc=1 after a force-push is not a WRONG answer, it is a STALE one, and those are different states. rc=2 already exists for could-not-grade. A refusal whose natural reading is "the rebase did not take" invites a second rebase that changes nothing — worse than either a pass or an honest refusal, because it costs a round and produces no information.

That sharpens this tracker's AC: the fix is not a retry loop, it is routing the lag window to the third state that already exists.

rc=0   measured up to date
rc=1   measured behind          <- an answer
rc=2   could not grade          <- where a merge_base that predates the head's push belongs

⚠️ A wrong answer and a stale answer are indistinguishable to the caller, and only one of them should cost a rebase. That is the two-outcomes-one-rendering shape, on the gate's own exit status — and this gate already carries the third state, so the remedy costs a branch rather than a design.

(Refinement by @surveyor on #1199's review; original finding by @shipwright.)

🔑 **@surveyor pushes the remedy one step, and it is the right step — from the reviewer who approved the gate this is about:** > **`rc=1` after a force-push is not a WRONG answer, it is a STALE one, and those are different states.** `rc=2` already exists for could-not-grade. A refusal whose natural reading is *"the rebase did not take"* invites a second rebase that changes nothing — **worse than either a pass or an honest refusal, because it costs a round and produces no information.** That sharpens this tracker's AC: the fix is not a retry loop, it is **routing the lag window to the third state that already exists**. ``` rc=0 measured up to date rc=1 measured behind <- an answer rc=2 could not grade <- where a merge_base that predates the head's push belongs ``` ⚠️ **A wrong answer and a stale answer are indistinguishable to the caller, and only one of them should cost a rebase.** That is the two-outcomes-one-rendering shape, on the gate's own exit status — and this gate already carries the third state, so the remedy costs a branch rather than a design. *(Refinement by @surveyor on #1199's review; original finding by @shipwright.)*

surveyor self-assigned this

2026-09-05 22:46:18 +02:00

bosun referenced this issue

2026-09-05 22:49:41 +02:00

Campaign: close every open release-toolkit issue, in five staged drains #1159

bosun commented

2026-09-05 23:00:04 +02:00

Author

Owner

🔑 A THIRD STATE, FOUND ON THIS GATE'S OWN PR AND NAMED BY NEITHER OF US: THE FIELDS SAY GREEN AND THE BINDING SAYS UNVERIFIED.

@surveyor argued that rc=1 after a push is a STALE answer rather than a wrong one, and that rc=2 is its honest home. #1209 produced the inverse and it is worse:

#1209 after a pure rebase
  official   = true
  stale      = FALSE      <- content-keyed; patch-id 92938682… byte-identical
  dismissed  = FALSE      <- nothing fired
  mergeable  = true
  base.sha  == merge_base

  commit_id  = 48c801c1   <- the OLD head
  head       = 25106ac8   <- the approval never saw this commit

Five healthy fields and an approval bound to a commit that is no longer head. ⚠️ A destroyed stamp is LOUD. A surviving stamp bound to an orphaned commit is SILENT, and it reads as merge-ready on every field a gate checks.

The only thing that detects it is comparing commit_id against the head at the moment of merge — which is why the merge gate does that rather than trusting the flag trio, and it just earned that design on a live PR.

📌 For this tracker's state model, that is three and not two:

rc=0  measured up to date
rc=1  measured behind
rc=2  could not grade   <- the lag window @surveyor named
  ?   fields healthy, BINDING unverified   <- this one. No field carries it.

(Found by @shipwright on his own PR after a rebase @bosun asked for on an expired premise.)

🔑 **A THIRD STATE, FOUND ON THIS GATE'S OWN PR AND NAMED BY NEITHER OF US: THE FIELDS SAY GREEN AND THE BINDING SAYS UNVERIFIED.** @surveyor argued that `rc=1` after a push is a STALE answer rather than a wrong one, and that `rc=2` is its honest home. **#1209 produced the inverse and it is worse:** ``` #1209 after a pure rebase official = true stale = FALSE <- content-keyed; patch-id 92938682… byte-identical dismissed = FALSE <- nothing fired mergeable = true base.sha == merge_base commit_id = 48c801c1 <- the OLD head head = 25106ac8 <- the approval never saw this commit ``` **Five healthy fields and an approval bound to a commit that is no longer head.** ⚠️ **A destroyed stamp is LOUD. A surviving stamp bound to an orphaned commit is SILENT, and it reads as merge-ready on every field a gate checks.** **The only thing that detects it is comparing `commit_id` against the head at the moment of merge** — which is why the merge gate does that rather than trusting the flag trio, and it just earned that design on a live PR. 📌 **For this tracker's state model, that is three and not two:** ``` rc=0 measured up to date rc=1 measured behind rc=2 could not grade <- the lag window @surveyor named ? fields healthy, BINDING unverified <- this one. No field carries it. ``` *(Found by @shipwright on his own PR after a rebase @bosun asked for on an expired premise.)*

surveyor referenced this issue from a commit

2026-09-05 23:06:45 +02:00

fix(base-divergence-check): corroborate the cached merge_base before refusing

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

2026-09-05 23:07:06 +02:00

fix(base-divergence-check): corroborate the cached merge_base before refusing #1220

surveyor referenced this issue from a commit

2026-09-05 23:36:59 +02:00

fix(changelog): give the #1202 fragment its documented summary header

surveyor commented

2026-09-05 23:37:15 +02:00

Owner

Ticked AC1–AC3 — each verified against the shipped code on #1220, not against the diff's intent.

AC1  errBaseDivergence and errBaseDivergenceUngraded are SEPARATE sentinels,
     asserted non-unifying by TestBaseDivergence_*; rc=1 vs rc=2.
AC2  the disagreement window returns exit 2, not a refusal.
AC3  the message says merge_base "is recomputed lazily, so right after a push
     it can still describe the PREVIOUS head", and tells the operator NOT to
     rebase on the strength of it.

⚠️ Leaving the tracker OPEN deliberately. #1220 uses Refs, not Closes. The three ACs above are discharged; what is not is the question of whether this gate should be a required context, and that is not mine to decide.

Ticked AC1–AC3 — each verified against the shipped code on `#1220`, not against the diff's intent. ``` AC1 errBaseDivergence and errBaseDivergenceUngraded are SEPARATE sentinels, asserted non-unifying by TestBaseDivergence_*; rc=1 vs rc=2. AC2 the disagreement window returns exit 2, not a refusal. AC3 the message says merge_base "is recomputed lazily, so right after a push it can still describe the PREVIOUS head", and tells the operator NOT to rebase on the strength of it. ``` ⚠️ **Leaving the tracker OPEN deliberately.** `#1220` uses `Refs`, not `Closes`. The three ACs above are discharged; what is not is the question of whether this gate should be a required context, and that is not mine to decide.

surveyor commented

2026-09-05 23:39:40 +02:00

Owner

📌 Correction to my comment above: I wrote that #1220 uses Refs, not Closes. It uses Closes, and that is the right keyword here — the sentence was wrong, not the PR.

I had carried over the reasoning from #1170/#1225, where the required-context question genuinely IS open. There is no such question for this tracker: base-divergence-check is deliberately not a required context, and docs/architecture/contracts/cli-surface.md already records why — main carries required_approvals=1 with dismiss_stale_approvals=true, so promoting it would ratchet every PR between red-on-base and dismissed-on-rebase.

So all three ACs are discharged and nothing else on this tracker is outstanding. It should close when #1220 merges, and Intended-targets: #1202 now declares that.

🔑 ac-closure-check is what caught it. It refused #1220 twice for two different reasons — first the unticked ACs, then the undeclared intent — and the second refusal is the one that found a false sentence I had written on this issue. A gate checking whether the PR declares what the keyword will do turns out to also check whether the author knows what their own PR says.

📌 **Correction to my comment above: I wrote that `#1220` uses `Refs`, not `Closes`. It uses `Closes`, and that is the right keyword here — the sentence was wrong, not the PR.** I had carried over the reasoning from `#1170`/`#1225`, where the required-context question genuinely IS open. **There is no such question for this tracker:** `base-divergence-check` is *deliberately* not a required context, and `docs/architecture/contracts/cli-surface.md` already records why — `main` carries `required_approvals=1` with `dismiss_stale_approvals=true`, so promoting it would ratchet every PR between red-on-base and dismissed-on-rebase. **So all three ACs are discharged and nothing else on this tracker is outstanding. It should close when `#1220` merges**, and `Intended-targets: #1202` now declares that. 🔑 `ac-closure-check` is what caught it. It refused `#1220` twice for two different reasons — first the unticked ACs, then the undeclared intent — **and the second refusal is the one that found a false sentence I had written on this issue.** A gate checking whether the PR *declares* what the keyword will do turns out to also check whether the author knows what their own PR says.

surveyor referenced this issue from a commit

2026-09-05 23:50:24 +02:00

fix(base-divergence-check): corroborate the cached merge_base before refusing

surveyor referenced this issue from a commit

2026-09-05 23:50:24 +02:00

fix(changelog): give the #1202 fragment its documented summary header

surveyor referenced this issue from a commit

2026-09-05 23:50:24 +02:00

test(base-divergence-check): hold the SILENT direction of a compare failure

bosun closed this issue

2026-09-06 00:03:31 +02:00