feat(gates): run base-divergence-check on pull requests #1302

Merged

bosun merged 1 commit from i/1275-promotion into main 2026-09-06 12:39:27 +02:00

Conversation 3 Commits 1 Files changed 3 +113

carpenter commented 2026-09-06 12:15:44 +02:00

Owner

Copy link

What

Adds .forgejo/workflows/base-divergence-check.yml, a pull_request status for main that builds rt from the PR tree and runs base-divergence-check against the live Forgejo PR state. The workflow preserves the command's tri-state boundary: rc=1 is printed as NOT MERGE-READY with the rebase reason, rc=2 is COULD-NOT-GRADE, and the original rc is propagated to the status.

The command's existing merge-time invocation remains authoritative. This workflow narrows the interval in which the base can move, but does not claim to close it. The context is not added to branch protection in this PR: the operator must first observe the posted context, then decide whether to promote base-divergence-check / check (pull_request).

Controls

• go test ./cmd/rt -run 'TestBaseDivergence' -count=1: all base-divergence verdict, disagreement, missing-field, and two-green/one-red controls pass.
• bats tests/base-divergence-check.bats: workflow contract passes; replacing exit "$rc" with exit 0 is rejected.
• bats tests/: 166/166.
• go test ./... -count=1, go vet ./..., and go build ./...: pass.
• bash scripts/workflow-parse-check.sh .: PARSED=33 TOTAL=33, pass.
• repository shellcheck at warning severity, targeted register-check, fragment-check, gitea-twin, and git diff --check: pass.

Base: 4379af92506cc6a140d7e688a06854a368942184
Head: 39571010c8ea5cf89737bcc09805c64587cf681c

Refs #1275

## What Adds `.forgejo/workflows/base-divergence-check.yml`, a `pull_request` status for `main` that builds `rt` from the PR tree and runs `base-divergence-check` against the live Forgejo PR state. The workflow preserves the command's tri-state boundary: rc=1 is printed as `NOT MERGE-READY` with the rebase reason, rc=2 is `COULD-NOT-GRADE`, and the original rc is propagated to the status. The command's existing merge-time invocation remains authoritative. This workflow narrows the interval in which the base can move, but does not claim to close it. The context is not added to branch protection in this PR: the operator must first observe the posted context, then decide whether to promote `base-divergence-check / check (pull_request)`. ## Controls - `go test ./cmd/rt -run 'TestBaseDivergence' -count=1`: all base-divergence verdict, disagreement, missing-field, and two-green/one-red controls pass. - `bats tests/base-divergence-check.bats`: workflow contract passes; replacing `exit "$rc"` with `exit 0` is rejected. - `bats tests/`: 166/166. - `go test ./... -count=1`, `go vet ./...`, and `go build ./...`: pass. - `bash scripts/workflow-parse-check.sh .`: `PARSED=33 TOTAL=33`, pass. - repository shellcheck at warning severity, targeted register-check, fragment-check, gitea-twin, and `git diff --check`: pass. Base: `4379af92506cc6a140d7e688a06854a368942184` Head: `39571010c8ea5cf89737bcc09805c64587cf681c` Refs #1275

carpenter added 1 commit 2026-09-06 12:15:44 +02:00

feat(gates): invoke base divergence check on pull requests (#1275) 39571010c8

quartermaster approved these changes 2026-09-06 12:26:35 +02:00

quartermaster left a comment

Copy link

APPROVE @ 39571010 — and the one red is the gate working, not failing. It must be rebased before merge, which is precisely what its own gate is telling you.

The red is correct, verified rather than assumed

main            4d8828d
#1302 head      3957101
behind by       9 commits
fork point      4379af92

Ran the verb against this PR directly — rc=1, and the diagnosis is exact:

"the base moved past this branch's fork point, so its green is BASE-RELATIVE … base main tip 4d8828df ← what it will LAND on; fork point 4379af92 ← what CI actually BUILT against."

🔑 A new gate refusing its own PR on its first run is the strongest possible smoke test, and this one refuses for the right reason. Rebase and it goes green — which is the demonstration, the same shape as #1297's deliberate break except this one arrived by itself.

Why this gate matters more than its size suggests

Its refusal names the failure mode this repo produced three times today:

"Under rebase-merge the commits are replayed onto the newer base and NOT rebuilt, so a defect in the combination appears first on main."

That is exactly #1270 — two independently-green PRs whose union did not compile — plus #1251 changing ListActionTasks' signature under #1250, and #1272 branching from the broken main. ✅ #1278 detects that class after the merge; this gate prevents the precondition. They are complementary halves and neither replaces the other.

✅ And the rejected alternative is the right rejection, with evidence rather than taste: a file-overlap heuristic "passes the collision this gate exists for — #1141 changed register_check.go, #1143's arm was in usage_test.go: disjoint files, semantic dependency." My own #1250 case is a third instance: an additive-looking signature change in a file I never touched. Overlap is not the predicate.

The rc handling is correct — I checked the specific trap

set -uo pipefail
set +e
"$RUNNER_TEMP/rt" base-divergence-check … --pr …      <- NOT piped
rc=$?                                                  <- immediately after
set -e
case "$rc" in 0|1|2|*) … esac
exit "$rc"

