From 0c216d99d0c0e920d5e680300b37b707a31ee274 Mon Sep 17 00:00:00 2001 From: jochen Date: Sun, 25 Oct 2026 18:37:59 +0100 Subject: [PATCH] Gate takes a merge learnt after the fact as past: no trunk move, the forge's time, never source.main (issue 1275, review round 2) A pull request found merged only by its absence from the open list went through PullMerged, which moved every other open delivery of its repository back to an old trunk (ready ones to proposed, composed checks out of date), dated the merge now, and so let a later catch-up write the old commit into source.main. pullMerged now takes such a merge as late: its own delivery is merged and its group closed, no other trunk moves, its time is the forge's or none, and LearntLate keeps it out of source.main in finish, settleForgeMerges and the catch-up. The gitea tool gitea_commit_statuses now answers the pull request's merged_at, and the gate reads a gone pull request by its number alone, so a deleted branch does not fail the read. --- modules/gitea/tools/index.ts | 4 +- .../cmd/mesh-delivery-gate/holder.go | 19 ++++++-- .../cmd/mesh-delivery-gate/issue1275_test.go | 45 +++++++++++++++++++ .../cmd/mesh-delivery-gate/merges.go | 10 ++++- .../cmd/mesh-delivery-gate/model.go | 7 ++- .../cmd/mesh-delivery-gate/ports.go | 4 +- .../cmd/mesh-delivery-gate/unchecked.go | 25 ++++------- 7 files changed, 88 insertions(+), 26 deletions(-) diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index e22a4965..3f36f29a 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -508,11 +508,11 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini if (!owner || !repo) throw new Error("name the repository: owner and repo"); if (!Number.isInteger(number) || number < 0) throw new Error(`${args.number} is not a pull request's number`); let pull: { number: number; title: string; state: string; merged: boolean; base?: string; head?: string; - head_sha?: string; merge_commit_sha?: string; html_url: string } | undefined; + head_sha?: string; merge_commit_sha?: string; merged_at?: string; html_url: string } | undefined; if (number > 0) { const p = await gitea.getPullRequest(owner, repo, number); pull = { number: p.number, title: p.title, state: p.state, merged: p.merged, base: p.base, head: p.head, - head_sha: p.head_sha, merge_commit_sha: p.merge_commit_sha, html_url: p.html_url }; + head_sha: p.head_sha, merge_commit_sha: p.merge_commit_sha, merged_at: p.merged_at, html_url: p.html_url }; if (!ref) ref = p.head_sha ?? ""; if (!base) base = p.base ?? ""; if (!ref) throw new Error(`${owner}/${repo}#${number} names no head commit`); diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go index 8c4779ca..505f36a5 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -646,7 +646,12 @@ func (h *Holder) leaveGroup(d *Delivery) { // PullMerged takes a merge: the commit it landed as, and the delivery it was — made from the forge's word // when this owner never heard its head (merged before it held the seat). -func (h *Holder) PullMerged(p PullEvent) { +func (h *Holder) PullMerged(p PullEvent) { h.pullMerged(p, false) } + +// pullMerged takes a merge. late is a merge learnt after the fact (settleGone): its own delivery is merged and its group +// closed, but no other delivery's trunk moves (the trunk has long moved past it, by merges heard as they happened), +// its time is the forge's or none (never now), and it never moves source.main (LearntLate). +func (h *Holder) pullMerged(p PullEvent, late bool) { if p.MergeCommit == "" { h.Refuse(errors.New("a merge announced with no merge commit; nothing published")) return @@ -721,16 +726,24 @@ func (h *Holder) PullMerged(p PullEvent) { d.MergedAs = p.MergeCommit if t, err := time.Parse(time.RFC3339, p.MergedAt); err == nil { d.MergedAt = t.UTC() - } else { + } else if !late { d.MergedAt = now } + if late { + d.LearntLate, d.InSource = true, true + if d.Ready != nil { + d.Ready.SourceMoved = true + } + } ts = append(ts, settle(d, Facts{Now: now})...) if d.State == Held && d.HeldWhy == "" { d.HeldWhy = "merged without a passing check: only a person decides what follows" } h.moved(d, ts) h.closeGroupOf(d) - h.trunkMoved(d, p.Base, p.MergeCommit, now) + if !late { + h.trunkMoved(d, p.Base, p.MergeCommit, now) + } } // trunkMoved takes a merge into a trunk as that trunk's move (novox/hq ADR 0323 §7): every other open delivery into diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go index 91d67e61..83ecc256 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_test.go @@ -2,6 +2,7 @@ package main import ( "errors" + "fmt" "strings" "sync" "testing" @@ -594,6 +595,50 @@ func TestAnApprovedGroupIsHandedWholeInItsOrderAndAStopIsSaid(t *testing.T) { } } }}, + // Review round 2: a merge learnt after the fact moves no other delivery's trunk (an approved group of the same + // repository keeps its trunk, its state and its composed check), takes the forge's time, and a catch-up after it + // does not move source.main. + {"a merge learnt late moves no trunk and no source", func(t *testing.T) { + w := approvedGroup(t) + allGreen(w) + w.merges.manual = true + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + const lateHead, lateMerge = "9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a9a", "8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b8b" + w.h.PullUpdated(pr(gCat, 9, lateHead, "fix/old", "modules/app/x.go")) + w.h.Checked(judged(gCat, 9, lateHead, "pass", gTrunk0)) + w.settleAll() + late := IDOf(gCat, lateHead) + trunkOf := func() map[string]string { + out := map[string]string{} + for _, id := range []string{IDOf(gCat, gHk), IDOf(gCtl, gHc)} { + out[id] = w.delivery(id).TrunkNow + " " + string(w.delivery(id).State) + } + return out + } + before, moves := trunkOf(), len(w.source.moves) + w.forge.pulls[gCat+"#9"] = ForgePull{Number: 9, State: "closed", Merged: true, MergeCommit: lateMerge, + MergedAt: "2026-10-01T09:00:00Z", HeadSHA: lateHead, Base: "main"} + w.forge.open = []OpenPull{{Owner: "novox", Repo: "mesh-catalog", Number: 2, Base: "main", HeadSHA: gHk}, + {Owner: "novox", Repo: "mesh-controller", Number: 1, Base: "main", HeadSHA: gHc}} + w.h.wantForge() + w.settleAll() + d := w.delivery(late) + if d.State != Merged || !d.MergedAt.Equal(time.Date(2026, 10, 1, 9, 0, 0, 0, time.UTC)) || !d.LearntLate { + t.Fatalf("the late merge is %s at %v (learnt late %v)", d.State, d.MergedAt, d.LearntLate) + } + if after := trunkOf(); fmt.Sprint(after) != fmt.Sprint(before) { + t.Fatalf("the group's trunk or state moved: %v, was %v", after, before) + } + if g := w.h.groups["feat/g"]; g.Check == nil || g.Check.OutOfDate != "" { + t.Fatalf("the group's composed check went out of date: %+v", g.Check) + } + w.h.SourceCatchUp() + w.settleAll() + if len(w.source.moves) != moves { + t.Fatalf("source.main moved for a merge learnt late: %+v", w.source.moves[moves:]) + } + }}, // Review round 1, blocker 2 (1275/AC6): an out-of-date refusal at t0 with no other event is said urgent at t0+10 // minutes: the gate asks itself for a look then. {"an out-of-date refusal turns urgent on the clock alone", func(t *testing.T) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go index 3d7a4499..e7e5f4f9 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -356,6 +356,9 @@ func (h *Holder) settleMerges() []func() { var members []string for hid := range r.Heads { m := h.deliveries[hid] + if m.LearntLate { + continue // learnt after the fact: never written to source.main (settleGone) + } commit := m.MergedAs if m.Ready != nil && m.Ready.MergeCommit != "" { commit = m.Ready.MergeCommit @@ -363,6 +366,9 @@ func (h *Holder) settleMerges() []func() { heads[m.Repository] = commit members = append(members, hid) } + if len(members) == 0 { + continue // every member learnt after the fact: nothing to write + } sort.Strings(members) last := h.deliveries[members[len(members)-1]] out = append(out, func() { h.finish(name, key, approved, last.ID, heads, members) }) @@ -441,7 +447,7 @@ func (h *Holder) settleForgeMerges() []func() { again = h.newestInSource() } for _, d := range h.deliveries { - if d.MergedAs == "" || (d.InSource && !again[d]) || gateMerges(d) { + if d.MergedAs == "" || (d.InSource && !again[d]) || gateMerges(d) || d.LearntLate { continue } if d.Base != MergeBranch || d.replayed() || h.newerMergeOf(d) { @@ -558,7 +564,7 @@ func (h *Holder) SourceCatchUp() { func (h *Holder) newestInSource() map[*Delivery]bool { newest := map[string]*Delivery{} for _, d := range h.deliveries { - if d.MergedAs == "" || !d.InSource || gateMerges(d) || d.Base != MergeBranch || d.replayed() { + if d.MergedAs == "" || !d.InSource || gateMerges(d) || d.LearntLate || d.Base != MergeBranch || d.replayed() { continue } if o := newest[d.Repository]; o == nil || mergedWhen(d).After(mergedWhen(o)) { diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go index fdbc961c..9f2a4de8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -82,8 +82,11 @@ type Delivery struct { MergedAt time.Time `json:"merged_at"` // InSource says the merge was looked at for source.main: written there, or left as older than the trunk's head (issue 888). // A merge the gate handed to the bridge on an Approve is moved there by finish instead (merges.go). - InSource bool `json:"in_source,omitempty"` - HeldWhy string `json:"held_why,omitempty"` + InSource bool `json:"in_source,omitempty"` + // LearntLate says its merge was learnt after the fact, from its absence among the forge's open pull requests + // (settleGone, issue 1275): it moved no trunk here, and it never moves source.main, not even at a catch-up. + LearntLate bool `json:"learnt_late,omitempty"` + HeldWhy string `json:"held_why,omitempty"` // FromForge is a delivery made from the forge's word at its merge: its head was never heard by this owner. FromForge bool `json:"from_forge,omitempty"` diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go index a2e8fb92..83d95a56 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -45,7 +45,9 @@ type ForgePull struct { Head string `json:"head,omitempty"` HeadSHA string `json:"head_sha,omitempty"` MergeCommit string `json:"merge_commit_sha,omitempty"` - HTMLURL string `json:"html_url,omitempty"` + // MergedAt is when it merged, as the forge says it (RFC 3339); empty when not merged or not said. + MergedAt string `json:"merged_at,omitempty"` + HTMLURL string `json:"html_url,omitempty"` } // ForgeStatus is one status of a commit, the newest of its context. diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go index 17132c32..368b3271 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/unchecked.go @@ -253,14 +253,15 @@ func (h *Holder) seeOpen(open []OpenPull) []*Delivery { // settleGone reads from the forge a pull request the open list does not hold, and takes its merge or its close as the // forge's event would have said it (issue 1275, review round 1: a pull request closed on 2026-10-17 was still ready here -// eight days later). A merge learnt this way is past and undated: its delivery is merged, and source.main is not moved -// for it (it could move it back); the trunk's newer merges and the catch-up at start move source.main. A forge that -// cannot say changes nothing. Outside the lock. +// eight days later). A merge learnt this way is past: its delivery is merged with the forge's time, moves no other +// delivery's trunk, and never moves source.main (pullMerged, late); the trunk's merges heard as they happened move +// those. A forge that cannot say changes nothing. Outside the lock. func (h *Holder) settleGone(d *Delivery) { h.mu.Lock() id, owner, repo, number, head, base, branch := d.ID, d.Owner(), d.Repo(), d.Number, d.Commit, d.Base, d.Branch h.mu.Unlock() - fc, err := h.Forge.Statuses(owner, repo, number, head, base) + // Read by its number alone: the forge reads the pull request's own head, which it keeps after the branch is gone. + fc, err := h.Forge.Statuses(owner, repo, number, "", base) if err != nil || fc == nil || fc.Pull == nil { if err != nil { h.Logf("[mesh-delivery] %s is not among the forge's open pull requests, and the forge cannot say what became of it: %v", id, err) @@ -273,18 +274,10 @@ func (h *Holder) settleGone(d *Delivery) { } ev := PullEvent{Owner: owner, Repo: repo, Number: number, Base: base, Head: branch, HeadSHA: head, HTMLURL: p.HTMLURL} if p.Merged && p.MergeCommit != "" { - ev.MergeCommit, ev.Merged = p.MergeCommit, true - h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it merged as %s", id, shortOf(p.MergeCommit, 12)) - h.PullMerged(ev) - h.mu.Lock() - if m := h.deliveries[id]; m != nil && m.MergedAs != "" { - m.InSource = true - if m.Ready != nil { - m.Ready.SourceMoved = true - } - h.keep(m) - } - h.mu.Unlock() + ev.MergeCommit, ev.Merged, ev.MergedAt = p.MergeCommit, true, p.MergedAt + h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it merged as %s at %s", id, + shortOf(p.MergeCommit, 12), orUnknown(p.MergedAt)) + h.pullMerged(ev, true) return } h.Logf("[mesh-delivery] %s is not among the forge's open pull requests: the forge says it was closed unmerged", id)