Merge an approved group only when every member merges now, and let no other merge land between its members (issue 1275)
On 2026-10-26 13:40 group fix/734-lost-ask merged two of its three members: the forge answered mesh-delivery#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. - mesh-delivery-gate: before any member is handed, every member is read for its required statuses, its head still the approved one, and the forge's own mergeable (in conflict, or still being worked out); the forge's "no" is asked again every 5 s for 2 minutes, then the group is held and said urgent with nothing merged. - mesh-delivery-gate: an approved merge waits while another approved merge's pass is under way into a repository it goes into (handed until merged whole, or stopped), whether or not mesh/check gates merges (it waits for a merge, never for a check: ADR 0551). - mesh-delivery-gate: tool delivery_merge_again asks a refused or held merge again on the operator's word. - gitea: the merge asks again after the forge's 405 "Please try again later" (1 s, 2 s, 4 s), and a pull request carries the forge's mergeable, which gitea_commit_statuses answers.
This commit is contained in:
+32
-5
@@ -98,6 +98,10 @@ export interface GiteaPull {
|
||||
html_url: string;
|
||||
/** The description: where a pull request says it goes after another (`after: <repository>`, 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<void> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -1220,3 +1240,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);
|
||||
}
|
||||
|
||||
@@ -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<void>((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();
|
||||
}
|
||||
}
|
||||
});
|
||||
@@ -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`);
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -28,26 +28,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, or still working that 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 +81,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 +96,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 +108,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 +141,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 +161,67 @@ 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, or it stopped (a member refused, or the group held). 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 of seconds, 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 _, am := range h.approvedMerges() {
|
||||
if am.run.AskID == c.AskID || !h.passUnderWay(am.run) {
|
||||
continue
|
||||
}
|
||||
for repo := range h.unmergedRepos(am.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)", firstOf(am.group, am.owner.ID), repo)
|
||||
}
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// unmergedRepos are the repositories (lowercase) of the deliveries an Approve covers that are not merged. Under the lock.
|
||||
func (h *Holder) unmergedRepos(r *ReadyRun) map[string]bool {
|
||||
out := map[string]bool{}
|
||||
for id := range r.Heads {
|
||||
if m := h.deliveries[id]; m != nil && m.MergedAs == "" && (m.Ready == nil || m.Ready.Merged == nil) {
|
||||
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 the rest neither refused nor held. Under the lock.
|
||||
func (h *Holder) passUnderWay(r *ReadyRun) bool {
|
||||
merged, stopped := false, false
|
||||
for id := range r.Heads {
|
||||
m := h.deliveries[id]
|
||||
if m == nil {
|
||||
continue
|
||||
}
|
||||
switch {
|
||||
case m.MergedAs != "" || (m.Ready != nil && m.Ready.Merged != nil):
|
||||
merged = true
|
||||
case m.Ready != nil && m.Ready.merging:
|
||||
return true
|
||||
case m.Ready != nil && m.Ready.RefusedSince != nil:
|
||||
stopped = true
|
||||
}
|
||||
if g := h.groups[m.Group]; m.Group != "" && g != nil && g.Hold != nil && g.Hold.Kind == "blocked" {
|
||||
stopped = true
|
||||
}
|
||||
}
|
||||
return merged && !stopped
|
||||
}
|
||||
|
||||
// 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 +257,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 +286,66 @@ 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"))
|
||||
}
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1,259 @@
|
||||
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)
|
||||
}
|
||||
}
|
||||
@@ -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 " +
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -73,6 +73,7 @@
|
||||
],
|
||||
"tools": [
|
||||
"delivery_hand_now",
|
||||
"delivery_merge_again",
|
||||
"delivery_status",
|
||||
"delivery_turns"
|
||||
],
|
||||
|
||||
Reference in New Issue
Block a user