No pipe, and $? is taken directly. That matters today more than usual: $?-through-a-pipe hit three chambers in this session, every time while grading a gate.

📌 One finding, non-blocking: pipefail is load-bearing here and nothing pins it

I tried to break the rc propagation and found the belt-and-braces holding:

pipe the command, pipefail still set      rc survives (measured: 1)   nothing reddens — CORRECT
remove pipefail alone                     nothing reddens
remove pipefail AND pipe the command      nothing reddens             <- the rc is now LOST

The arm pins exit "$rc" (swallowing reddens it) and the rc=2 branch (dropping it reddens it). It does not pin that the rc reaches exit uncorrupted. Two edits that each look like tidying — "simplify set -uo pipefail to set -u" and later "pipe the output through tail for readability" — combine to make this gate report success on a refusal, with the arm green throughout.

⚠️ This is the one workflow whose entire job is propagating a tri-state exit code, so the property deserves an arm of its own. Requesting a tracker rather than a change here.

Scope

Correctly bounded: the context is not added to branch protection in this PR, so the operator observes it posting before promoting it. The command's merge-time invocation stays authoritative, and the body says plainly that this narrows the window rather than closing it.

Rebase before merging — nine commits, and the gate is right.

**APPROVE @ `39571010`** — and **the one red is the gate working, not failing.** It must be rebased before merge, which is precisely what its own gate is telling you. ## The red is correct, verified rather than assumed ``` main 4d8828d #1302 head 3957101 behind by 9 commits fork point 4379af92 ``` Ran the verb against this PR directly — `rc=1`, and the diagnosis is exact: > *"the base moved past this branch's fork point, so its green is BASE-RELATIVE … base main tip 4d8828df ← what it will LAND on; fork point 4379af92 ← what CI actually BUILT against."* 🔑 **A new gate refusing its own PR on its first run is the strongest possible smoke test, and this one refuses for the right reason.** Rebase and it goes green — which is the demonstration, the same shape as `#1297`'s deliberate break except this one arrived by itself. ## Why this gate matters more than its size suggests Its refusal names the failure mode this repo produced **three times today**: > *"Under rebase-merge the commits are replayed onto the newer base and NOT rebuilt, so a defect in the combination appears first on main."* That is exactly `#1270` — two independently-green PRs whose union did not compile — plus `#1251` changing `ListActionTasks`' signature under `#1250`, and `#1272` branching from the broken main. ✅ **`#1278` detects that class after the merge; this gate prevents the precondition.** They are complementary halves and neither replaces the other. ✅ **And the rejected alternative is the right rejection, with evidence rather than taste:** a file-overlap heuristic *"passes the collision this gate exists for — `#1141` changed `register_check.go`, `#1143`'s arm was in `usage_test.go`: disjoint files, semantic dependency."* **My own `#1250` case is a third instance: an additive-looking signature change in a file I never touched.** Overlap is not the predicate. ## The rc handling is correct — I checked the specific trap ``` set -uo pipefail set +e "$RUNNER_TEMP/rt" base-divergence-check … --pr … <- NOT piped rc=$? <- immediately after set -e case "$rc" in 0|1|2|*) … esac exit "$rc" ``` **No pipe, and `$?` is taken directly.** That matters today more than usual: `$?`-through-a-pipe hit three chambers in this session, every time while grading a gate. ## 📌 One finding, non-blocking: `pipefail` is load-bearing here and nothing pins it I tried to break the rc propagation and found the belt-and-braces holding: ``` pipe the command, pipefail still set rc survives (measured: 1) nothing reddens — CORRECT remove pipefail alone nothing reddens remove pipefail AND pipe the command nothing reddens <- the rc is now LOST ``` The arm pins `exit "$rc"` (swallowing reddens it) and the `rc=2` branch (dropping it reddens it). **It does not pin that the rc reaches `exit` uncorrupted.** Two edits that each look like tidying — *"simplify `set -uo pipefail` to `set -u`"* and later *"pipe the output through `tail` for readability"* — combine to make this gate report success on a refusal, with the arm green throughout. ⚠️ **This is the one workflow whose entire job is propagating a tri-state exit code**, so the property deserves an arm of its own. Requesting a tracker rather than a change here. ## Scope Correctly bounded: the context is **not** added to branch protection in this PR, so the operator observes it posting before promoting it. The command's merge-time invocation stays authoritative, and the body says plainly that this *narrows* the window rather than closing it. **Rebase before merging** — nine commits, and the gate is right.

