From 0c6b7b5d64ae695906226d0dc9203ed6b0f02f44 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 10:50:39 +0100 Subject: [PATCH 1/4] Gate: seed source.main with the trunk head of a repository it names none of (issue 1422) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit source.main moved only at a merge, so a repository never merged through the gate had no trunk head and the checker could never judge its pull requests. The gate already hears main's head on each gitea.pull.updated (base_sha); it now adds it for a repository source.main lacks, from the newest open pull request into main, in one compare-and-set write that never replaces a head. A pull request into another base seeds nothing (ADR 0484 §11). --- .../cmd/mesh-delivery-gate/holder.go | 2 + .../issue1726_trunk_test.go | 31 ++++++++++ .../cmd/mesh-delivery-gate/merges.go | 61 +++++++++++++++++++ .../cmd/mesh-delivery-gate/ready.go | 1 + 4 files changed, 95 insertions(+) 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 823c89d9..957d4be8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -57,6 +57,8 @@ type Holder struct { // catchUp asks the next look to weigh again the newest merge of each repository against source.main (SourceCatchUp). catchUp bool + // seeded are the repositories source.main was found to name, or was seeded with, since this owner started (seedTrunks). + seeded map[string]bool mu sync.Mutex deliveries map[string]*Delivery diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go index 5c7bd1bc..532c1e63 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go @@ -1,6 +1,7 @@ package main import ( + "strings" "testing" "time" @@ -48,6 +49,36 @@ func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testi t.Fatalf("a second catch-up moved source.main again: %d moves, then %d", n, len(w.source.moves)) } + // Issue 1422: a repository never merged through the gate gets main's head as the forge announced its open pull request + // with, once, beside the others; a pull request into another base, or one announced with no base head (the forge could + // not read it), seeds nothing; a head source.main holds is never replaced. + const never, trunkN, trunkM = "novox/never-merged", "eeee000000000000000000000000000000000001", "eeee000000000000000000000000000000000002" + s := newWorld(t) + s.source.main = &sdk.Source{V: sdk.Version, Group: "before", Heads: map[string]string{i1726Repo: i1726Merge1}} + other := pr("novox/other-base", 6, "ffff000000000000000000000000000000000001", "fix/c", "a.go") + other.Base, other.BaseSHA = "nox-mesh", trunkM + s.h.PullUpdated(other) + unread := pr("novox/unread", 2, "ffff000000000000000000000000000000000002", "fix/d", "a.go") + s.h.PullUpdated(unread) + seed := pr(never, 4, "ffff000000000000000000000000000000000003", "fix/e", "a.go") + seed.BaseSHA = trunkN + s.h.PullUpdated(seed) + s.settleAll() + if h := s.source.main.Heads; h[never] != trunkN || h[i1726Repo] != i1726Merge1 || h["novox/other-base"] != "" || h["novox/unread"] != "" || len(s.source.moves) != 1 { + t.Fatalf("source.main is %v after %d moves; wanted %s seeded at %s, the catalogue kept and nothing else, in one move", h, len(s.source.moves), never, trunkN) + } + if !strings.Contains(s.logs(), "seeded with "+trunkN[:12]) { + t.Errorf("the seed was not said: %s", s.logs()) + } + again := pr(never, 5, "ffff000000000000000000000000000000000004", "fix/f", "a.go") + again.BaseSHA = trunkM + s.h.PullUpdated(again) + s.h.SourceCatchUp() + s.settleAll() + if got := s.source.main.Heads[never]; got != trunkN || len(s.source.moves) != 1 { + t.Fatalf("a seeded head was replaced by %s (%d moves)", got, len(s.source.moves)) + } + o := newWorld(t) o.source.main = &sdk.Source{V: sdk.Version, Group: "newer", Heads: map[string]string{i1726Repo: i1726Merge2}} o.h.PullUpdated(pr(i1726Repo, 471, head2, "fix/b", "modules/app/b.go")) 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 a3055a32..f6fec216 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "reflect" + "regexp" "sort" "strings" "time" @@ -524,6 +525,66 @@ func (h *Holder) settleForgeMerges() []func() { return out } +// trunkHead is a full commit id as the forge names a branch's head. +var trunkHead = regexp.MustCompile(`^[0-9a-f]{40}$`) + +// seedTrunks gives source.main the trunk head of a repository it names none of (issue 1422): one never merged through the +// gate, whose open pull requests the checker could otherwise never judge. The head is the base_sha the forge announced the +// newest open pull request into main with (gitea.pull.updated; ADR 0323), the trunk's head when it was announced. It is +// added in one compare-and-set write that never replaces a head source.main holds, so a merge always wins; the first merge +// through the gate moves it as any other. source.main is per repository and names main's head only (ADR 0484 §11, MergeBranch): +// a pull request into another base seeds nothing. A repository is weighed once per start after it was found or seeded. +// Under the lock; the writing happens outside it. +func (h *Holder) seedTrunks() []func() { + newest := map[string]*Delivery{} + for _, d := range h.deliveries { + if h.seeded[d.Repository] || d.Base != MergeBranch || !trunkHead.MatchString(d.BaseSHA) || d.State.Final() || d.MergedAs != "" { + continue + } + if o := newest[d.Repository]; o == nil || d.Created.After(o.Created) { + newest[d.Repository] = d + } + } + var out []func() + for repository, d := range newest { + id, head, group := d.ID, d.BaseSHA, firstOf(d.Branch, d.Repository) + out = append(out, func() { + seeded := false + err := h.Source.Move(func(prev *sdk.Source) (sdk.Source, bool) { + seeded = false + next := sdk.Source{Group: group, Heads: map[string]string{}} + if prev != nil { + if prev.Heads[repository] != "" { + return *prev, false // named already: never replaced + } + next.Group = prev.Group + for r, held := range prev.Heads { + next.Heads[r] = held + } + } + next.Heads[repository] = head + seeded = true + return next, true + }) + h.mu.Lock() + defer h.mu.Unlock() + if err != nil { + h.Logf("[mesh-delivery] %s: source.main names no trunk head of %s, and it cannot be seeded yet: %v; asked again in %s", id, repository, err, askAgainAfter) + h.again("source.main, seeded with the trunk head of a repository it names none of") + return + } + if h.seeded == nil { + h.seeded = map[string]bool{} + } + h.seeded[repository] = true + if seeded { + h.Logf("[mesh-delivery] source.main named no trunk head of %s: seeded with %s, main's head as the forge announced %s with, so its pull requests are judged (issue 1422)", repository, shortOf(head, 12), id) + } + }) + } + return out +} + // replayLate is how long after its merge a merge heard only from the forge's word is a replay: the forge announces a merge // within seconds, so one heard this late was replayed from the bus's history, and the source already holds what came after. const replayLate = 15 * time.Minute diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go index c3ecbb17..d998ea80 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -294,6 +294,7 @@ func (h *Holder) readyAsks(now time.Time) []func() { asks = append(asks, h.turnWrites()...) // the merge turns, after the holds of this look (ADR 0548 §3) asks = append(asks, h.settleMerges()...) asks = append(asks, h.settleForgeMerges()...) + asks = append(asks, h.seedTrunks()...) asks = append(asks, h.withdrawnKeys()...) asks = append(h.cancels, asks...) h.cancels = nil From e057670bc41fc6551b137e3adce88aae78e3771d Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 11:05:38 +0100 Subject: [PATCH 2/4] Gate: seed source.main from the latest announcement's base head, not the newest delivery's (issue 1422) A delivery made earlier can be announced again after a newer one, with main's head moved on: its base_sha is the freshest. The gate now keeps when each base_sha was announced and seeds from the latest. --- .../cmd/mesh-delivery-gate/holder.go | 9 ++++--- .../issue1726_trunk_test.go | 27 ++++++++++++++++--- .../cmd/mesh-delivery-gate/merges.go | 15 ++++++++--- .../cmd/mesh-delivery-gate/model.go | 3 +++ 4 files changed, 44 insertions(+), 10 deletions(-) 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 957d4be8..c8c3d02a 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -345,8 +345,8 @@ func (h *Holder) pullUpdated(p PullEvent) (isNew bool) { // Announced again: nothing new — but for a delivery whose verdict is out of date, the announcement is its // check asked again against the trunk's head (the merge tool's ask, novox/hq ADR 0323 §5), and its bound // runs from now (§7). The base head it was announced with is kept, for a check this owner asks after it. - if p.BaseSHA != "" && d.BaseSHA != p.BaseSHA { - d.BaseSHA = p.BaseSHA + if p.BaseSHA != "" { + d.BaseSHA, d.BaseSHAAt = p.BaseSHA, now h.keep(d) } // A delivery made from its verdict before its head was heard (issue 1726) holds none of what an announcement @@ -441,7 +441,7 @@ func (h *Holder) fillFromAnnouncement(d *Delivery, p PullEvent) { d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body) d.PushedBy = PushedIn(p.Body) if p.BaseSHA != "" { - d.BaseSHA = p.BaseSHA + d.BaseSHA, d.BaseSHAAt = p.BaseSHA, h.Now() } d.Number = p.Number h.Logf("[mesh-delivery] %s: its pull request's announcement filled its base (%s), branch and title", d.ID, p.Base) @@ -560,6 +560,9 @@ func (h *Holder) freshAnnouncement(p PullEvent, now time.Time, startedOver strin Title: p.Title, Base: p.Base, Branch: p.Head, HTMLURL: p.HTMLURL, CloneURL: p.CloneURL, Paths: p.Paths, PathsTruncated: p.PathsTruncated, Removed: p.Removed, ModuleDirs: p.ModuleDirs, ModuleDirsSaid: p.ModuleDirsSaid, After: AfterIn(p.Body), Issues: IssuesIn(p.Title + "\n" + p.Body), Created: now, BaseSHA: p.BaseSHA} + if p.BaseSHA != "" { + d.BaseSHAAt = now + } d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body) d.PushedBy = PushedIn(p.Body) why := fmt.Sprintf("%s#%d's head announced", d.Repository, d.Number) diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go index 532c1e63..0f3bea30 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go @@ -55,12 +55,12 @@ func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testi const never, trunkN, trunkM = "novox/never-merged", "eeee000000000000000000000000000000000001", "eeee000000000000000000000000000000000002" s := newWorld(t) s.source.main = &sdk.Source{V: sdk.Version, Group: "before", Heads: map[string]string{i1726Repo: i1726Merge1}} - other := pr("novox/other-base", 6, "ffff000000000000000000000000000000000001", "fix/c", "a.go") + other := pr("novox/other-base", 6, "f1f1000000000000000000000000000000000001", "fix/c", "a.go") other.Base, other.BaseSHA = "nox-mesh", trunkM s.h.PullUpdated(other) - unread := pr("novox/unread", 2, "ffff000000000000000000000000000000000002", "fix/d", "a.go") + unread := pr("novox/unread", 2, "f2f2000000000000000000000000000000000002", "fix/d", "a.go") s.h.PullUpdated(unread) - seed := pr(never, 4, "ffff000000000000000000000000000000000003", "fix/e", "a.go") + seed := pr(never, 4, "f3f3000000000000000000000000000000000003", "fix/e", "a.go") seed.BaseSHA = trunkN s.h.PullUpdated(seed) s.settleAll() @@ -70,7 +70,7 @@ func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testi if !strings.Contains(s.logs(), "seeded with "+trunkN[:12]) { t.Errorf("the seed was not said: %s", s.logs()) } - again := pr(never, 5, "ffff000000000000000000000000000000000004", "fix/f", "a.go") + again := pr(never, 5, "f4f4000000000000000000000000000000000004", "fix/f", "a.go") again.BaseSHA = trunkM s.h.PullUpdated(again) s.h.SourceCatchUp() @@ -79,6 +79,25 @@ func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testi t.Fatalf("a seeded head was replaced by %s (%d moves)", got, len(s.source.moves)) } + // The freshest announcement's base head is the seed, not the newest delivery's: #4, made first, is announced again after #5 + // with main's head moved on. + const trunkF = "eeee000000000000000000000000000000000003" + f := newWorld(t) + first := pr(never, 4, "f5f5000000000000000000000000000000000005", "fix/g", "a.go") + first.BaseSHA = trunkN + f.h.PullUpdated(first) + f.later(time.Minute) + second := pr(never, 5, "f6f6000000000000000000000000000000000006", "fix/h", "a.go") + second.BaseSHA = trunkM + f.h.PullUpdated(second) + f.later(time.Minute) + first.BaseSHA = trunkF + f.h.PullUpdated(first) + f.settleAll() + if got := f.source.main.Heads[never]; got != trunkF { + t.Fatalf("seeded with %s; wanted %s, the base head of the latest announcement", got, trunkF) + } + o := newWorld(t) o.source.main = &sdk.Source{V: sdk.Version, Group: "newer", Heads: map[string]string{i1726Repo: i1726Merge2}} o.h.PullUpdated(pr(i1726Repo, 471, head2, "fix/b", "modules/app/b.go")) 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 f6fec216..ec1b6cec 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/merges.go @@ -529,8 +529,8 @@ func (h *Holder) settleForgeMerges() []func() { var trunkHead = regexp.MustCompile(`^[0-9a-f]{40}$`) // seedTrunks gives source.main the trunk head of a repository it names none of (issue 1422): one never merged through the -// gate, whose open pull requests the checker could otherwise never judge. The head is the base_sha the forge announced the -// newest open pull request into main with (gitea.pull.updated; ADR 0323), the trunk's head when it was announced. It is +// gate, whose open pull requests the checker could otherwise never judge. The head is the base_sha of the latest announcement of +// any open pull request into main (gitea.pull.updated; ADR 0323; lastAnnounced), the trunk's freshest head the gate heard. It is // added in one compare-and-set write that never replaces a head source.main holds, so a merge always wins; the first merge // through the gate moves it as any other. source.main is per repository and names main's head only (ADR 0484 §11, MergeBranch): // a pull request into another base seeds nothing. A repository is weighed once per start after it was found or seeded. @@ -541,7 +541,7 @@ func (h *Holder) seedTrunks() []func() { if h.seeded[d.Repository] || d.Base != MergeBranch || !trunkHead.MatchString(d.BaseSHA) || d.State.Final() || d.MergedAs != "" { continue } - if o := newest[d.Repository]; o == nil || d.Created.After(o.Created) { + if o := newest[d.Repository]; o == nil || lastAnnounced(d).After(lastAnnounced(o)) { newest[d.Repository] = d } } @@ -585,6 +585,15 @@ func (h *Holder) seedTrunks() []func() { return out } +// lastAnnounced is when the forge last announced a delivery's head with its base head: BaseSHAAt, or when the gate made it +// for one kept before that was written. +func lastAnnounced(d *Delivery) time.Time { + if !d.BaseSHAAt.IsZero() { + return d.BaseSHAAt + } + return d.Created +} + // replayLate is how long after its merge a merge heard only from the forge's word is a replay: the forge announces a merge // within seconds, so one heard this late was replayed from the bus's history, and the source already holds what came after. const replayLate = 15 * time.Minute 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 e46c968f..05e86510 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/model.go @@ -66,6 +66,9 @@ type Delivery struct { // BaseSHA is the base branch's head the forge last announced its head with (ADR 0323): sent with a check this // owner asks, for a verdict no build judges to name its trunk commit. BaseSHA string `json:"base_sha,omitempty"` + // BaseSHAAt is when the forge last announced its head with BaseSHA: the freshest of a repository's announcements gives + // source.main its trunk head when it names none (seedTrunks, issue 1422). Zero for one kept before it was written. + BaseSHAAt time.Time `json:"base_sha_at,omitzero"` // AskedAgain is when its head was announced again while its verdict was out of date — the merge tool's ask of // its check against the trunk's head (ADR 0323 §5) — and no verdict has come since: its one-hour bound runs from // then. Nil while nothing is asked, and cleared by a verdict or another move of its trunk. From 7c2aa0a7391242517d9bd556655cd8f268a46ce1 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 11:06:53 +0100 Subject: [PATCH 3/4] gitea: set a default branch and a pull request's base by named tools; announce a retargeted pull request again (issue 1422) --- modules/gitea/README.md | 9 ++++ modules/gitea/client.ts | 24 +++++++++++ modules/gitea/guard.ts | 2 + modules/gitea/index.ts | 6 +-- modules/gitea/pulls.ts | 10 ++++- modules/gitea/test/guard.test.ts | 71 +++++++++++++++++++++++++++++-- modules/gitea/test/paging.test.ts | 4 +- modules/gitea/test/pulls.test.ts | 15 ++++++- modules/gitea/test/token.test.ts | 1 + modules/gitea/tools/index.ts | 55 ++++++++++++++++++++++++ 10 files changed, 184 insertions(+), 13 deletions(-) diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 23411ea1..c6fe87c5 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -312,6 +312,15 @@ read-only. With a `user` it adds the teams that user is in and its effective rig listing those above read: the check that a build key's user, `nox-builder` in `builders`, reads and pushes nothing. The tools' token lacks `read:organization` (the forge answers 403), so the read goes with the administrator's login, GET only. +### Two settings have named tools, and a retargeted pull request is judged again + +`gitea_api` refuses a repository's settings and a pull request's base (guard.ts). Issue 1422 note 7 gave each its tool: +`gitea_default_branch_set` (owner, repo, branch, why) and `gitea_pull_base_set` (owner, repo, number, base, why). Both +act as the tools' account, need a reason, refuse a branch the repository does not have, and answer the value before and +after; a default must also be a branch a protection rule guards, and only an open pull request has its base changed. +A pull request's announced head is kept under `owner/repo#number@base`, so a retarget (same head, new base) is +announced again into the new base by the next poll instead of being skipped as already announced. + ### The merging account writes the repositories the mesh delivers `mesh-tools` sets every commit status and merges; outside the organisation it only reads (above), so a pull request of diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 60372f6f..24667981 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -834,6 +834,30 @@ export class GiteaClient { return (raw ?? []).map((b) => ({ name: String(b?.name ?? ""), commit: String(b?.commit?.id ?? ""), protected: Boolean(b?.protected) })); } + /** One branch as the forge keeps it: whether a protection rule guards it; null when the repository has no such branch. */ + async branchState(owner: string, repo: string, branch: string): Promise<{ protected: boolean } | null> { + try { + const b = await this.request(`${repoPath(owner, repo)}/branches/${pathSegments("branch", branch)}`); + return { protected: Boolean(b?.protected) }; + } catch (err) { + if (/: 404 /.test(err instanceof Error ? err.message : "")) return null; + throw err; + } + } + + /** Make a branch the repository's default (issue 1422): what the forge shows, clones and opens pull requests against. */ + async setDefaultBranch(owner: string, repo: string, branch: string): Promise { + return GiteaClient.mapRepo(await this.request(repoPath(owner, repo), { method: "PATCH", body: JSON.stringify({ default_branch: branch }) })); + } + + /** Change the branch an open pull request merges into (issue 1422). The head stays; the forge recomputes the diff. */ + async setPullBase(owner: string, repo: string, index: number, base: string): Promise { + indexOf(index); + return GiteaClient.mapPull( + await this.request(`${repoPath(owner, repo)}/pulls/${index}`, { method: "PATCH", body: JSON.stringify({ base }) }), + ); + } + async deleteBranch(owner: string, repo: string, branch: string): Promise { await this.request(`${repoPath(owner, repo)}/branches/${pathSegments("branch", branch)}`, { method: "DELETE" }); } diff --git a/modules/gitea/guard.ts b/modules/gitea/guard.ts index 414cf46b..e285ab90 100644 --- a/modules/gitea/guard.ts +++ b/modules/gitea/guard.ts @@ -12,6 +12,8 @@ // forge as themselves; // - `gitea_branch_protection_set` lets no person push and no administrator merge past a rule, and on main keeps // the tools' account the only one that merges; it requires no status, as no rule of the mesh does (ADR 0534 §5); +// - `gitea_default_branch_set` and `gitea_pull_base_set` are the two settings gitea_api refuses, as named tools: a branch that +// exists (and, for a default, that a rule guards), a stated reason, the tools' account, the value before and after (issue 1422); // - `gitea_merge_pull_request` merges only into a branch that refuses its push, only a head whose mesh/check passed // against the trunk's head, approved by the operator where the branch requires no status (ADR 0534 §4), and only // that head; diff --git a/modules/gitea/index.ts b/modules/gitea/index.ts index 21e62e13..dad9c88e 100644 --- a/modules/gitea/index.ts +++ b/modules/gitea/index.ts @@ -53,7 +53,7 @@ import { announce, keepWatermark, readWatermark } from "./announce.js"; import { mergeTool } from "./merge.js"; import { GiteaAdmin, GiteaClient, movedSince } from "./client.js"; import { keepMainRules, mainRulesStep } from "./mainrules.js"; -import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, commentFor, composedGate, groupGateStatus, headsToAnnounce, maintained, +import { CHECK_CONTEXT, CHECK_STATUS, REPO_CHECK_CONTEXT, commentFor, composedGate, groupGateStatus, headKey, headsToAnnounce, maintained, requiredOf, statusesFor, writesGate, type Announced, type Checked, repoCheckFacts, reposToLook } from "./pulls.js"; import { GateWriter } from "./gate.js"; import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js"; @@ -243,7 +243,7 @@ let heads: Announced = {}; let primedPulls = false; /** **A head, announced once, whichever path saw it first** (novox/hq issue 729): the poll or the forge's event. */ async function announceHead(client: GiteaClient, repo: RepoOf, pull: OpenOf): Promise { - const key = `${repo.full_name}#${pull.number}`; + const key = headKey(repo.full_name, pull.number, pull.base); const sha = String(pull.head_sha ?? ""); const before = heads[key]; // Claimed before the first wait, so the other path finds it taken while this one is still announcing. @@ -307,7 +307,7 @@ async function pollPulls(client: GiteaClient): Promise { // beyond them unannounced. A sweep and the first pass read every page. const open = await client.listOpenPullsSince(repo.owner, repo.name, sweep ? "" : floor); for (const pull of headsToAnnounce(repo.full_name, open, heads)) { - const key = `${repo.full_name}#${pull.number}`; + const key = headKey(repo.full_name, pull.number, pull.base); const fresh = primedPulls || (!!pull.updated_at && pull.updated_at > dayAgo); // The first look after a start remembers nothing: a head the forge already shows a merge check of was announced. if (fresh && !primedPulls && await alreadyChecked(client, repo, pull)) { diff --git a/modules/gitea/pulls.ts b/modules/gitea/pulls.ts index 602013a8..957e43da 100644 --- a/modules/gitea/pulls.ts +++ b/modules/gitea/pulls.ts @@ -141,13 +141,19 @@ export function planText(plan: ChangePlan): string { return lines.join("\n"); } -/** The heads already announced, keyed `owner/repo#number`, so a restart announces nothing twice. */ +/** The heads already announced, keyed by `headKey`, so a restart announces nothing twice. */ export type Announced = Record; +/** The key a pull request's announced head is kept under: `owner/repo#number@base`. The base is in it because a pull + * request retargeted keeps its head, and a head judged into one base is not judged into another (issue 1422, de-spiegel #6). */ +export function headKey(full: string, number: number, base?: string): string { + return `${full}#${number}@${base ?? ""}`; +} + /** Which open pull requests have a head not yet announced. A pull request merged or closed is not * open and is never asked about. */ export function headsToAnnounce(full: string, pulls: GiteaPull[], announced: Announced): GiteaPull[] { - return pulls.filter((p) => p.state === "open" && !!p.head_sha && announced[`${full}#${p.number}`] !== p.head_sha); + return pulls.filter((p) => p.state === "open" && !!p.head_sha && announced[headKey(full, p.number, p.base)] !== p.head_sha); } /** A pull request the forge found updated, by its repository: what `searchOpenPulls` answers. */ diff --git a/modules/gitea/test/guard.test.ts b/modules/gitea/test/guard.test.ts index 295de67b..f6b1ec9e 100644 --- a/modules/gitea/test/guard.test.ts +++ b/modules/gitea/test/guard.test.ts @@ -34,6 +34,12 @@ const state = { // refuses it the rules' list, as gitea's reqAdmin does, and it reads a branch's protection from the branch. admin: false, lastCommit: "2026-01-01T00:00:00Z", + // The repository's default branch and pull request 7's base, as the forge keeps them (issue 1422 note 7). + defaultBranch: "main", + base: "main", + prState: "open", + // Branches the forge answers 404 for; "scratch" exists but no protection rule guards it. + absent: ["nope"], // The pull request's head; the head the merge commit joined (the same, unless a test says otherwise) — the forge's // record of the pull request keeps saying `head` either way, as it would in the race issue 381 is about; and the // merge commit's own sha. @@ -66,7 +72,10 @@ beforeEach(() => { state.statuses = judgedAt(TRUNK); state.trunk = TRUNK; state.emitted = []; - state.rules = [protectedMain()]; + state.rules = [protectedMain(), { rule_name: "release/*", enable_push: false }]; + state.defaultBranch = "main"; + state.base = "main"; + state.prState = "open"; state.admin = false; state.head = REVIEWED; state.landed = ""; @@ -98,12 +107,22 @@ const server = createServer(async (req, res) => { state.asked.push(`${method} ${path}${url.search}`); if (method !== "GET") state.sent.push({ method, path, body }); const repo = "/repos/novox/mesh-catalog"; - if (method === "GET" && path === repo) return json(res, 200, { full_name: "novox/mesh-catalog", default_branch: "main" }); + if (method === "GET" && path === repo) return json(res, 200, { full_name: "novox/mesh-catalog", default_branch: state.defaultBranch }); + if (method === "PATCH" && path === repo) { + if (body?.default_branch) state.defaultBranch = body.default_branch; + return json(res, 200, { full_name: "novox/mesh-catalog", default_branch: state.defaultBranch }); + } + if (method === "PATCH" && path === `${repo}/pulls/7`) { + if (body?.base) state.base = body.base; + return json(res, 201, { number: 7, title: "x", state: state.prState, merged: false, head: { ref: "feat/x", sha: state.head }, + base: { ref: state.base }, html_url: "http://fake/7" }); + } if (path.startsWith(`${repo}/branch_protections`) && !state.admin) { return json(res, 403, { message: "user should be an owner or a collaborator with admin write of a repository" }); } if (method === "GET" && path === `${repo}/branch_protections`) return json(res, 200, state.rules); const branch = method === "GET" && path.startsWith(`${repo}/branches/`) ? decodeURIComponent(path.slice(`${repo}/branches/`.length)) : null; + if (branch !== null && state.absent.includes(branch)) return json(res, 404, { message: "branch does not exist" }); if (branch !== null) { // As the forge says it to any reader of the code: the rule that covers the branch, as the forge matches it. const rule = (state.rules as any[]).find((r) => ruleCovers(String(r.rule_name ?? r.branch_name), branch)); @@ -125,8 +144,8 @@ const server = createServer(async (req, res) => { return json(res, 200, [{ commit: { committer: { date: state.lastCommit } } }]); } if (method === "GET" && path === `${repo}/pulls/7`) { - return json(res, 200, { number: 7, title: "x", state: state.merged ? "closed" : "open", merged: state.merged, - head: { ref: "feat/x", sha: state.head }, base: { ref: "main" }, html_url: "http://fake/7", + return json(res, 200, { number: 7, title: "x", state: state.merged ? "closed" : state.prState, merged: state.merged, + head: { ref: "feat/x", sha: state.head }, base: { ref: state.base }, html_url: "http://fake/7", merge_commit_sha: state.merged ? state.mergeCommit : null }); } if (method === "GET" && path === `${repo}/git/commits/${state.mergeCommit}`) { @@ -809,3 +828,47 @@ test("gitea_merge_pull_request judges a main requiring mesh/repo-check alone on state.statuses = [judgedAt(TRUNK)[1]]; assert.equal(((await merge()) as Record).merged, true); }); + +// Issue 1422 note 7: the two settings gitea_api refuses (ADR 0266) have named tools, held to the same guard as +// gitea_branch_protection_set: the tools' account, a stated reason, a branch that exists and that a rule guards. +test("gitea_default_branch_set answers the default before and after, and refuses a branch that is missing, unguarded or unexplained", async () => { + const set = (args: Record) => + tool("gitea_default_branch_set").run({ owner: "novox", repo: "mesh-catalog", why: "the trunk was recorded as unknown", ...args }); + await assert.rejects(set({ branch: "nope" }), /no branch nope/); + await assert.rejects(set({ branch: "scratch" }), /no protection rule/); + await assert.rejects(set({ branch: "release/1", why: " " }), /why/); + await assert.rejects(set({ branch: ".." }), /dot segment|not a branch/); + assert.deepEqual(writes(), []); + assert.deepEqual(await set({ branch: "release/1" }), { branch: "release/1", before: "main", after: "release/1", changed: true, + why: "the trunk was recorded as unknown" }); + assert.deepEqual(writes(), ["PATCH /repos/novox/mesh-catalog"]); + assert.deepEqual(state.sent[0].body, { default_branch: "release/1" }); + assert.equal(state.defaultBranch, "release/1"); + state.sent = []; + assert.equal(((await set({ branch: "release/1" })) as any).changed, false, "already the default: nothing is sent"); + assert.deepEqual(writes(), []); +}); + +test("gitea_pull_base_set answers the base before and after, and refuses a base that is missing, a closed pull request or no reason", async () => { + const set = (args: Record) => + tool("gitea_pull_base_set").run({ owner: "novox", repo: "mesh-catalog", number: 7, why: "it was opened against the wrong trunk", ...args }); + await assert.rejects(set({ base: "nope" }), /no branch nope/); + await assert.rejects(set({ base: "release/1", why: "" }), /why/); + await assert.rejects(set({ base: "release/1", number: 0 }), /pull request's number/); + state.prState = "closed"; + await assert.rejects(set({ base: "release/1" }), /not open/); + state.prState = "open"; + assert.deepEqual(writes(), []); + assert.deepEqual(await set({ base: "release/1" }), { number: 7, before: "main", after: "release/1", changed: true, + why: "it was opened against the wrong trunk" }); + assert.deepEqual(writes(), ["PATCH /repos/novox/mesh-catalog/pulls/7"]); + assert.deepEqual(state.sent[0].body, { base: "release/1" }); + assert.equal(state.base, "release/1"); +}); + +test("the two new tools are named for what mesh_search is asked, and gitea_api still refuses both settings", () => { + assert.match(tool("gitea_default_branch_set").description, /default branch/i); + assert.match(tool("gitea_pull_base_set").description, /pull request.s base/i); + assert.ok("refused" in judgeApiCall("PATCH", "/repos/a/b", { default_branch: "x" })); + assert.ok("refused" in judgeApiCall("PATCH", "/repos/a/b/pulls/7", { base: "x" })); +}); diff --git a/modules/gitea/test/paging.test.ts b/modules/gitea/test/paging.test.ts index d6eba14d..aefda7de 100644 --- a/modules/gitea/test/paging.test.ts +++ b/modules/gitea/test/paging.test.ts @@ -45,7 +45,7 @@ test("a new head beyond the newest 20 open pull requests is announced, exactly o const { headsToAnnounce } = await import("../pulls.ts"); // 25 open, every head announced before; #3 — the 23rd most recently updated — got a new head. const announced: Record = {}; - for (let n = 1; n <= 25; n++) announced[`novox/hq#${n}`] = sha(n, "a"); + for (let n = 1; n <= 25; n++) announced[`novox/hq#${n}@main`] = sha(n, "a"); const { client, server, asked } = await serve(25, { 3: sha(3, "b") }); try { // The fault: the newest 20 alone do not hold it. @@ -58,7 +58,7 @@ test("a new head beyond the newest 20 open pull requests is announced, exactly o const open = await client.listOpenPullsSince("novox", "hq", ""); for (const p of headsToAnnounce("novox/hq", open, announced)) { said.push(`#${p.number}@${p.head_sha}`); - announced[`novox/hq#${p.number}`] = String(p.head_sha); + announced[`novox/hq#${p.number}@${p.base}`] = String(p.head_sha); } } assert.deepEqual(said, [`#3@${sha(3, "b")}`], "the new head past the newest 20, announced once over two passes"); diff --git a/modules/gitea/test/pulls.test.ts b/modules/gitea/test/pulls.test.ts index fcecc44b..c548ae78 100644 --- a/modules/gitea/test/pulls.test.ts +++ b/modules/gitea/test/pulls.test.ts @@ -8,16 +8,27 @@ import { createServer } from "node:http"; test("each open pull request's new head is announced once, and nothing closed or merged", async () => { const { headsToAnnounce } = await import("../pulls.ts"); const pulls = [ - { number: 1, title: "a", state: "open", merged: false, head_sha: "aaa", html_url: "" }, + { number: 1, title: "a", state: "open", merged: false, head_sha: "aaa", base: "main", html_url: "" }, { number: 2, title: "b", state: "open", merged: false, head_sha: "bbb", html_url: "" }, { number: 3, title: "c", state: "closed", merged: true, head_sha: "ccc", html_url: "" }, { number: 4, title: "d", state: "open", merged: false, html_url: "" }, ]; - const announced = { "novox/mesh-catalog#1": "aaa", "novox/mesh-catalog#2": "old" }; + const announced = { "novox/mesh-catalog#1@main": "aaa", "novox/mesh-catalog#2@": "old" }; assert.deepEqual(headsToAnnounce("novox/mesh-catalog", pulls, announced).map((p) => p.number), [2], "only a head not announced, of a pull request that is open"); }); +test("a pull request retargeted keeps its head and is announced again, into its new base (de-spiegel #6)", async () => { + const { headsToAnnounce, headKey } = await import("../pulls.ts"); + const was = { number: 6, title: "a", state: "open", merged: false, head_sha: "aaa", base: "old-trunk", html_url: "" }; + const announced: Record = { [headKey("novox/de-spiegel", 6, "old-trunk")]: "aaa" }; + assert.deepEqual(headsToAnnounce("novox/de-spiegel", [was], announced), [], "the same head into the same base is announced once"); + const retargeted = { ...was, base: "main" }; + assert.deepEqual(headsToAnnounce("novox/de-spiegel", [retargeted], announced).map((p) => p.number), [6], + "the same head into a new base is judged afresh"); + assert.notEqual(headKey("novox/de-spiegel", 6, "main"), headKey("novox/de-spiegel", 6, "old-trunk")); +}); + test("a verdict is the head commit's status; an error is the forge's error, never a success", async () => { const { statusFor, commentFor, CHECK_CONTEXT } = await import("../pulls.ts"); const base = { owner: "novox", repo: "mesh-catalog", number: 7, commit: "0123456789abcdef", id: "build-1", on: "laptop" }; diff --git a/modules/gitea/test/token.test.ts b/modules/gitea/test/token.test.ts index a0f83af2..ad7c9e18 100644 --- a/modules/gitea/test/token.test.ts +++ b/modules/gitea/test/token.test.ts @@ -539,6 +539,7 @@ test("the tools register once there is a way to a token, and the first call mint "gitea_list_branches", "gitea_delete_branch", "gitea_branch_protection_get", "gitea_team_rights", "gitea_branch_protection_set", + "gitea_default_branch_set", "gitea_pull_base_set", "gitea_note_append", "gitea_delivery_view", "gitea_commit_statuses", "gitea_open_pulls", "gitea_commit_trees", "gitea_contains", "gitea_list_labels", "gitea_create_label", "gitea_api", diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index fc3b4d0b..7b32e275 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -488,6 +488,61 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }, }, + // ---- Two settings gitea_api refuses (issue 1422 note 7; guard.ts), each its own named tool ---- + { + name: "gitea_default_branch_set", + description: "Set a repository's default branch — the branch the forge shows, clones and opens pull requests against, and the one gitea_api never writes a file to. Refused when the branch does not exist, when no protection rule guards it (a default nothing protects would open it to direct writes), or when no reason (why) is given; acts as the tools' account, so the forge refuses it where that account is no administrator of the repository. Answers the default branch before and after.", + input: { + owner: { type: "string", description: "the repository owner" }, + repo: { type: "string", description: "the repository name" }, + branch: { type: "string", description: "the branch to make the default; it must exist and be guarded by a protection rule" }, + why: { type: "string", description: "why, in a sentence: kept in the call's record" }, + }, + run: async (args) => { + const owner = String(args.owner), repo = String(args.repo), branch = String(args.branch ?? "").trim(); + const why = String(args.why ?? "").trim(); + if (!branch) throw new Error("name the branch to make the default"); + if (!why) throw new Error("say why (why): a default branch is changed for a reason, which the call's record keeps"); + pathSegments("branch", branch); + const state = await gitea.branchState(owner, repo, branch); + if (!state) throw new Refused(`${owner}/${repo} has no branch ${branch}, so it cannot be the default branch. Nothing was sent`); + if (!state.protected) { + throw new Refused(`no protection rule guards ${branch} of ${owner}/${repo}: a default branch nothing protects takes direct writes ` + + `(gitea_branch_protection_set guards it first). Nothing was sent`); + } + const before = String((await gitea.getRepo(owner, repo)).default_branch ?? ""); + if (before === branch) return { branch, before, after: before, changed: false, why }; + const after = String((await gitea.setDefaultBranch(owner, repo, branch)).default_branch ?? ""); + return { branch, before, after, changed: before !== after, why }; + }, + }, + { + name: "gitea_pull_base_set", + description: "Change an open pull request's base — the branch it merges into — keeping its head. The checks were judged against the base it had, so the pull request is announced again into the new one. Refused when the base branch does not exist, when the pull request is not open, or when no reason (why) is given; acts as the tools' account. Answers the pull request's base before and after.", + input: { + owner: { type: "string", description: "the repository owner" }, + repo: { type: "string", description: "the repository name" }, + number: { type: "number", description: "the pull request's number" }, + base: { type: "string", description: "the branch it should merge into; it must exist" }, + why: { type: "string", description: "why, in a sentence: kept in the call's record" }, + }, + run: async (args) => { + const owner = String(args.owner), repo = String(args.repo), number = Number(args.number); + const base = String(args.base ?? "").trim(), why = String(args.why ?? "").trim(); + if (!Number.isInteger(number) || number < 1) throw new Error(`${args.number} is not a pull request's number`); + if (!base) throw new Error("name the base branch"); + if (!why) throw new Error("say why (why): a pull request's base is changed for a reason, which the call's record keeps"); + pathSegments("branch", base); + if (!(await gitea.branchState(owner, repo, base))) throw new Refused(`${owner}/${repo} has no branch ${base}, so a pull request cannot merge into it. Nothing was sent`); + const had = await gitea.getPullRequest(owner, repo, number); + if (had.state !== "open" || had.merged) throw new Refused(`${owner}/${repo}#${number} is not open, so its base is not changed. Nothing was sent`); + const before = String(had.base ?? ""); + if (before === base) return { number, before, after: before, changed: false, why }; + const after = String((await gitea.setPullBase(owner, repo, number, base)).base ?? ""); + return { number, before, after, changed: before !== after, why }; + }, + }, + // ---- A delivery's note, view and statuses (novox/hq ADR 0239), asked by mesh-delivery ---- { name: "gitea_note_append", From f5d4b2f86ed47430eb369cea6a4b4bf180c94ed5 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 11:12:21 +0100 Subject: [PATCH 4/4] Gate: take a retargeted pull request's new base, so a retarget to main is seeded and asked and one away from main is not (issue 1422) A re-announcement updated the base head but never the base: a pull request retargeted from nox-mesh to main stayed 'into nox-mesh' and was never asked or seeded, and one retargeted away from main kept its open ask. --- .../cmd/mesh-delivery-gate/holder.go | 23 ++++++++++++++ .../issue1726_trunk_test.go | 31 +++++++++++++++++++ 2 files changed, 54 insertions(+) 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 c8c3d02a..b2337024 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -352,6 +352,8 @@ func (h *Holder) pullUpdated(p PullEvent) (isNew bool) { // A delivery made from its verdict before its head was heard (issue 1726) holds none of what an announcement // carries: filled from this one, so its base is known and the operator is asked. h.fillFromAnnouncement(d, p) + // A pull request retargeted to another base (review of issue 1422): what it merges into is the forge's word now. + h.retarget(d, p) // The review record, corrected in the description without a push, is read again (ADR 0421 §5). if by, at := ReviewIn(p.Body); p.Body != "" && (by != d.ReviewedBy || at != d.ReviewedAt) { d.ReviewedBy, d.ReviewedAt = by, at @@ -453,6 +455,27 @@ func (h *Holder) fillFromAnnouncement(d *Delivery, p PullEvent) { h.Poke() } +// retarget takes the base a live, unmerged delivery's pull request was announced again with when it differs from the one +// kept: into main it is asked and merged as any other, and source.main may be seeded from it (seedTrunks); out of main an +// ask nobody answered yet is taken back, and the next look says it is not asked (ready.go). A delivery whose base is not yet +// known is fillFromAnnouncement's. Said in the log. Under the lock. +func (h *Holder) retarget(d *Delivery, p PullEvent) { + if p.Base == "" || d.Base == "" || p.Base == d.Base || d.State.Final() || d.MergedAs != "" || + (d.Ready != nil && d.Ready.Merged != nil) { + return + } + from := d.Base + d.Base = p.Base + if r := d.Ready; r != nil { + r.OtherBase = "" + if r.AskID != "" && r.Approved == nil { + h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, fmt.Sprintf("%s was retargeted from %s to %s", d.ID, from, p.Base))) + } + } + h.Logf("[mesh-delivery] %s: its pull request was retargeted from %s to %s", d.ID, from, p.Base) + h.keep(d) +} + // PullEdited takes a pull request's edit (novox/hq issue 1079): its description is read again for the review record, the // `after:` lines and the issues it names, exactly as an announcement reads them. An Approve ask still unanswered whose words // the new record changes is withdrawn and asked again with it read, without a word on the pull request: nothing was diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go index 0f3bea30..4848162d 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1726_trunk_test.go @@ -98,6 +98,37 @@ func TestStartingWritesTheNewestMergeOfEachRepositoryNotYetInSourceMain(t *testi t.Fatalf("seeded with %s; wanted %s, the base head of the latest announcement", got, trunkF) } + // A green pull request retargeted from nox-mesh to main is seeded and asked; one retargeted from main to nox-mesh has + // its ask taken back and is not merged (review round 2). + retarget := func(from, to string) (*world, int) { + w := newWorld(t) + w.op.auto = false + p := pr(never, 7, readyHead, "feat/x", "src/a.go") + p.Base, p.BaseSHA = from, trunkN + w.h.PullUpdated(p) + w.h.Checked(judgedAt(never, 7, readyHead, "pass", "t1t1t1t1t1t1")) + open := openPull(never, 7, readyHead) + open.Base = from + w.forge.open = []OpenPull{open} + w.h.SeeOpenPulls() + w.settleAll() + before := len(w.op.open()) + p.Base, open.Base = to, to + w.forge.open = []OpenPull{open} + w.h.PullUpdated(p) + w.h.SeeOpenPulls() + w.settleAll() + w.later(2 * time.Minute) + w.settleAll() + return w, before + } + if r, before := retarget("nox-mesh", "main"); before != 0 || r.source.main == nil || r.source.main.Heads[never] != trunkN || len(r.op.open()) != 1 { + t.Fatalf("retargeted to main: %d asks before, source.main %+v, open asks %v; wanted none before, then seeded at %s and asked", before, r.source.main, r.op.open(), trunkN) + } + if r, before := retarget("main", "nox-mesh"); before != 1 || len(r.op.open()) != 0 || len(r.forge.merges) != 0 || !strings.Contains(r.logs(), "retargeted") { + t.Fatalf("retargeted to nox-mesh: %d asks before, open asks %v, merges %v; wanted one before, then none and the retarget said", before, r.op.open(), r.forge.merges) + } + o := newWorld(t) o.source.main = &sdk.Source{V: sdk.Version, Group: "newer", Heads: map[string]string{i1726Repo: i1726Merge2}} o.h.PullUpdated(pr(i1726Repo, 471, head2, "fix/b", "modules/app/b.go"))