diff --git a/modules/gitea/client.ts b/modules/gitea/client.ts index 418c3e49..ed94c29c 100644 --- a/modules/gitea/client.ts +++ b/modules/gitea/client.ts @@ -98,6 +98,10 @@ export interface GiteaPull { html_url: string; /** The description: where a pull request says it goes after another (`after: `, novox/hq ADR 0239). */ body?: string; + /** Whether the forge would merge it into its base now (gitea's `mergeable`): false while it is in conflict, while the + * forge is still working that out after its base moved, or while it is a work in progress; absent when the forge did not + * say. What a group's merge reads of every member before it merges any (novox/hq issue 1275). */ + mergeable?: boolean; } /** An open pull request with what its base requires and what its head has: what gitea_open_pulls answers. */ @@ -789,14 +793,29 @@ export class GiteaClient { /** Merge a pull request — never forced past its checks, and, given the head that was checked, only that head: * the forge refuses the merge when the branch moved since (novox/hq ADR 0266). */ + /** How long a merge waits before it asks again after the forge's 405 "Please try again later" (novox/hq issue 1275): + * the forge says so while it works out whether the pull request merges, for a few seconds after its base moved. */ + static mergeRetryAfter: number[] = [1000, 2000, 4000]; + async mergePullRequest(owner: string, repo: string, index: number, method = "merge", deleteBranch = false, headCommit?: string): Promise { indexOf(index); - await this.request(`${repoPath(owner, repo)}/pulls/${index}/merge`, { - method: "POST", - body: JSON.stringify({ Do: method, delete_branch_after_merge: deleteBranch, force_merge: false, - ...(headCommit ? { head_commit_id: headCommit } : {}) }), - }); + for (let attempt = 0; ; attempt++) { + try { + await this.request(`${repoPath(owner, repo)}/pulls/${index}/merge`, { + method: "POST", + body: JSON.stringify({ Do: method, delete_branch_after_merge: deleteBranch, force_merge: false, + ...(headCommit ? { head_commit_id: headCommit } : {}) }), + }); + return; + } catch (err) { + // Only the forge's transient answer is asked again, briefly; every other refusal, and this one past the last + // wait, is the caller's to say. head_commit_id keeps a head that moved in between from landing. + const wait = GiteaClient.mergeRetryAfter[attempt]; + if (wait === undefined || !tryAgainLater(err)) throw err; + await new Promise((r) => setTimeout(r, wait)); + } + } } // ---- Mappers: the wire shape is broad and unstable; the mesh sees only these fields. ---- @@ -842,6 +861,7 @@ export class GiteaClient { updated_at: p.updated_at ?? undefined, html_url: p.html_url, body: typeof p.body === "string" ? p.body : undefined, + mergeable: typeof p.mergeable === "boolean" && !p.merged ? p.mergeable : undefined, }; } } @@ -1229,3 +1249,10 @@ export function directoriesAbove(paths: string[]): string[] { function encodePath(path: string): string { return pathSegments("file path", path); } + +/** Whether an error is the forge's 405 "Please try again later" on a merge: its answer while the pull request's + * mergeability is being worked out (or it is in conflict, which a later ask says the same way). */ +export function tryAgainLater(err: unknown): boolean { + const words = err instanceof Error ? err.message : String(err); + return / 405 /.test(words) && /try again later/i.test(words); +} diff --git a/modules/gitea/test/merge-retry.test.ts b/modules/gitea/test/merge-retry.test.ts new file mode 100644 index 00000000..e8f8de58 --- /dev/null +++ b/modules/gitea/test/merge-retry.test.ts @@ -0,0 +1,68 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createServer } from "node:http"; + +// **The forge's 405 "Please try again later" on a merge is asked again briefly** (novox/hq issue 1275, 2026-10-26 13:40): +// mesh-delivery#73's merge was answered so a few seconds after its base moved, and counted as refused at once. The forge +// says it while it works out whether the pull request merges; the merge is asked again after a short wait, and every other +// refusal is answered at once. A pull request's own mergeable is carried, so a group can be read whole before it merges. + +/** A forge whose merge of novox/hq#7 answers each of `answers` in turn (the last for ever), and whose pull request says + * `mergeable`. */ +async function serve(answers: { status: number; body: string }[], mergeable?: boolean) { + const { GiteaClient } = await import("../client.ts"); + let merges = 0; + const server = createServer((req, res) => { + const url = new URL(req.url ?? "", "http://x"); + if (url.pathname === "/api/v1/repos/novox/hq/pulls/7/merge" && req.method === "POST") { + const a = answers[Math.min(merges++, answers.length - 1)]; + res.statusCode = a.status; + return res.end(a.body); + } + if (url.pathname === "/api/v1/repos/novox/hq/pulls/7") { + return res.end(JSON.stringify({ number: 7, state: "open", merged: false, base: { ref: "main" }, + head: { ref: "b", sha: "a".repeat(40) }, html_url: "h/7", ...(mergeable === undefined ? {} : { mergeable }) })); + } + res.statusCode = 404; + res.end("{}"); + }); + await new Promise((r) => server.listen(0, r)); + const client = new GiteaClient(`http://127.0.0.1:${(server.address() as any).port}`, "t"); + GiteaClient.mergeRetryAfter = [1, 1, 1]; + return { client, server, merges: () => merges }; +} + +const later = { status: 405, body: '{"message":"Please try again later"}' }; +const ok = { status: 200, body: "" }; + +test("a merge answered 'try again later' while the forge works out its mergeability, then merged", async (t) => { + const cases: { name: string; answers: { status: number; body: string }[]; merged: boolean; asked: number }[] = [ + { name: "merged on the third ask", answers: [later, later, ok], merged: true, asked: 3 }, + { name: "still 'later' after the last wait: refused, with the forge's words", answers: [later], merged: false, asked: 4 }, + { name: "a refusal of another kind is answered at once", answers: [{ status: 409, body: '{"message":"head moved"}' }], merged: false, asked: 1 }, + ]; + for (const c of cases) { + await t.test(c.name, async () => { + const { client, server, merges } = await serve(c.answers); + try { + const done = client.mergePullRequest("novox", "hq", 7, "merge", false, "a".repeat(40)); + if (c.merged) await done; + else await assert.rejects(done, (err: Error) => /40[59]/.test(err.message)); + assert.equal(merges(), c.asked); + } finally { + server.close(); + } + }); + } +}); + +test("a pull request carries the forge's mergeable, and none when the forge did not say", async () => { + for (const mergeable of [true, false, undefined]) { + const { client, server } = await serve([ok], mergeable); + try { + assert.equal((await client.getPullRequest("novox", "hq", 7)).mergeable, mergeable); + } finally { + server.close(); + } + } +}); diff --git a/modules/gitea/tools/index.ts b/modules/gitea/tools/index.ts index 3f36f29a..fe33da7e 100644 --- a/modules/gitea/tools/index.ts +++ b/modules/gitea/tools/index.ts @@ -492,7 +492,7 @@ function allGiteaTools(gitea: GiteaClient, admin?: () => GiteaAdmin): ToolDefini }, { name: "gitea_commit_statuses", - description: "Read a commit's statuses — or a pull request's head's — as the forge keeps them: the newest of each context (mesh/check, mesh/delivery, …) with its state, description, link, when it was set and by whom; and whether the branch it merges into would let it merge, every status that branch's protection requires being a success. Reads only.", + description: "Read a commit's statuses — or a pull request's head's — as the forge keeps them: the newest of each context (mesh/check, mesh/delivery, …) with its state, description, link, when it was set and by whom; and whether the branch it merges into would let it merge, every status that branch's protection requires being a success; for a pull request, also the forge's own mergeable (false while it is in conflict with its base or the forge is still working that out). Reads only.", input: { owner: { type: "string", description: "the repository owner" }, repo: { type: "string", description: "the repository name" }, @@ -508,11 +508,12 @@ 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; merged_at?: string; html_url: string } | undefined; + head_sha?: string; merge_commit_sha?: string; merged_at?: string; html_url: string; mergeable?: boolean } | 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, merged_at: p.merged_at, 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, + ...(p.mergeable === undefined ? {} : { mergeable: p.mergeable }) }; 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/approval.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go index 83d7af29..3229d6d8 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/approval.go @@ -390,12 +390,12 @@ func (h *Holder) approval(d *Delivery, now time.Time) (string, func()) { if gid == "" { break } - says, err := h.notASuccess(m) + says, forge, err := h.notASuccess(m) if err != nil { readErr = err break } - if says != "" { + if says != "" && !forge { // the forge's merge answer is weighed at the merge, where it is asked again (group_merge.go) holdMember, holdSays = m.id, says break } diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go index bb41b294..8b2fd6a2 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/group_merge.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "sort" "strings" "time" ) @@ -28,26 +29,46 @@ func readOf(d *Delivery, head string) memberRead { return memberRead{id: d.ID, owner: d.Owner(), repo: d.Repo(), base: d.Base, head: head, number: d.Number} } -// notAsSuccess reads a member's required statuses at its head and says, in the forge's words, which are not a success; -// empty when all are. An error is a +// forgeMergeGrace is how long the forge may say a member does not merge now before the group is held for it (issue 1275): +// the forge says so for a few seconds after a base moved, while it works out whether the pull request still merges; within +// the grace the merge is asked again every forgeMergeAgain. Past it, the member is in conflict with its trunk: held, said +// urgent, and asked again on the next event (a new head, a status). +const ( + forgeMergeGrace = 2 * time.Minute + forgeMergeAgain = 5 * time.Second +) + +// notASuccess reads a member's required statuses at its head and says, in the forge's words, which are not a success, +// and whether the forge says the pull request merges into its base now (its head still the one approved, and not in +// conflict); empty when all hold. forge says the reason is the forge's merge answer rather than a status. An error is a // read that could not be made: the member then counts as not mergeable. -func (h *Holder) notASuccess(m memberRead) (says string, err error) { +func (h *Holder) notASuccess(m memberRead) (says string, forge bool, err error) { fc, err := h.Forge.Statuses(m.owner, m.repo, m.number, m.head, m.base) if err != nil { - return "", err + return "", false, err } if fc.Merge.Mergeable == nil { - return "", fmt.Errorf("the forge could not say whether %s may merge: %s", m.id, fc.Merge.Says) + return "", false, fmt.Errorf("the forge could not say whether %s may merge: %s", m.id, fc.Merge.Says) } - if *fc.Merge.Mergeable { - return "", nil + if !*fc.Merge.Mergeable { + var parts []string + for _, b := range fc.Merge.Blocking { + parts = append(parts, fmt.Sprintf("%s (%s)", b.Context, b.State)) + } + return fmt.Sprintf("%s requires %s; not a success: %s", fc.Merge.Branch, strings.Join(fc.Merge.Required, ", "), + strings.Join(parts, ", ")), false, nil } - var parts []string - for _, b := range fc.Merge.Blocking { - parts = append(parts, fmt.Sprintf("%s (%s)", b.Context, b.State)) + if p := fc.Pull; p != nil && m.number > 0 { + if p.HeadSHA != "" && !sameCommit(p.HeadSHA, m.head) { + return fmt.Sprintf("its head is now %s, not the %s approved", shortOf(p.HeadSHA, 12), shortOf(m.head, 12)), false, nil + } + if p.Mergeable != nil && !*p.Mergeable { + base := firstOf(p.Base, m.base) + return fmt.Sprintf("the forge says it does not merge into %s now (in conflict with it, a work in progress, or still "+ + "being worked out after %s moved)", base, base), true, nil + } } - return fmt.Sprintf("%s requires %s; not a success: %s", fc.Merge.Branch, strings.Join(fc.Merge.Required, ", "), - strings.Join(parts, ", ")), nil + return "", false, nil } // unmergedLive are a group's members not yet merged and not over, in the group's order. Under the lock. @@ -61,10 +82,13 @@ func (h *Holder) unmergedLive(g *Group) []*Delivery { return out } -// groupMergeable reads every member's required statuses at the head the Approve covers, and says whether all are a success. -// When one is not, or cannot be read, nothing is handed to the bridge: the group is held, said urgent (ADR 0441 §2, §4), and -// the target's turn is given back so the next event that decides whether it may merge asks it again. A read that could not -// be made is a failed ask, tried again after askAgainAfter. Outside the lock. +// groupMergeable reads every member's required statuses, its head and the forge's merge answer at the head the Approve +// covers, and says whether all hold: no member is merged unless every member merges now (issue 1275, 2026-10-26: two of +// three merged and the third refused). When one does not, or cannot be read, nothing is handed to the bridge: the group is +// held, said urgent (ADR 0441 §2, §4), and the target's turn is given back so the next event that decides whether it may +// merge asks it again. A read that could not be made is a failed ask, tried again after askAgainAfter; the forge saying a +// member does not merge now is asked again every forgeMergeAgain for forgeMergeGrace before the group is held. Outside +// the lock. func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { release := func() { if d := h.deliveries[target]; d != nil && d.Ready != nil { @@ -73,7 +97,7 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { } } for _, m := range reads { - says, err := h.notASuccess(m) + says, forge, err := h.notASuccess(m) if err == nil && says == "" { continue } @@ -85,6 +109,25 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { h.mu.Unlock() return false } + if forge { + if h.unmergeableSince == nil { + h.unmergeableSince = map[string]time.Time{} + } + key, now := gid+" "+m.id, h.Now() + first, seen := h.unmergeableSince[key] + if !seen { + first = now + h.unmergeableSince[key] = now + h.Logf("[mesh-delivery] group %s: nothing is merged yet: %s at %s: %s; asked again every %s for %s", gid, m.id, + shortOf(m.head, 12), says, forgeMergeAgain, forgeMergeGrace) + } + if now.Sub(first) < forgeMergeGrace { + release() + h.lookAt(now.Add(forgeMergeAgain), "a group's member the forge was still weighing") + h.mu.Unlock() + return false + } + } if g := h.groups[gid]; g != nil { if g.Hold == nil || g.Hold.Kind != "blocked" || g.Hold.Says != says || g.Hold.Member != m.id { h.Logf("[mesh-delivery] group %s is approved and NOT merged: %s at %s: %s", gid, m.id, shortOf(m.head, 12), says) @@ -99,13 +142,18 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { g.Hold = &GroupHold{Kind: "blocked", Member: m.id, Says: says, Since: since} h.keepGroup(g) } - // No timer: the status turning success is an event on the forge (CommitStatus, mergeAgainNow), and it asks the - // merge again at once (issue 1275: a pass that stops part way resumes on an event). + // No timer: the status turning success, or a new head, is an event on the forge (CommitStatus, mergeAgainNow), and + // it asks the merge again at once (issue 1275: a pass that stops part way resumes on an event). release() h.mu.Unlock() return false } h.mu.Lock() + for key := range h.unmergeableSince { + if strings.HasPrefix(key, gid+" ") { + delete(h.unmergeableSince, key) + } + } if g := h.groups[gid]; g != nil && g.Hold != nil { g.Hold = nil h.keepGroup(g) @@ -114,6 +162,98 @@ func (h *Holder) groupMergeable(gid, target string, reads []memberRead) bool { return true } +// passWaits is why an approved merge is not handed yet because another approved merge's pass is under way into a +// repository it goes into; empty when none is (issue 1275, 2026-10-26 13:40: group feat/890-placement merged +// mesh-delivery#72 between the members of fix/734-lost-ask, whose mesh-delivery#73 then conflicted). A pass is under way +// from its first member handed until every member is merged; one half merged stays under way while it is refused or held, +// until it is whole, declined, or its remaining members are gone or asked anew (a new head is a new Approve). The first +// merge handed has the repositories; the other is handed once that pass ends, on the event that ends it. This holds +// whether or not mesh/check gates the merge: it waits for a merge, never for a check (ADR 0551). Under the lock. +func (h *Holder) passWaits(d *Delivery) string { + c := h.covering(d) + if c == nil || c.Approved == nil || c.AskID == "" { + return "" + } + mine := h.unmergedRepos(c) + for _, p := range h.passes() { + if p.run.AskID == c.AskID || !h.passUnderWay(p.run) { + continue + } + for repo := range h.unmergedRepos(p.run) { + if mine[repo] { + return fmt.Sprintf("approved; waits for the merge of %s into %s to end, so no other merge lands there between "+ + "its members (issue 1275)", p.name, repo) + } + } + } + return "" +} + +// pass is an approved merge not merged whole: the run holding its Approve and the name of its group (or pull request). +type pass struct { + run *ReadyRun + name string +} + +// passes are the approved merges not merged whole and not declined, each ask once. Unlike approvedMerges it keeps one whose +// members merged or were refused: a half-merged pass still holds its repositories (issue 1275). Under the lock. +func (h *Holder) passes() []pass { + ids := make([]string, 0, len(h.deliveries)) + for id := range h.deliveries { + ids = append(ids, id) + } + sort.Strings(ids) + seen := map[string]bool{} + var out []pass + for _, id := range ids { + d := h.deliveries[id] + r := d.Ready + if r == nil || r.Approved == nil || r.Declined != nil || r.AskID == "" || len(r.Heads) == 0 || seen[r.AskID] || + d.Base != MergeBranch || h.allMerged(r.Heads) { + continue + } + seen[r.AskID] = true + out = append(out, pass{run: r, name: askGroup(d)}) + } + return out +} + +// unmergedRepos are the repositories (lowercase) of the deliveries an Approve covers that are not merged and may still be +// merged on it: not over (closed, held), at the head approved, and covered by this Approve, not a newer one. Under the lock. +func (h *Holder) unmergedRepos(r *ReadyRun) map[string]bool { + out := map[string]bool{} + for id, sha := range r.Heads { + m := h.deliveries[id] + if m == nil || m.MergedAs != "" || (m.Ready != nil && m.Ready.Merged != nil) || m.State.Final() || !sameCommit(m.Commit, sha) { + continue + } + if c := h.covering(m); c == nil || c.AskID != r.AskID { + continue // asked anew (its new head, or a new Approve): that ask is the pass now + } + out[strings.ToLower(m.Repository)] = true + } + return out +} + +// passUnderWay is whether an approved merge's pass has started and not ended: a member is handed to the bridge, or some +// are merged and others may still merge on this Approve, refused or held or not (issue 1275, review: a held half-merged +// group that let another group start, then asked again, waited on that group while it waited on it). Under the lock. +func (h *Holder) passUnderWay(r *ReadyRun) bool { + if len(h.unmergedRepos(r)) == 0 { + return false + } + for id := range r.Heads { + m := h.deliveries[id] + if m == nil { + continue + } + if m.MergedAs != "" || (m.Ready != nil && m.Ready.Merged != nil) || (m.Ready != nil && m.Ready.merging) { + return true + } + } + return false +} + // groupLines are the groups that are held or half merged, as urgent stalled lines the gate says as conditions (conditions.go): // one for each, on its first member not merged, until every member is merged or a person decides (issue 1275). A member // the merge tool refused is said by readyLines, with its group; this adds a group held for a required status, and a @@ -149,9 +289,10 @@ func (h *Holder) groupLines(now time.Time) []StalledLine { } out = append(out, StalledLine{ID: g.Hold.Member, Number: numberOf(h.deliveries[g.Hold.Member]), State: first.State, For: now.Sub(g.Hold.Since).Round(time.Second).String(), Bound: "0s", Severity: "urgent", - H2: "none: the required status turns success, which asks the merge again — or the operator's decision", - Says: fmt.Sprintf("group %s %s (merged %d of %d): %s waits on a required status "+ - "at its head: %s. Needed: that status turns success (its check asked again), or the operator decides.", + H2: "none: the required status turns success or a new head merges, which asks the merge again — or the operator's decision", + Says: fmt.Sprintf("group %s %s (merged %d of %d): %s does not merge now "+ + "at its head: %s. Needed: that status turns success (its check asked again), a new head that merges "+ + "(rebased on its trunk), or the operator decides.", g.ID, stands, merged, len(h.membersOf(g)), g.Hold.Member, g.Hold.Says)}) case merged > 0 && !refused && !running: out = append(out, StalledLine{ID: first.ID, Number: first.Number, State: first.State, @@ -177,3 +318,69 @@ func holdMember(g *Group) string { } return g.Hold.Member } + +// MergeAgain is the tool delivery_merge_again: on the operator's word, the merge of an approved delivery the forge refused, +// or of a group held part way, is asked again at once at the heads the Approve named (issue 1275): a refusal the forge +// meant as "later", a conflict cleared on the trunk, or a refusal repeated until the gate stopped asking. A delivery whose +// head moved is asked again by its new head, never by this. Refused, naming why, for one that is not an approved merge +// waiting after a refusal or a hold. +func (h *Holder) MergeAgain(id, why string) (string, error) { + h.mu.Lock() + defer h.mu.Unlock() + d := h.deliveries[id] + if d == nil { + return "", fmt.Errorf("no delivery %q", id) + } + if strings.TrimSpace(why) == "" { + return "", fmt.Errorf("say why: the merge asked again is the operator's word, kept in the log") + } + c := h.covering(d) + if c == nil || c.Approved == nil || c.Declined != nil || c.Heads[d.ID] != d.Commit { + return "", fmt.Errorf("%s is not approved at its head %s: nothing to ask again (a new head is asked on its own)", id, shortOf(d.Commit, 12)) + } + if d.MergedAs != "" || (d.Ready != nil && d.Ready.Merged != nil) { + return "", fmt.Errorf("%s is merged already", id) + } + g := h.groups[d.Group] + if d.Group == "" { + g = nil + } + held := g != nil && g.Hold != nil && g.Hold.Kind == "blocked" + refused := false + for hid := range c.Heads { + if m := h.deliveries[hid]; m != nil && m.Ready != nil && m.Ready.Merged == nil && m.Ready.RefusedSince != nil { + refused = true + } + } + if !held && !refused { + return "", fmt.Errorf("%s was neither refused nor held: %s", id, firstOf(d.readyWords(), "it is merged at its turn")) + } + if waits := h.passWaits(d); waits != "" { + return "", fmt.Errorf("%s is not asked again yet: it %s; it is asked at that pass's end", id, strings.TrimPrefix(waits, "approved; ")) + } + for hid := range c.Heads { + if m := h.deliveries[hid]; m != nil && m.Ready != nil && m.Ready.Merged == nil { + m.Ready.Repeats, m.Ready.LastTry, m.Ready.merging = 0, time.Time{}, false + h.keep(m) + } + } + if held { + g.Hold = nil + h.keepGroup(g) + } + for key := range h.unmergeableSince { + if g != nil && strings.HasPrefix(key, g.ID+" ") { + delete(h.unmergeableSince, key) + } + } + h.Logf("[mesh-delivery] %s: its merge is asked again on the operator's word, at the heads approved: %s", id, why) + return fmt.Sprintf("%s's merge is asked again at the next look, every member read first; nothing merges unless every member merges now", id), nil +} + +// readyWords is what an approved delivery's run says it waits for, if anything. +func (d *Delivery) readyWords() string { + if d.Ready == nil { + return "" + } + return d.Ready.Waits +} 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 71f10581..06b56023 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/holder.go @@ -79,6 +79,9 @@ type Holder struct { // (onestatus.go, issue 1302); runningSince when this owner first saw each running (ADR 0548 §2, turns.go). checker map[string]checkerVerdict runningSince map[string]time.Time + // unmergeableSince is when the forge first said a group's member does not merge now, by group and member (issue 1275, + // group_merge.go): within forgeMergeGrace that is the forge still working out mergeability, asked again shortly. + unmergeableSince map[string]time.Time // Turns is the gate's bucket turns (turns.go, ADR 0548 §3); nil keeps none. turnsHeld are the turns as written there, // by repository, read once at the first look (turnsLoaded). Turns TurnStore diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_whole_test.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_whole_test.go new file mode 100644 index 00000000..8271fdef --- /dev/null +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/issue1275_whole_test.go @@ -0,0 +1,347 @@ +package main + +import ( + "errors" + "fmt" + "sort" + "strings" + "testing" + + sdk "git.novox.be/novox/mesh-sdk/go/delivery" +) + +// Issue 1275, met a third time (2026-10-26 13:40, group fix/734-lost-ask): the operator approved mesh-sdk#92, +// mesh-controller#472 and mesh-delivery#73; the first two merged, the forge answered #73's merge 405 "Please try again +// later", and in the seconds between, group feat/890-placement merged mesh-delivery#72, so #73 then conflicted. A group is +// merged only when every member merges now, and no other approved merge lands in a repository between its members. + +const ( + wSDK, wCtl, wDel = "novox/mesh-sdk", "novox/mesh-controller", "novox/mesh-delivery" + wH92, wH472, wH73 = "3203358c4cb5fa5bed223f7ce85577bbb70a346e", "ccc7ba334c82554d809b5b38d8eb0eb5f36be7be", "bf574172154a9f6ed7d4e790643e0dcab2a10a91" + wH72, wH73b = "72727272727272727272727272727272727272aa", "b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0" + forgeLater = "Gitea API /repos/novox/mesh-delivery/pulls/73/merge: 405 {\"message\":\"Please try again later\"}" + conflictWords = "does not merge into main now" + headMovedWords = "its head is now" +) + +var lostAsk = []member{ + {wSDK, 92, wH92, "go/buildstep/ask.go"}, + {wCtl, 472, wH472, "internal/grants.go"}, + {wDel, 73, wH73, "modules/mesh-delivery-checker/internal/check/lostask.go,modules/mesh-delivery-checker/go.mod"}, +} + +// forgePull says what the forge answers of a pull request: its head and whether it merges now. +func forgePull(w *world, repository string, number int, head string, mergeable bool) { + w.forge.mu.Lock() + defer w.forge.mu.Unlock() + m := mergeable + w.forge.pulls[fmt.Sprintf("%s#%d", repository, number)] = ForgePull{Number: number, State: "open", Base: "main", HeadSHA: head, Mergeable: &m} +} + +func TestAnApprovedGroupMergesNoMemberUnlessEveryMemberMergesNow(t *testing.T) { + approve := func(t *testing.T) *world { + w := approvedGroupOf(t, lostAsk...) + allGreenFor(w, lostAsk...) + for _, m := range lostAsk { + forgePull(w, m.repo, m.number, m.head, true) + } + return w + } + cases := []struct { + name string + // before is the forge as the Approve lands; after, once the grace has passed (nil: as before). + before, after func(w *world) + // merged is how many members merge in the end; line what the urgent line says, when one stands. + merged int + line string + }{ + {name: "a member in conflict with its trunk: none merged, the group held and said urgent", merged: 0, line: conflictWords, + before: func(w *world) { + forgePull(w, wDel, 73, wH73, false) + w.forge.refuseRepo = map[string]error{wDel: errors.New(forgeLater)} // what the merge tool would answer + }}, + {name: "the forge still weighing a member: none merged, then every member once it says yes", merged: 3, + before: func(w *world) { forgePull(w, wDel, 73, wH73, false) }, + after: func(w *world) { forgePull(w, wDel, 73, wH73, true) }}, + {name: "a member's head moved after the Approve: none merged, said urgent", merged: 0, line: headMovedWords, + before: func(w *world) { forgePull(w, wDel, 73, wH73b, true) }}, + {name: "every member merges now: all merged in one pass", merged: 3, before: func(*world) {}}, + } + // The recovery of a group that ended half merged: its remaining member's new head, or the operator's word. + t.Run("a half-merged group is made whole by its remaining member's new head", halfMergedMadeWholeByANewHead) + t.Run("a member refused part way is asked again on the operator's word", refusedPartWayAskedAgainOnTheOperatorsWord) + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + w := approve(t) + c.before(w) + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if c.after == nil && c.merged == 3 { + if len(w.forge.merges) != 3 { + t.Fatalf("merged %v, wanted all three", w.forge.merges) + } + return + } + if len(w.forge.merges) != 0 { + t.Fatalf("a member was merged while another does not merge now: %v", w.forge.merges) + } + if c.after != nil { + c.after(w) + } + w.later(forgeMergeGrace + time30s) + w.settleAll() + if len(w.forge.merges) != c.merged { + t.Fatalf("merged %v, wanted %d", w.forge.merges, c.merged) + } + lines := urgentLines(w) + if c.line == "" { + if len(lines) != 0 { + t.Fatalf("a group merged whole still says %+v", lines) + } + return + } + if len(lines) != 1 || !strings.Contains(lines[0].Says, "feat/g") || + !strings.Contains(lines[0].Says, IDOf(wDel, wH73)) || !strings.Contains(lines[0].Says, c.line) || + !strings.Contains(lines[0].Says, "nothing is merged") { + t.Fatalf("the held group is not one urgent line naming the member and why: %+v", lines) + } + }) + } +} + +// handing are the pull requests the gate has handed the forge bridge and the bridge has not answered, in every merges key. +func handing(w *world) []sdk.MergePull { + w.merges.mu.Lock() + defer w.merges.mu.Unlock() + var out []sdk.MergePull + for _, v := range w.merges.values { + for _, p := range v.Pulls { + if p.State == sdk.MergeMerging { + out = append(out, p) + } + } + } + sort.Slice(out, func(i, j int) bool { return out[i].Repository < out[j].Repository }) + return out +} + +func handedWords(ps []sdk.MergePull) string { + var out []string + for _, p := range ps { + out = append(out, fmt.Sprintf("%s#%d", p.Repository, p.Number)) + } + return strings.Join(out, " ") +} + +// Two approved merges into one repository: the one whose pass started has the repository until every member merged, and +// the other is handed only then (2026-10-26: mesh-delivery#72 of feat/890-placement merged between fix/734-lost-ask's +// members). This holds while mesh/check gates no merge too (ADR 0551): it waits for a merge, never for a check. +func TestTwoApprovedMergesIntoOneRepositoryDoNotInterleave(t *testing.T) { + for _, gates := range []bool{true, false} { + t.Run(fmt.Sprintf("check-gates-merge %v", gates), func(t *testing.T) { + w := approvedGroupOf(t, lostAsk...) + if !gates { + ungated(w) + } + w.merges.manual = true + allGreenFor(w, lostAsk...) + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if got := handedWords(handing(w)); got != "novox/mesh-sdk#92" { + t.Fatalf("handed %q, wanted the group's first member", got) + } + // feat/890-placement: a lone pull request into mesh-delivery, approved while the group's pass is under way. + w.h.PullUpdated(pr(wDel, 72, wH72, "feat/890-placement", "modules/mesh-delivery-checker/go.mod")) + w.h.Checked(judged(wDel, 72, wH72, "pass", gTrunk0)) + setChecks(w, wDel, wH72, "success", "success") + w.settleAll() + if open := w.op.open(); len(open) != 1 { + t.Fatalf("the lone pull request is asked once: %v", open) + } else { + w.op.answer(open[0], OptionApprove) + } + w.settleAll() + var order []string + steps := []struct { + repo string + number int + head string + }{{wSDK, 92, wH92}, {wCtl, 472, wH472}, {wDel, 73, wH73}} + if !gates { + // While the check gates the merge, #72's verdict went out of date with #73's merge and it waits for its + // check against the new trunk; while it gates none, #72 is handed once the group's pass ended. + steps = append(steps, struct { + repo string + number int + head string + }{wDel, 72, wH72}) + } + for _, next := range steps { + handed := handing(w) + if len(handed) != 1 || handed[0].Repository != next.repo || handed[0].Number != next.number { + t.Fatalf("after %v: handed %q, wanted only %s#%d", order, handedWords(handed), next.repo, next.number) + } + merge := strings.Repeat(string(rune('a'+next.number%20)), 40) + bridgeMerged(w, next.repo, next.number, next.head, merge) + w.h.PullMerged(merged(next.repo, next.number, next.head, merge)) + w.settleAll() + order = append(order, fmt.Sprintf("%s#%d", next.repo, next.number)) + } + if left := handing(w); len(left) != 0 { + t.Fatalf("still handed: %q", handedWords(left)) + } + }) + } +} + +// A group that ended half merged (its last member refused after the others merged) is made whole by its remaining member's +// new head: rebased on its trunk, checked, composed with the merged members, asked once and merged, and the urgent line +// is cleared (issue 1275, the recovery of 2026-10-26 13:40). +func halfMergedMadeWholeByANewHead(t *testing.T) { + w := approvedGroupOf(t, lostAsk...) + allGreenFor(w, lostAsk...) + w.forge.refuseRepo = map[string]error{wDel: errors.New(forgeLater)} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 3 || len(urgentLines(w)) != 1 { + t.Fatalf("the case: two merged and the third refused, said urgent: %v %+v", w.forge.merges, urgentLines(w)) + } + w.forge.refuseRepo = nil + // #73 rebased on its trunk: a new head, checked, composed with the members merged. + w.h.PullUpdated(pr(wDel, 73, wH73b, "feat/g", "modules/mesh-delivery-checker/go.mod")) + w.h.Checked(judged(wDel, 73, wH73b, "pass", gT1)) + setChecks(w, wDel, wH73b, "success", "success") + w.settleAll() + open := w.op.open() + if len(open) != 1 { + t.Fatalf("the remaining member's new head is asked once: %v", open) + } + w.op.answer(open[0], OptionApprove) + w.settleAll() + if last := w.forge.merges[len(w.forge.merges)-1]; len(w.forge.merges) != 4 || !strings.Contains(last, "mesh-delivery#73@"+wH73b) { + t.Fatalf("the new head is not merged alone: %v", w.forge.merges) + } + if lines := urgentLines(w); len(lines) != 0 { + t.Fatalf("the group is whole and still said: %+v", lines) + } +} + +// delivery_merge_again (issue 1275, the tools rule): a member refused after its siblings merged, with no new head and no +// trunk move to ask it again, is asked again on the operator's word, at the head approved; a delivery neither refused nor +// held is refused, naming why. +func refusedPartWayAskedAgainOnTheOperatorsWord(t *testing.T) { + w := approvedGroupOf(t, lostAsk...) + allGreenFor(w, lostAsk...) + if _, err := w.h.MergeAgain(IDOf(wDel, wH73), "too early"); err == nil { + t.Fatal("a delivery not approved is asked again") + } + w.forge.refuseRepo = map[string]error{wDel: errors.New(forgeLater)} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.forge.refuseRepo = nil + w.settleAll() + if len(w.forge.merges) != 3 { + t.Fatalf("the case: the third member refused and not asked again by itself: %v", w.forge.merges) + } + if _, err := w.h.MergeAgain(IDOf(wDel, wH73), ""); err == nil { + t.Fatal("asked again with no why") + } + if _, err := w.h.MergeAgain(IDOf(wDel, wH73), "the forge said try again later"); err != nil { + t.Fatal(err) + } + w.settleAll() + if len(w.forge.merges) != 4 || len(urgentLines(w)) != 0 { + t.Fatalf("not merged on the operator's word: %v %+v", w.forge.merges, urgentLines(w)) + } + if _, err := w.h.MergeAgain(IDOf(wDel, wH73), "again"); err == nil || !strings.Contains(err.Error(), "merged already") { + t.Fatalf("a merged delivery is asked again: %v", err) + } +} + +// approveSecondGroup makes a second group of these members on branch, checked, composed together and approved; the world +// already holds group feat/g. +func approveSecondGroup(t *testing.T, w *world, branch string, ms ...member) { + t.Helper() + for _, m := range ms { + w.h.PullUpdated(pr(m.repo, m.number, m.head, branch, strings.Split(m.paths, ",")...)) + w.h.Checked(judged(m.repo, m.number, m.head, "pass", gTrunk0)) + } + allGreenFor(w, ms...) + w.settleAll() + g := w.h.groups[branch] + if g == nil || g.Check == nil { + t.Fatalf("no composed check was asked for %s: %+v", branch, g) + } + c := CheckedEvent{Group: branch, ID: g.Check.Asked, Verdict: "pass", Summary: "composed"} + for _, m := range ms { + owner, repo, _ := strings.Cut(m.repo, "/") + c.Members = append(c.Members, struct { + Owner string `json:"owner"` + Repo string `json:"repo"` + Commit string `json:"commit"` + Trunk string `json:"trunk,omitempty"` + }{Owner: owner, Repo: repo, Commit: m.head, Trunk: gTrunk0[:12]}) + } + w.h.Checked(c) + w.settleAll() + for _, key := range w.op.open() { + if strings.Contains(key, strings.ReplaceAll(branch, "/", "_")) || strings.Contains(key, branch) { + w.op.answer(key, OptionApprove) + } + } +} + +// A group held half merged keeps its repositories until it is whole or declined: another group sharing one waits, and the +// operator's word that asks the held group again never leaves the two waiting on each other (issue 1275, review round 1: +// a2 waited for group B, b2 waited for group A, for ever). And the operator's word is refused while another pass it would +// wait for is under way, naming it. +func TestAHeldHalfMergedGroupKeepsItsRepositoriesAndNoTwoPassesWaitOnEachOther(t *testing.T) { + groupA := []member{lostAsk[0], lostAsk[2]} // mesh-sdk#92, then mesh-delivery#73 + groupB := []member{lostAsk[1], {wDel, 72, wH72, "modules/mesh-delivery-checker/go.mod"}} + t.Run("half merged and held: the other group waits, and both merge once the held one is asked again", func(t *testing.T) { + w := approvedGroupOf(t, groupA...) + allGreenFor(w, groupA...) + w.forge.refuseRepo = map[string]error{wDel: errors.New(forgeLater)} + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + if len(w.forge.merges) != 2 { + t.Fatalf("the case: mesh-sdk#92 merged, mesh-delivery#73 refused: %v", w.forge.merges) + } + approveSecondGroup(t, w, "feat/890-placement", groupB...) + w.settleAll() + if len(w.forge.merges) != 2 { + t.Fatalf("group feat/890-placement merged while feat/g is half merged into a repository both go into: %v", w.forge.merges) + } + w.forge.refuseRepo = nil + if _, err := w.h.MergeAgain(IDOf(wDel, wH73), "the forge said try again later"); err != nil { + t.Fatal(err) + } + w.settleAll() + want := []string{"mesh-sdk#92", "mesh-delivery#73", "mesh-delivery#73", "mesh-controller#472", "mesh-delivery#72"} + if len(w.forge.merges) != len(want) { + t.Fatalf("merged %v, wanted %v: the held group first, then the other", w.forge.merges, want) + } + for i, m := range want { + if !strings.Contains(w.forge.merges[i], m+"@") { + t.Fatalf("merged %v, wanted %v in that order", w.forge.merges, want) + } + } + }) + t.Run("the operator's word is refused while another pass it would wait for is under way", func(t *testing.T) { + w := approvedGroupOf(t, groupA...) + allGreenFor(w, groupA...) + w.forge.refuseRepo = map[string]error{wSDK: errors.New(forgeLater)} // refused first: nothing of feat/g merged + w.op.answer(w.op.open()[0], OptionApprove) + w.settleAll() + w.merges.manual = true + approveSecondGroup(t, w, "feat/890-placement", groupB...) + w.settleAll() + if got := handedWords(handing(w)); got != "novox/mesh-controller#472" { + t.Fatalf("the other group's pass did not start: handed %q", got) + } + _, err := w.h.MergeAgain(IDOf(wSDK, wH92), "asked again") + if err == nil || !strings.Contains(err.Error(), "feat/890-placement") { + t.Fatalf("asked again while a pass it waits for is under way: %v", err) + } + }) +} diff --git a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go index e5fe73af..58ff5b46 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/main.go @@ -376,6 +376,20 @@ func tools(h *Holder, l *listening) []stdio.Tool { } return h.HandNow(strArg(a, "id"), strArg(a, "why")) }}, + {Name: "delivery_merge_again", + Description: "On the operator's word, ask again at once the merge of an approved delivery the forge refused, or of an approved " + + "group held or half merged, at the heads the Approve named (novox/hq issue 1275): every member is read first and " + + "nothing merges unless every member merges now. For a refusal that was the forge's \"try again later\", a conflict " + + "cleared on the trunk, or a refusal the gate stopped asking after it repeated. A member whose head moved is asked by its " + + "new head, never by this. Refused, naming why, for a delivery that is not an approved merge waiting after a refusal or a hold.", + Input: map[string]any{"type": "object", "properties": map[string]any{"id": str("the delivery's id (owner/repository@commit), any member of the group"), + "why": str("the operator's word, kept in the log")}, "required": []string{"id", "why"}}, + Run: func(a map[string]any) (any, error) { + if err := need(a, "id", "why"); err != nil { + return nil, err + } + return h.MergeAgain(strArg(a, "id"), strArg(a, "why")) + }}, {Name: "delivery_status", Description: "Whether this owner works: how many deliveries in each state, the groups, what is owed to " + "the forge and the bus and not yet done, how many owed effects were tried since it started (done, failed " + 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 83d95a56..a18d5cab 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ports.go @@ -48,6 +48,9 @@ type ForgePull struct { // 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"` + // Mergeable is the forge's own word on whether it merges into its base now (gitea's mergeable): false while it is in + // conflict with its base, or while the forge works that out after the base moved; nil when the forge did not say. + Mergeable *bool `json:"mergeable,omitempty"` } // ForgeStatus is one status of a commit, the newest of its context. 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 0fba9e14..cf2d5e5b 100644 --- a/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go +++ b/modules/mesh-delivery-gate/cmd/mesh-delivery-gate/ready.go @@ -254,6 +254,9 @@ func (h *Holder) readyAsks(now time.Time) []func() { if waits == "" && gates { waits = h.turnWaits(d, now) // nothing merges into a repository while a check of other heads judges it (ADR 0548 §2) } + if waits == "" { + waits = h.passWaits(d) // no merge lands in a repository between another approved group's members (issue 1275) + } if waits != r.Waits { r.Waits = waits h.keep(d) diff --git a/modules/mesh-delivery-gate/module.json b/modules/mesh-delivery-gate/module.json index 4a4e32f2..afe42fc1 100644 --- a/modules/mesh-delivery-gate/module.json +++ b/modules/mesh-delivery-gate/module.json @@ -73,6 +73,7 @@ ], "tools": [ "delivery_hand_now", + "delivery_merge_again", "delivery_status", "delivery_turns" ],