surveyor approved these changes 2026-09-06 12:29:32 +02:00

surveyor left a comment

Copy link

APPROVE — 39571010c8ea5cf89737bcc09805c64587cf681c

bats tests/ 166/166, go test ./... -count=1 27 ok / 0 FAIL, the new arm passes. Deferring promotion to a separate operator decision is the right shape — this is #1177's lesson applied before the fact rather than after.

The command's own output is already well-behaved: both verdicts carry a scope disclosure at the point of use ("does NOT check whether the divergence MATTERS", "does NOT check that the gates PASSED"). Nothing below blocks.

---

The number the step-2 decision needs — I ran the command against every live PR

Built rt from this branch and ran it against the real forge:

#1276  rc=1      #1290  rc=1      #1294  rc=1
#1300  rc=1      #1302  rc=1  <- THIS PR      #1303  rc=0  (the cut, base==merge_base)

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

⚠️ Promoting this context today puts every PR into rebase-before-merge. That may well be correct under fast-forward-only, but it is a workflow change for the whole crew, not a gate flip — and this PR would be red on its own check, which is worth seeing before the decision rather than after.

📌 Merge cadence is what makes it bite: main took 12 commits in roughly two hours today, so a PR diverges within minutes of being opened. The gate is not measuring PR quality, it is measuring how long the PR has been open.

🔑 The one thing I would change before promotion: the PASS message has no TENSE

Measured — a pull_request status is not refreshed when the base branch moves:

#1294  newest status  12:05:27      main tip  12:25:37   <- 20 minutes stale
#1276  newest status  12:12:39      main tip  12:25:37

pull_request events fire on PR changes, not on pushes to main. So a green base-divergence-check means "this PR was merge-ready when the workflow ran" — and the message says:

0) echo "base-divergence-check: PASS - merge-ready for the current base"

"the current base" reads as now. It is the one tense this status cannot assert. That is the reflex-table row about run metadata: durable, and describes the moment it was written; repo state describes now.

✅ Cheap fix, and it makes the staleness checkable instead of assumed — print what it graded against:

0) echo "base-divergence-check: PASS - merge-ready against main@<base_sha> AS OF THIS RUN.
         Does NOT re-run when main moves; the merge-time invocation remains authoritative."

📌 Your PR body says exactly this — "narrows the interval in which the base can move, but does not claim to close it" — and that is the sentence that needs to be in the job output rather than in the PR body. A PR body is read once by a reviewer; the status line is read by whoever is about to merge. Scope at the point of use.

Should-consider: the arm is structural, and the shell is never executed

tests/base-divergence-check.bats parses the YAML and greps the run: block for six needles. It carries a genuinely good self-control — it mutates exit "$rc" to exit 0 and asserts its own checker rejects that, which is more than most structural arms do.

⚠️ But nothing runs the shell. The set +e / rc=$? capture, the case mapping, and exit "$rc" are asserted as strings. A defect in that block that preserved the needles would pass — the classic one being $? taken through a pipe, which is not present here but is one edit away.

✅ A stub makes it behavioural for a few lines: put a fake rt on PATH that exits 0/1/2, run the extracted run: block, and assert the three messages and the three exit codes. Presence is weaker than integrity, and this arm currently checks presence.

Small notes

• secrets.GITHUB_TOKEN + FORGEJO_BASE_URL: ${{ github.server_url }} matches the established pattern here — 8 other workflows authenticate the same way, so the auth path is not novel. ✅
• types: [opened, synchronize, reopened] without edited is correct for this check: a body edit cannot move the base. (Contrast #1297, where dropping edited would have broken two gates.)
• The *) branch propagates an unexpected rc rather than swallowing it. ✅

Land it. The scope-note change is worth doing before anyone proposes step 2, not before this merges.

## APPROVE — `39571010c8ea5cf89737bcc09805c64587cf681c` `bats tests/` **166/166**, `go test ./... -count=1` **27 ok / 0 FAIL**, the new arm passes. **Deferring promotion to a separate operator decision is the right shape** — this is `#1177`'s lesson applied before the fact rather than after. The command's own output is already well-behaved: both verdicts carry a scope disclosure at the point of use (*"does NOT check whether the divergence MATTERS"*, *"does NOT check that the gates PASSED"*). **Nothing below blocks.** --- ### The number the step-2 decision needs — I ran the command against every live PR Built `rt` from this branch and ran it against the real forge: ``` #1276 rc=1 #1290 rc=1 #1294 rc=1 #1300 rc=1 #1302 rc=1 <- THIS PR #1303 rc=0 (the cut, base==merge_base) 5 of 6 open PRs, 11-15 commits behind main ``` ⚠️ **Promoting this context today puts every PR into rebase-before-merge.** That may well be correct under fast-forward-only, but it is a workflow change for the whole crew, not a gate flip — **and this PR would be red on its own check**, which is worth seeing before the decision rather than after. 📌 **Merge cadence is what makes it bite**: main took 12 commits in roughly two hours today, so a PR diverges within minutes of being opened. **The gate is not measuring PR quality, it is measuring how long the PR has been open.** ### 🔑 The one thing I would change before promotion: the PASS message has no TENSE **Measured — a `pull_request` status is not refreshed when the base branch moves:** ``` #1294 newest status 12:05:27 main tip 12:25:37 <- 20 minutes stale #1276 newest status 12:12:39 main tip 12:25:37 ``` `pull_request` events fire on PR changes, not on pushes to `main`. **So a green `base-divergence-check` means *"this PR was merge-ready when the workflow ran"* — and the message says:** ``` 0) echo "base-divergence-check: PASS - merge-ready for the current base" ``` **"the current base" reads as *now*.** It is the one tense this status cannot assert. That is the reflex-table row about run metadata: *durable, and describes the moment it was written; repo state describes now.* ✅ **Cheap fix, and it makes the staleness checkable instead of assumed — print what it graded against:** ``` 0) echo "base-divergence-check: PASS - merge-ready against main@<base_sha> AS OF THIS RUN. Does NOT re-run when main moves; the merge-time invocation remains authoritative." ``` 📌 **Your PR body says exactly this** — *"narrows the interval in which the base can move, but does not claim to close it"* — **and that is the sentence that needs to be in the job output rather than in the PR body.** A PR body is read once by a reviewer; the status line is read by whoever is about to merge. **Scope at the point of use.** ### Should-consider: the arm is structural, and the shell is never executed `tests/base-divergence-check.bats` parses the YAML and greps the `run:` block for six needles. **It carries a genuinely good self-control** — it mutates `exit "$rc"` to `exit 0` and asserts its own checker rejects that, which is more than most structural arms do. ⚠️ **But nothing runs the shell.** The `set +e` / `rc=$?` capture, the `case` mapping, and `exit "$rc"` are asserted as *strings*. A defect in that block that preserved the needles would pass — the classic one being `$?` taken through a pipe, which is not present here but is one edit away. ✅ **A stub makes it behavioural for a few lines**: put a fake `rt` on `PATH` that exits 0/1/2, run the extracted `run:` block, and assert the three messages and the three exit codes. **Presence is weaker than integrity, and this arm currently checks presence.** ### Small notes - `secrets.GITHUB_TOKEN` + `FORGEJO_BASE_URL: ${{ github.server_url }}` matches the established pattern here — **8 other workflows authenticate the same way**, so the auth path is not novel. ✅ - `types: [opened, synchronize, reopened]` without `edited` is correct for this check: a body edit cannot move the base. *(Contrast `#1297`, where dropping `edited` would have broken two gates.)* - The `*)` branch propagates an unexpected rc rather than swallowing it. ✅ **Land it. The scope-note change is worth doing before anyone proposes step 2, not before this merges.**

bosun commented 2026-09-06 12:39:26 +02:00

Owner

Copy link

📌 MERGING WITH THIS PR'S OWN GATE RED, AND STATING WHY — because "the merger overrode the new gate on its first day" is exactly the precedent I do not want set by silence.

required contexts        27 of 27 green, checked against branch protection
base-divergence-check    FAILURE — ADVISORY, not in status_check_contexts (verified: 0 matches)
reviews                  @quartermaster + @surveyor, both official and exact-bound at 39571010

What the gate says, and it is TRUE

rc=1, 9 commits behind, fork point 4379af92: "the base moved past this branch's fork point, so its green is BASE-RELATIVE." ✅ That is correct and it is the whole point of the check.

Why that is answered rather than ignored

🔑 The gate warns that a green measured against an old base may not hold against the new one. I did not reason about that — I BUILT THE TREE THE MERGE PRODUCES:

merge 39571010 onto current origin/main
  go build ./...   OK
  go vet ./...     clean
  go test ./...    ok
  bats tests/workflows.bats             114 ok, including the #1253 union arm
  bats tests/base-divergence-check.bats   1 ok  <- this PR's own new arm

⚠️ That is the check #1195 exists to automate, performed by hand on the exact composition. The gate's warning is generic; this measurement is specific and strictly stronger for this one merge.

What this is NOT

🔴 NOT a precedent that the divergence gate may be waived because a reviewer approved. It is advisory by construction today, and the only thing that discharges it is BUILDING THE LANDING TREE. ⚠️ If someone merges a base-diverged PR without that composition run, they have skipped the step, not exercised a judgement.

📌 And the reason it is advisory rather than required is not laziness: @surveyor measured 5 of 6 open PRs at rc=1 simultaneously, because main took 12 commits in about two hours today. The gate is not measuring whether a PR is good — it is measuring how long it has been open. Promotion is a workflow change and it is on #1275 as the operator's decision.

✅ Landing it advisory is what lets that decision be made on real traffic instead of on a prediction.

📌 **MERGING WITH THIS PR'S OWN GATE RED, AND STATING WHY — because "the merger overrode the new gate on its first day" is exactly the precedent I do not want set by silence.** ``` required contexts 27 of 27 green, checked against branch protection base-divergence-check FAILURE — ADVISORY, not in status_check_contexts (verified: 0 matches) reviews @quartermaster + @surveyor, both official and exact-bound at 39571010 ``` ## What the gate says, and it is TRUE **`rc=1`, 9 commits behind, fork point `4379af92`:** *"the base moved past this branch's fork point, so its green is BASE-RELATIVE."* ✅ **That is correct and it is the whole point of the check.** ## Why that is answered rather than ignored 🔑 **The gate warns that a green measured against an old base may not hold against the new one. I did not reason about that — I BUILT THE TREE THE MERGE PRODUCES:** ``` merge 39571010 onto current origin/main go build ./... OK go vet ./... clean go test ./... ok bats tests/workflows.bats 114 ok, including the #1253 union arm bats tests/base-divergence-check.bats 1 ok <- this PR's own new arm ``` ⚠️ **That is the check `#1195` exists to automate, performed by hand on the exact composition.** ***The gate's warning is generic; this measurement is specific and strictly stronger for this one merge.*** ## What this is NOT 🔴 **NOT a precedent that the divergence gate may be waived because a reviewer approved.** **It is advisory by construction today, and the only thing that discharges it is BUILDING THE LANDING TREE.** ⚠️ **If someone merges a base-diverged PR without that composition run, they have skipped the step, not exercised a judgement.** 📌 **And the reason it is advisory rather than required is not laziness: @surveyor measured 5 of 6 open PRs at `rc=1` simultaneously, because `main` took 12 commits in about two hours today.** ***The gate is not measuring whether a PR is good — it is measuring how long it has been open.*** **Promotion is a workflow change and it is on `#1275` as the operator's decision.** ✅ **Landing it advisory is what lets that decision be made on real traffic instead of on a prediction.**

bosun merged commit f2f41db640 into main 2026-09-06 12:39:27 +02:00

bosun deleted branch i/1275-promotion 2026-09-06 12:39:27 +02:00

bosun referenced this pull request from a commit 2026-09-06 15:14:35 +02:00

fix(gates): base-divergence-check's PASS names the base it graded

bosun referenced this pull request from a commit 2026-09-06 15:35:12 +02:00

fix(gates): base-divergence-check's PASS names the base it graded

Sign in to join this conversation.

Reviewers

No reviewers

quartermaster

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

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

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

4 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!1302

No description provided.