novox/mesh-catalog
13
0
Fork 0
Code Issues Pull Requests 3 Actions Packages Projects Releases Wiki Activity

Merge pull request 'A group approved together merges in one pass and goes out in one delivery plan (design 47 milestone 3 fix: issues 1049, 1082, 1047, 1083, 1079)' (#356) from feat/group-one-pass into main

Browse Source
This commit was merged in pull request #356.
This commit is contained in:
mesh-tools
2026-10-17 23:44:21 +00:00
parent 0c56f60305 7383bc3170
commit 44e7002d3c
16 changed files with 759 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
modules/gitea/index.ts
+12 -1
View File
@@ -46,7 +46,7 @@ import { CHECK_CONTEXT, REPO_CHECK_CONTEXT, commentFor, composedGate, groupGateS
repoCheckFacts, reposToLook } from "./pulls.js";
import { GateWriter, clonedAs, type BuildStarted } from "./gate.js";
import { HOOK_PATH, HOOK_PORT, Pace, claimHead, ensureSystemHook, eventsTool, keptSecret, receiver, type ForgeEvent } from "./webhook.js";
import { keepSiblings, noteVerdict, readSiblings, siblingsFile, siblingsOf, weighSiblings } from "./siblings.js";
import { keepSiblings, noteComposed, noteVerdict, readSiblings, siblingsFile, siblingsOf, weighSiblings } from "./siblings.js";
import { announcementOf, keepTook, lateVerdictComment, outOfDateWords, sameCommit, tookFile, weighTrunks } from "./trunk.js";
// Without a way to a token — configured, or mintable with the admin account (token.ts) — there is
@@ -423,6 +423,9 @@ async function setVerdict(client: GiteaClient, event: { body: unknown }): Promis
* writer (gate.ts), judged against the trunk as a member's own verdict is: one that names a trunk commit that is no longer
* the trunk's head is out of date when it lands, and said so, pending. */
async function setGroupVerdict(client: GiteaClient, c: Checked): Promise<void> {
// The base branch each member merges into, read with its pull request below: the composed verdict names the siblings of
// each member from it (issue 1049).
const bases = new Map<string, string>();
for (const m of c.members ?? []) {
if (!m.owner || !m.repo || !m.commit || !c.verdict) {
console.error(`[gitea] a composed verdict of ${c.group} names a member with no repository, commit or verdict; ignored`);
@@ -431,6 +434,7 @@ async function setGroupVerdict(client: GiteaClient, c: Checked): Promise<void> {
await gates.verdict(m.owner, m.repo, m.commit, async () => {
const pull = m.number ? await client.getPullRequest(m.owner, m.repo, m.number).catch(() => undefined) : undefined;
const target = pull?.html_url || undefined;
if (m.number && pull?.base) bases.set(`${m.owner}/${m.repo}#${m.number}`, pull.base);
let status = groupGateStatus(c, m);
if (m.trunk && pull?.base) {
const head = await client.branchHead(m.owner, m.repo, pull.base).catch(() => undefined);
@@ -441,6 +445,13 @@ async function setGroupVerdict(client: GiteaClient, c: Checked): Promise<void> {
`its merge gate ${status.state}`);
}, c.id);
}
// The siblings of each member are the other members at the heads and base commits this verdict composed (issue 1049): a
// sibling's own merge of such a head is no move of the member's base. Kept for what the merge tool and the siblings'
// weighing read; nothing is run.
if (c.verdict && c.members?.length) {
const members = c.members.map((m) => ({ ...m, base: m.number ? bases.get(`${m.owner}/${m.repo}#${m.number}`) : undefined }));
if (noteComposed(siblingNamed, members)) keepSiblings(siblingsRecord, siblingNamed);
}
}
// **This module is the one writer of a head's mesh/merge-gate** (novox/hq issue 446): the announcement's pending words,
modules/gitea/siblings.ts
+31
View File
@@ -130,6 +130,37 @@ export function noteVerdict(record: SiblingRecord, key: string, head: string, si
return true;
}
/** One member of a composed verdict, as the group's composed check read it: its pull request, the head it judged, the
* trunk commit that head was merged onto, and the branch the pull request merges into. */
export interface ComposedMember {
owner: string;
repo: string;
number?: number;
commit: string;
trunk?: string;
base?: string;
}
/**
* **A group's composed verdict names, for each member, the other members at the heads and base commits it composed**
* (novox/hq ADR 0403 §5, issue 1049): the composed check read every member's head together, so it is the verdict to trust
* for the siblings. Without this, the record held what a member's OWN verdict named, which went stale when a sibling got
* a new head; the sibling's own merge of the head the composed check judged then read as another head's merge, put the
* member's gate back to pending, and the group merged in two passes. A member the verdict cannot name (no pull request
* number, trunk commit or base branch), or a verdict of fewer than two such members, changes nothing. Answers whether
* the record changed.
*/
export function noteComposed(record: SiblingRecord, members: ComposedMember[]): boolean {
const named = members.filter((m) => m.owner && m.repo && m.number && m.commit && m.trunk && m.base);
if (named.length < 2 || named.length !== members.length) return false;
for (const m of named) {
const siblings = named.filter((o) => o !== m).map((o): SiblingNamed => ({
repository: `${o.owner}/${o.repo}`, number: o.number!, head: o.commit, base: o.base!, base_commit: o.trunk! }));
record[memberKey(m.owner, m.repo, m.number!)] = { head: m.commit, siblings };
}
return true;
}
// --- what a sibling's move is ---------------------------------------------------------------------------------
/** What weighing a sibling reads of the forge. */
modules/gitea/test/guard.test.ts
+50
View File
@@ -630,3 +630,53 @@ test("gitea_merge_pull_request refuses a head the Approve did not name", async (
assert.equal(state.merged, false);
assert.equal(((await merge(["novox/mesh-catalog@" + REVIEWED.toUpperCase()])) as Record<string, unknown>).merged, true);
});
// **A group approved together merges in one pass, on the real merge tool** (novox/hq ADR 0418, issue 1049): the member's
// own verdict named its sibling at an old head; the sibling got a new head, the group's composed verdict passed for both,
// the sibling merged that head, and the member then merges on its first ask: the sibling's own merge is no move of its
// base. The records are the module's own (siblings.ts), the weighing is what the sibling's merge announcement does.
test("a group of two merges in one pass: the sibling's merge of the composed head does not stop the member's merge (issue 1049)", async () => {
const { keepSiblings, siblingsFile, noteComposed, weighSiblings } = await import("../dist/siblings.js");
const both = ["novox/mesh-catalog@" + REVIEWED, "novox/mesh-host@" + SIBLING_HEAD];
const OLD_SIBLING = "9".repeat(40), HOST_BASE = "c".repeat(40), HOST_MERGE = "f".repeat(40);
const beside = `head ${REVIEWED.slice(0, 12)} merged into trunk ${TRUNK.slice(0, 12)} beside mesh-host#114@${OLD_SIBLING.slice(0, 8)}`;
state.statuses = [{ context: "mesh/merge-gate", state: "success",
description: `head ${REVIEWED.slice(0, 12)} merged into trunk ${TRUNK.slice(0, 12)} \u2014 pass: group g composed: ok` },
{ context: "mesh/repo-check", state: "success", description: `${beside} \u2014 pass: y` }];
state.groupStatus = "success";
state.siblingGroup = "success";
// The member's own verdict named the sibling at its old head.
const record: Record<string, any> = { "novox/mesh-catalog#7": { head: REVIEWED,
siblings: [{ repository: "novox/mesh-host", number: 114, head: OLD_SIBLING, base: "main", base_commit: HOST_BASE }] } };
// The composed verdict then judged both new heads.
noteComposed(record, [{ owner: "novox", repo: "mesh-catalog", number: 7, commit: REVIEWED, trunk: TRUNK.slice(0, 12), base: "main" },
{ owner: "novox", repo: "mesh-host", number: 114, commit: SIBLING_HEAD, trunk: HOST_BASE.slice(0, 12), base: "main" }]);
// The sibling merges its head; its announcement weighs the member's record. The member's gate is written only by what
// that weighing decides, through the writer.
const pulled = { number: 114, title: "", html_url: "", merged: true, state: "closed", head_sha: SIBLING_HEAD, merge_commit_sha: HOST_MERGE };
const forge = {
async getPullRequest(_o: string, repo: string, n: number) {
return repo === "mesh-host" ? pulled : { number: n, title: "", html_url: "", merged: false, state: "open", head_sha: REVIEWED };
},
async branchHead() { return HOST_MERGE; },
async commitParents() { return [HOST_BASE, SIBLING_HEAD]; },
};
const wrote: string[] = [];
const gatesOut = { async outOfDate(_o: string, _r: string, sha: string, decide: (c: any) => any[]) {
const put = decide({ "mesh/merge-gate": { state: "success", description: state.statuses[0].description },
"mesh/repo-check": { state: "success", description: "x" } });
for (const s of put) {
wrote.push(`${sha.slice(0, 12)} ${s.context} ${s.state}`);
state.statuses = [{ ...s }, ...state.statuses.filter((x) => x.context !== s.context)];
}
return put;
} };
await weighSiblings(forge as any, gatesOut as any, record, "novox/mesh-host", 114);
assert.deepEqual(wrote, [], "the member's statuses were left alone");
keepSiblings(siblingsFile(stateDir), record);
// The member merges on its first ask, with the one Approve's heads.
const done = (await tool("gitea_merge_pull_request").run({ owner: "novox", repo: "mesh-catalog", number: 7, head: REVIEWED,
approved_heads: both })) as Record<string, unknown>;
assert.equal(done.merged, true);
assert.equal(state.emitted.length, 0, "no check was asked");
});
modules/gitea/test/siblings.test.ts
+55
View File
@@ -268,3 +268,58 @@ test("a merged or closed member's entry is dropped, so a sibling's later push ne
assert.deepEqual(mine, {});
assert.deepEqual(own.written, []);
});
// --- the composed verdict names the siblings of each member (issue 1049) --------------------------------------
// Issue 1049: the 2026-10-17 23:48 group (mesh-controller #375 beside mesh-catalog #353). #375's own verdict named #353
// at its old head; #353 then got a new head, and the group's composed verdict passed for both new heads. The record still
// held the old head, so #353's own merge of the NEW head read as "merged otherwise than its head" and put #375's gate back
// to pending, and the group merged in two passes.
const composedMembers = () => [
{ owner: "novox", repo: "mesh-controller", number: 346, commit: MEMBER_HEAD, trunk: "111111111111", base: "main" },
{ owner: "novox", repo: "mesh-host", number: 114, commit: HOST_NEW, trunk: HOST_BASE.slice(0, 12), base: "main" },
];
const staleRecord = (): SiblingRecord => ({ "novox/mesh-controller#346": { head: MEMBER_HEAD, siblings: siblingsOf([sibling]) } });
test("without the composed verdict's siblings, a sibling's own merge of its new head is a move (the pin of issue 1049)", async () => {
const w = world();
Object.assign(w.host, { merged: true, state: "closed", merge_commit_sha: MERGE, head_sha: HOST_NEW });
w.parents = [HOST_BASE, HOST_NEW];
await weigh(w, staleRecord());
assert.equal(w.written.length, 2, "the record names the old head: the merge reads as another head's");
});
test("a composed verdict names the siblings of each member at the heads and base commits it composed, so a sibling's own merge of its new head is no move (issue 1049)", async () => {
const { noteComposed } = await import("../dist/siblings.js");
const record = staleRecord();
assert.equal(noteComposed(record, composedMembers()), true);
assert.deepEqual(record["novox/mesh-controller#346"], { head: MEMBER_HEAD,
siblings: [{ repository: "novox/mesh-host", number: 114, head: HOST_NEW, base: "main", base_commit: HOST_BASE.slice(0, 12) }] });
assert.deepEqual(record["novox/mesh-host#114"], { head: HOST_NEW,
siblings: [{ repository: "novox/mesh-controller", number: 346, head: MEMBER_HEAD, base: "main", base_commit: "111111111111" }] });
const w = world();
Object.assign(w.host, { merged: true, state: "closed", merge_commit_sha: MERGE, head_sha: HOST_NEW });
w.parents = [HOST_BASE, HOST_NEW];
await weigh(w, record);
assert.deepEqual(w.written, [], "its own merge of the head the composed check judged leaves the member's gate alone");
assert.equal(record["novox/mesh-controller#346"].siblings[0].merged, true);
// A merge of any other head, or onto any other base, is still a move.
const other = world();
const again: SiblingRecord = {};
noteComposed(again, composedMembers());
Object.assign(other.host, { merged: true, state: "closed", merge_commit_sha: MERGE, head_sha: HOST_NEW });
other.parents = [HOST_MOVED, HOST_NEW];
await weigh(other, again);
assert.equal(other.written.length, 2);
});
test("a composed verdict keeps nothing for a member it cannot name, and a group of one is none (issue 1049)", async () => {
const { noteComposed } = await import("../dist/siblings.js");
const record: SiblingRecord = {};
const [a, b] = composedMembers();
assert.equal(noteComposed(record, [a]), false);
assert.equal(noteComposed(record, [a, { ...b, number: undefined }]), false, "a member with no pull request number");
assert.equal(noteComposed(record, [a, { ...b, trunk: undefined }]), false, "a member with no trunk commit");
assert.equal(noteComposed(record, [a, { ...b, base: undefined }]), false, "a member with no base branch");
assert.deepEqual(record, {});
});
modules/gitea/test/webhook.test.ts
+17
View File
@@ -333,3 +333,20 @@ test("the polls look every 30 s until events are heard, every five minutes while
pace.heard();
assert.equal(pace.every(), 300_000, "the next delivery");
});
// Issue 1079: a pull request's edit event carries its description, so mesh-delivery reads a Reviewed-by line written after
// the head was announced. Other pull request events do not carry it (they are many, and the description is long).
test("a pull request's edit event carries its description, and no other pull request event does (issue 1079)", async () => {
const { eventOf } = await import("../webhook.ts");
const pull = { number: 3, title: "t", body: "Reviewed-by: kb@g14 at " + "a".repeat(40), head: { ref: "feat/x", sha: "a".repeat(40) },
base: { ref: "main" } };
const repository = { name: "hq", owner: { login: "novox" } };
const edited = eventOf("pull_request", { action: "edited", pull_request: pull, repository });
assert.equal(edited.name, "forge.pull-request.edited");
assert.equal(edited.body.body, pull.body);
assert.equal(edited.body.head_sha, "a".repeat(40));
assert.equal(edited.body.number, 3);
for (const action of ["opened", "synchronized", "reopened"]) {
assert.equal(eventOf("pull_request", { action, pull_request: pull, repository }).body.body, undefined, action);
}
});
modules/gitea/webhook.ts
+3 -1
View File
@@ -112,7 +112,9 @@ function forgeEventOf(said: string, payload: any): ForgeEvent {
base: str(pull.base?.ref), merged: !!pull.merged, merge_commit_sha: str(pull.merge_commit_sha),
merged_at: str(pull.merged_at), html_url: str(pull.html_url),
labels: Array.isArray(pull.labels) ? pull.labels.map((l: any) => str(l?.name)).filter(Boolean) : undefined,
label: str(payload.label?.name) } };
label: str(payload.label?.name),
// An edit carries the description (issue 1079): mesh-delivery reads the review record in it again.
...(action === "edited" ? { body: str(pull.body) } : {}) } };
}
if (kind === "issue_comment") {
const issue = payload.issue ?? {};
modules/mesh-delivery/cmd/mesh-delivery/approval.go
+21
View File
@@ -380,6 +380,27 @@ func (h *Holder) dropAsk(id, why string) func() {
}
}
// withdrawAsk takes back an ask nobody answered, on every delivery it covers, so the next look asks again with what is
// known now (issue 1079). Unlike dropAsk it ends no Approve and says nothing on a pull request: there was none. Under the
// lock; the router's cancel runs with the look's asks.
func (h *Holder) withdrawAsk(id, why string) func() {
for _, d := range h.deliveries {
if d.Ready != nil && d.Ready.AskID == id && d.Ready.Approved == nil {
d.Ready.AskID, d.Ready.Ask, d.Ready.Heads = "", nil, nil
h.keep(d)
}
}
h.Logf("[mesh-delivery] the operator's ask %s is withdrawn to be asked again: %s", id, why)
return func() {
if h.Operator == nil {
return
}
if err := h.Operator.Cancel(id); err != nil {
h.Logf("[mesh-delivery] the operator's ask %s could not be withdrawn (%v)", id, err)
}
}
}
// pendingAsks are the asks waiting for an answer, by id; under the lock.
func (h *Holder) pendingAsks() []string {
seen := map[string]bool{}
modules/mesh-delivery/cmd/mesh-delivery/bus_test.go
+36 -4
View File
@@ -107,7 +107,9 @@ type runtime struct {
// the asks the operator was asked, and the controller's walks and the forge's open pull requests as the test plays
// them.
operatorAsks []asks.Ask
openPulls []OpenPull
// operatorCancels are the ids of the asks taken back from the router, in order (issue 1079).
operatorCancels []string
openPulls []OpenPull
}
type askedOfMesh struct {
@@ -268,6 +270,17 @@ func (r *runtime) answer(m rpcMessage) {
r.operatorAsks = append(r.operatorAsks, a)
r.mu.Unlock()
}
if strings.Contains(p.Subject, ".accept.cancel.") {
var c struct {
ID string `json:"id"`
}
if err := json.Unmarshal(p.Body, &c); err != nil {
return nil, err
}
r.mu.Lock()
r.operatorCancels = append(r.operatorCancels, c.ID)
r.mu.Unlock()
}
return map[string]any{}, nil
case "mesh/seat.record":
return nil, nil
@@ -416,7 +429,7 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB
announced := pr("novox/app", 7, head, "fix/x", "src/a.go")
announced.BaseSHA = "t1t1t1t1t1t1"
announced.Title = "Fix the apostrophe (issue 975)"
announced.Body = "Reviewed-by: lead@shanks at " + head
announced.Body = "The description, with no review line yet."
b.publish(t, "mesh.mod.gitea.event.pull.updated", announced)
passed := b.publish(t, "mesh.seat."+ControllerSeat+".event.checked", judgedAt("novox/app", 7, head, "pass", "t1t1t1t1t1t1"))
@@ -430,10 +443,29 @@ func TestAPassingVerdictAsksTheApproveAndItsApproveMergesWithinSecondsOverARealB
ask := rt.operatorAsks[0]
asked := time.Now()
rt.mu.Unlock()
if !strings.Contains(ask.Explanation, "reviewed by lead") {
t.Fatalf("the ask does not name the reviewer: %s", ask.Explanation)
if !strings.Contains(ask.Explanation, "not reviewed at this head") {
t.Fatalf("the first ask does not say the record is missing: %s", ask.Explanation)
}
t.Logf("the operator was asked %s after the verdict's event", asked.Sub(passed))
// The description is edited to add the review record (issue 1079): the forge module's edit event carries it. The unanswered
// ask is withdrawn from the router and asked again, once, with the record read; nothing else is asked of the mesh.
first := ask
b.publish(t, "mesh.mod.gitea.event.forge.pull-request.edited", map[string]any{"owner": "novox", "repo": "app", "number": 7,
"title": announced.Title, "head_sha": head, "base": "main", "body": "Reviewed-by: lead@shanks at " + head})
rt.until("the first ask to be withdrawn and the operator asked again", 5*time.Second, func() bool {
rt.mu.Lock()
defer rt.mu.Unlock()
return len(rt.operatorCancels) >= 1 && len(rt.operatorAsks) >= 2
})
rt.mu.Lock()
if len(rt.operatorCancels) != 1 || rt.operatorCancels[0] != first.ID || len(rt.operatorAsks) != 2 {
t.Fatalf("withdrawn %v, asked %d; wanted the first ask %s withdrawn once and exactly one new ask", rt.operatorCancels, len(rt.operatorAsks), first.ID)
}
ask = rt.operatorAsks[1]
rt.mu.Unlock()
if ask.ID == first.ID || !strings.Contains(ask.Explanation, "reviewed by lead on shanks") || strings.Contains(ask.Explanation, "not reviewed") {
t.Fatalf("the new ask is not a fresh one naming the reviewer: %s %s", ask.ID, ask.Explanation)
}
if len(rt.asksOf("gitea.gitea_merge_pull_request")) != 0 {
t.Fatalf("merged before the Approve")
}
modules/mesh-delivery/cmd/mesh-delivery/fakes_test.go
+5
View File
@@ -428,6 +428,8 @@ type fakeForge struct {
merges, comments []string
approvedHeads [][]string
refuse error
// refuseRepo refuses the merges of one repository only ("owner/name"), beside refuse (issue 1049).
refuseRepo map[string]error
// calls are the comments in the order asked.
calls []string
}
@@ -711,6 +713,9 @@ func (f *fakeForge) Merge(owner, repo string, number int, head string, approved
defer f.mu.Unlock()
f.merges = append(f.merges, fmt.Sprintf("%s/%s#%d@%s", owner, repo, number, head))
f.approvedHeads = append(f.approvedHeads, approved)
if err := f.refuseRepo[owner+"/"+repo]; err != nil {
return err
}
return f.refuse
}
modules/mesh-delivery/cmd/mesh-delivery/group_test.go
+207
View File
@@ -3,6 +3,7 @@ package main
import (
"encoding/json"
"sort"
"strings"
"testing"
"time"
)
@@ -188,3 +189,209 @@ func TestAMemberWhoseGateIsTheComposedChecksCountsAsPassed(t *testing.T) {
t.Fatalf("a member whose repository check failed is ready")
}
}
// Issue 1049 (survey question 1, confirmed here): a composed check asked after one member has already merged is asked
// with ALL the members, the merged one at the head it merged, never with the unmerged one alone. The controller needs two
// or more members to compose (mesh-controller delivery.go askGroupCheck), and a merge of a contained head is a no-op
// there.
func TestAComposedCheckAskedAfterOneMemberMergedIsAskedWithAllMembers(t *testing.T) {
w := approvedGroup(t)
w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
w.h.PullMerged(merged(gCat, 2, gHk, gT1))
w.settleAll()
if g := w.h.groups["feat/g"]; g == nil || !g.Closed {
t.Fatalf("the group did not close when a member merged: %+v", g)
}
// The unmerged member gets a new head and its own verdict: the group's composed check is due again.
const gHc2 = "e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3e3"
asked := len(w.ctl.checks)
w.h.PullUpdated(pr(gCtl, 1, gHc2, "feat/g", "internal/a.go"))
w.h.Checked(judged(gCtl, 1, gHc2, "pass", gT1))
w.settleAll()
var composed []string
for _, c := range w.ctl.checks[asked:] {
if len(c) > 7 && c[:7] == "feat/g:" {
composed = append(composed, c)
}
}
if len(composed) != 1 || composed[0] != "feat/g:"+IDOf(gCat, gHk)+","+IDOf(gCtl, gHc2) {
t.Fatalf("the composed check was asked with %v; wanted once, with both members (the merged one at its head)", composed)
}
}
// Issue 1082 (the 2026-10-17 23:48 group): a closed group's member whose merge the tool refused after its sibling merged
// asks the group's composed check again at once, and merges on its pass with the one Approve (ADR 0418 §4, ADR 0421).
func TestAClosedGroupsMemberWhoseMergeWasRefusedAsksTheComposedCheckAgainAtOnce(t *testing.T) {
w := approvedGroup(t)
w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
// The catalogue member merged, the controller's was refused.
w.h.PullMerged(merged(gCat, 2, gHk, gT1))
asked := len(w.ctl.checks)
w.settleAll()
var composed []string
for _, c := range w.ctl.checks[asked:] {
if strings.HasPrefix(c, "feat/g:") {
composed = append(composed, c)
}
}
if len(composed) != 1 || composed[0] != "feat/g:"+IDOf(gCat, gHk)+","+IDOf(gCtl, gHc) {
t.Fatalf("the refusal asked the composed check %v; wanted once, at once, with both members", composed)
}
// Not again while that check is on its way.
w.later(2 * time.Minute)
w.settleAll()
if got := w.ctl.checks[asked:]; len(got) != 1 {
t.Fatalf("asked again while the composed check was on its way: %v", got)
}
// It passes; the controller's member merges on the same Approve.
w.forge.refuseRepo = nil
g := w.h.groups["feat/g"]
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"})
w.settleAll()
if d := w.delivery(IDOf(gCtl, gHc)); d.Ready.Merged == nil {
t.Fatalf("the refused member was not merged on the new composed verdict: %+v", d.Ready)
}
if w.op.count() != 1 {
t.Fatalf("a second Approve was asked: %d", w.op.count())
}
}
// Issue 1082: a refusal loop is not silent. A merge refused past ten minutes in a closed group (ADR 0418 §3) is a stalled
// line naming the member and the merge tool's last answer, which the controller raises as a condition.
func TestARefusalLoopInAClosedGroupIsSaidPastTenMinutes(t *testing.T) {
w := approvedGroup(t)
w.forge.refuseRepo = map[string]error{gCtl: errAskedAgain}
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
w.h.PullMerged(merged(gCat, 2, gHk, gT1))
w.settleAll()
// Each composed pass is followed by a refusal again: the loop.
for range 12 {
w.later(time.Minute)
g := w.h.groups["feat/g"]
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed again"})
w.settleAll()
if lines := w.h.Stalled(); len(lines) != 0 && w.now.Sub(*w.delivery(IDOf(gCtl, gHc)).Ready.RefusedSince) < 10*time.Minute {
t.Fatalf("said before its bound: %+v", lines)
}
}
lines := w.h.Stalled()
if len(lines) != 1 || lines[0].ID != IDOf(gCtl, gHc) || lines[0].Bound != "10m0s" ||
!strings.Contains(lines[0].Says, errAskedAgain.Error()) || !strings.Contains(lines[0].Says, "group feat/g") {
t.Fatalf("the loop was not said, naming the member, its group and the tool's answer: %+v", lines)
}
}
// Issue 1047 (the live case of 2026-10-17 23:34, ADR 0395 §1, design 47): a group member that is ready when a third party's
// merge moves its trunk, with no Approve asked yet (the composed check has not passed), has its own check asked at once,
// and the Approve is asked once the group's composed check has passed on it.
func TestATrunkMoveAsksTheCheckOfAGroupMemberWhoseApproveIsNotAskedYet(t *testing.T) {
w := newWorld(t)
w.op.auto = false
w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go"))
w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json"))
w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0))
w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0))
w.settleAll()
if w.op.count() != 0 {
t.Fatalf("the composed check has not passed, yet the operator was asked: %d", w.op.count())
}
asked := len(w.ctl.checks)
w.h.PullMerged(merged(gCat, 5, "5555555555555555", gT1))
w.settleAll()
got := w.ctl.checks[asked:]
if len(got) != 1 || got[0] != ":"+IDOf(gCat, gHk) {
t.Fatalf("the trunk's move asked %v; wanted the moved member's own check once", got)
}
if last := w.ctl.baseSHAs[len(w.ctl.baseSHAs)-1]; last != gT1[:12] && last != gT1 {
t.Fatalf("its check was not asked against the trunk's head: %v", w.ctl.baseSHAs)
}
// Not asked again at the next look while it is on its way.
w.later(time.Second)
w.settleAll()
if len(w.ctl.checks[asked:]) != 1 {
t.Fatalf("asked twice: %v", w.ctl.checks[asked:])
}
// The composed check that was on its way when the trunk moved answers, judged against the old trunk (ADR 0403 §6): it is
// out of date as it lands, so the group does not become ready on it and the operator is not asked.
g := w.h.groups["feat/g"]
w.later(time.Minute)
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed on the old trunk",
Members: []struct {
Owner string `json:"owner"`
Repo string `json:"repo"`
Commit string `json:"commit"`
Trunk string `json:"trunk,omitempty"`
}{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]},
{Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}})
w.settleAll()
if w.op.count() != 0 {
t.Fatalf("the operator was asked on a composed pass judged against the old trunk: %d", w.op.count())
}
// The moved member's fresh verdict arrives: the composed check is asked again, and only its pass asks the operator.
w.later(time.Minute)
composedBefore := len(w.ctl.checks)
w.h.Checked(judged(gCat, 2, gHk, "pass", gT1))
w.settleAll()
last := w.ctl.checks[len(w.ctl.checks)-1]
if len(w.ctl.checks) == composedBefore || !strings.HasPrefix(last, "feat/g:") {
t.Fatalf("the composed check was not asked again on the fresh verdict: %v", w.ctl.checks[composedBefore:])
}
if w.op.count() != 0 {
t.Fatalf("the operator was asked before the new composed check passed: %d", w.op.count())
}
g = w.h.groups["feat/g"]
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed on the new trunk",
Members: []struct {
Owner string `json:"owner"`
Repo string `json:"repo"`
Commit string `json:"commit"`
Trunk string `json:"trunk,omitempty"`
}{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]},
{Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gT1[:12]}}})
w.settleAll()
if w.op.count() != 1 {
t.Fatalf("the operator is asked once, on the new pass: %d", w.op.count())
}
}
// Issue 1047 (review round 2): the other order. The moved member's fresh verdict lands before the stale composed pass (one
// build against two): the pass is out of date as it lands, the composed check is asked again, and the operator is asked
// only after that check passes.
func TestAStaleComposedPassLandingAfterTheMovedMembersFreshVerdictIsAskedAgain(t *testing.T) {
w := newWorld(t)
w.op.auto = false
w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go"))
w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json"))
w.h.Checked(judged(gCtl, 1, gHc, "pass", gTrunk0))
w.h.Checked(judged(gCat, 2, gHk, "pass", gTrunk0))
w.settleAll()
g := w.h.groups["feat/g"]
w.later(time.Minute)
w.h.PullMerged(merged(gCat, 5, "5555555555555555", gT1))
w.settleAll()
w.later(time.Minute)
w.h.Checked(judged(gCat, 2, gHk, "pass", gT1)) // the fresh verdict first
w.settleAll()
w.later(time.Minute)
before := len(w.ctl.checks)
w.h.Checked(CheckedEvent{Group: "feat/g", ID: g.Check.Asked, Verdict: "pass", Summary: "composed on the old trunk",
Members: []struct {
Owner string `json:"owner"`
Repo string `json:"repo"`
Commit string `json:"commit"`
Trunk string `json:"trunk,omitempty"`
}{{Owner: "novox", Repo: "mesh-controller", Commit: gHc, Trunk: gTrunk0[:12]},
{Owner: "novox", Repo: "mesh-catalog", Commit: gHk, Trunk: gTrunk0[:12]}}})
w.settleAll()
if len(w.ctl.checks) == before || !strings.HasPrefix(w.ctl.checks[len(w.ctl.checks)-1], "feat/g:") {
t.Fatalf("the composed check was not asked again after the stale pass: %v", w.ctl.checks[before:])
}
if w.op.count() != 0 {
t.Fatalf("the operator was asked before the new composed check passed: %d", w.op.count())
}
}
modules/mesh-delivery/cmd/mesh-delivery/holder.go
+99 -6
View File
@@ -359,6 +359,12 @@ func (h *Holder) PullUpdated(p PullEvent) (isNew bool) {
// another pull request with the same head (novox/hq issue 783): started over below, exactly as for a first
// announcement, under the same id — never a transition out of Stopped.
}
if startedOver != "" {
// The record the new pull request takes over leaves the group it was in (issue 1083): the new one joins its own.
if old := h.deliveries[id]; old != nil && old.Group != "" {
h.leaveGroup(old)
}
}
replaced := ""
for _, old := range h.deliveries {
if old.Repository == p.repository() && old.Number == p.Number && !sameCommit(old.Commit, p.HeadSHA) &&
@@ -390,15 +396,61 @@ func (h *Holder) PullUpdated(p PullEvent) (isNew bool) {
return isNew
}
// 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
// approved, so no Approve ended. An answered ask is left as it is. The record is never a gate (ADR 0421 §5). An edit with no
// description, or of a pull request with no open delivery at that head, changes nothing.
func (h *Holder) PullEdited(p PullEvent) {
if p.HeadSHA == "" || p.Number == 0 || p.Body == "" {
return
}
h.mu.Lock()
defer h.mu.Unlock()
d := h.deliveries[IDOf(p.repository(), p.HeadSHA)]
if d == nil || d.Number != p.Number || d.State.Final() || d.MergedAs != "" {
return
}
before, _ := reviewWords(d)
d.ReviewedBy, d.ReviewedAt = ReviewIn(p.Body)
d.After = AfterIn(p.Body)
title := p.Title
if title == "" {
title = d.Title
}
d.Issues = IssuesIn(title + "\n" + p.Body)
h.keep(d)
if after, _ := reviewWords(d); after == before {
return
}
if r := h.covering(d); r != nil && r.AskID != "" && r.Approved == nil && r.Declined == nil {
h.cancels = append(h.cancels, h.withdrawAsk(r.AskID, d.ID+"'s review record changed: "+shortOf(d.ReviewedAt, 12)))
}
}
// startedOverFor is why a delivery already kept at a head's id is started over for a pull request's number rather
// than taken as announced again: only when it was stopped solely by an earlier, closed duplicate pull request with
// the same head — still Stopped, ClosedUnmerged, never merged, and under another number (novox/hq issue 783).
// Empty for the ordinary 'announced again' path, or for any other stop (a person's, a group's, its own close).
// than taken as announced again: it is Stopped, never merged, and under another number. Stopped by an earlier, closed
// duplicate pull request with the same head (novox/hq issue 783), or by a person or a group before its pull request
// closed (issue 1083: the new pull request at that commit takes the record over, the key staying repository@commit).
// Empty for the ordinary 'announced again' path, or for the same pull request's own stop.
func startedOverFor(d *Delivery, number int) string {
if d != nil && d.State == Stopped && d.ClosedUnmerged && d.MergedAs == "" && d.Number != number {
if d == nil || d.State != Stopped || d.MergedAs != "" || d.Number == number {
return ""
}
if d.ClosedUnmerged {
return fmt.Sprintf("#%d with the same head closed unmerged", d.Number)
}
return ""
said := fmt.Sprintf("#%d with the same head was stopped", d.Number)
if n := len(d.Transitions); n > 0 && d.Transitions[n-1].To == Stopped {
last := d.Transitions[n-1]
if last.By != "" {
said += " by " + last.By
}
if last.Why != "" {
said += ": " + last.Why
}
}
return said
}
// freshAnnouncement makes a delivery for p's head exactly as a first announcement would: its own id, proposed.
@@ -553,6 +605,9 @@ func (h *Holder) PullMerged(p PullEvent) {
// merge in every field but its id: FromForge, and its check the forge's word on the head, so a long-past
// merge replayed from the bus's history still falls under the history rule (issue 313) and is not held for
// a person (review round 1).
if d.Group != "" {
h.leaveGroup(d) // issue 1083: the record taken over leaves its group
}
fresh, t, err := h.freshAnnouncement(p, now, over)
if err != nil {
h.refuse(err)
@@ -616,7 +671,9 @@ func (h *Holder) PullMerged(p PullEvent) {
// trunkMoved takes a merge into a trunk as that trunk's move (novox/hq ADR 0323 §7): every other open delivery into
// it keeps the trunk's new head, and a ready one whose verdict names the commit before goes back to proposed. A
// group's composed check goes out of date when the merge is not one of its members and a member still open merges
// into that trunk (§6); the group then reads checking. Nothing is asked: a recheck or the merge tool judges again.
// into that trunk (§6); the group then reads checking. Nothing is asked here: at the same look askFresh asks the check of
// a group's member, or of a head with an Approve asked or given (issue 1047); a lone head's is judged again by a recheck
// or the merge tool (the rest of ADR 0395 §1 is issue 888).
func (h *Holder) trunkMoved(merged *Delivery, base, commit string, now time.Time) {
if base == "" || commit == "" {
return
@@ -772,6 +829,21 @@ func (h *Holder) Checked(c CheckedEvent) {
}
}
}
// A composed check that was on its way when a trunk moved lands judged against the old trunk: out of date as it lands
// (novox/hq ADR 0403 §6, issue 1047), and asked again once a member is judged again.
if g.Check.OutOfDate == "" {
for id, judged := range g.Check.Trunks {
if d := h.deliveries[id]; d != nil && d.MergedAs == "" && !d.State.Final() && d.TrunkNow != "" && !sameCommit(judged, d.TrunkNow) {
g.Check.OutOfDate = fmt.Sprintf("composed check out of date: %s was judged against trunk %s, the trunk is now %s",
id, shortOf(judged, 12), shortOf(d.TrunkNow, 12))
// From when the composed check was asked: a member judged since then, even before this verdict landed,
// counts as judged again.
g.Check.OutOfDateAt = g.Check.At
h.Logf("[mesh-delivery] group %s: %s", g.ID, g.Check.OutOfDate)
break
}
}
}
g.Updated = now
h.keepGroup(g)
h.Logf("[mesh-delivery] group %s composed: %s — %s", g.ID, c.Verdict, c.Summary)
@@ -1847,6 +1919,16 @@ func (h *Holder) planGroup(g *Group, members []*Delivery, now time.Time) []func(
}
}
again := g.Check != nil && (!sameMembers(g.Check.Commits, commits) || g.Check.OutOfDate != "" && rejudgedSince(members, g.Check.OutOfDateAt))
// A closed group's member whose merge the tool refused, after the composed check answered, is judged again at once
// (novox/hq ADR 0418 §4, issue 1082): the refusal is a sibling's merge having moved what the verdict stands on, and
// nothing else asks the composed check again. Not while a check is on its way (its verdict is empty then).
if !again && g.Closed && g.Check != nil && g.Check.Verdict != "" {
if refused := refusedSince(members, g.Check.At); refused != nil {
again = true
h.Logf("[mesh-delivery] group %s: the merge of %s was refused (%s), so its composed check is asked again", g.ID,
refused.ID, refused.Ready.Answer)
}
}
if allReady && (g.Check == nil && !g.Closed || again) {
ms := membersOfAsk(members)
id := g.ID
@@ -2426,6 +2508,17 @@ func containsString(xs []string, x string) bool {
}
// rejudgedSince is whether a member was judged again after a moment: a verdict of its own heard since.
// refusedSince is a member whose merge the tool refused, asked at or after the time given and not merged since: the first in
// the group's order, or nil.
func refusedSince(members []*Delivery, at time.Time) *Delivery {
for _, d := range members {
if r := d.Ready; r != nil && r.Merged == nil && d.MergedAs == "" && r.RefusedSince != nil && !r.LastTry.Before(at) {
return d
}
}
return nil
}
func rejudgedSince(members []*Delivery, at time.Time) bool {
for _, d := range members {
if d.Check != nil && d.Check.At.After(at) {
modules/mesh-delivery/cmd/mesh-delivery/issue1079_test.go
+101
View File
@@ -0,0 +1,101 @@
package main
import (
"strings"
"testing"
)
// novox/hq issue 1079: mesh-catalog #354's description was edited ten seconds after it was opened to add its
// `Reviewed-by` line, but the line was read only when the head was announced, so the Approve's ask said "not reviewed at
// this head". The forge's edit event is followed; an Approve ask not yet answered is withdrawn and asked again with the
// record read (the operator agreed, 2026-10-18). The review record is never a gate (ADR 0421 §5).
// askedUnreviewed is a green pull request whose Approve is asked while its description holds no Reviewed-by line.
func askedUnreviewed(t *testing.T) *world {
w := newWorld(t)
w.op.auto = false
p := pr("novox/app", 7, readyHead, "feat/x", "src/a.go")
p.Title = "Fix the apostrophe (issue 948)"
w.h.PullUpdated(p)
w.h.Checked(judgedAt("novox/app", 7, readyHead, "pass", "t1t1t1t1t1t1"))
w.forge.open = []OpenPull{openPull("novox/app", 7, readyHead)}
w.h.SeeOpenPulls()
w.settleAll()
if open := w.op.open(); len(open) != 1 || !strings.Contains(w.op.ask(open[0]).Explanation, "not reviewed at this head") {
t.Fatalf("setup: the ask is %v", w.op.open())
}
return w
}
func edited(body string) PullEvent {
p := pr("novox/app", 7, readyHead, "feat/x")
p.Body = body
return p
}
func TestAReviewedByLineWrittenAfterTheAskWithdrawsTheUnansweredAskAndAsksAgain(t *testing.T) {
w := askedUnreviewed(t)
first := w.op.open()[0]
w.h.PullEdited(edited("Reviewed-by: reviewer@shanks at " + readyHead))
w.settleAll()
open := w.op.open()
if len(open) != 1 || open[0] == first || w.op.count() != 2 {
t.Fatalf("after the edit the open asks are %v (first %s), %d asked; wanted the first withdrawn and one new", open, first, w.op.count())
}
if len(w.op.cancelled) != 1 || w.op.cancelled[0] != first {
t.Fatalf("the first ask was not withdrawn: %v", w.op.cancelled)
}
if e := w.op.ask(open[0]).Explanation; !strings.Contains(e, "reviewed by reviewer on shanks at "+shortOf(readyHead, 12)) ||
strings.Contains(e, "not reviewed") {
t.Fatalf("the new ask does not say the record: %s", e)
}
if len(w.forge.comments) != 0 {
t.Fatalf("a withdrawn ask is not an ended Approve, so nothing is said on the pull request: %v", w.forge.comments)
}
// The new ask is the one the Approve answers.
w.op.answer(open[0], OptionApprove)
w.settleAll()
if len(w.forge.merges) != 1 {
t.Fatalf("the new ask's Approve did not merge: %v", w.forge.merges)
}
}
func TestAnEditThatChangesNothingTheAskSaysAsksNothingAndNoEditChangesAnAnsweredAsk(t *testing.T) {
w := askedUnreviewed(t)
// The same words: no description with a line, then a description without one.
w.h.PullEdited(edited("A new sentence in the description."))
w.settleAll()
if w.op.count() != 1 || len(w.op.cancelled) != 0 {
t.Fatalf("an edit that changes no word of the ask asked again: %d asks, cancelled %v", w.op.count(), w.op.cancelled)
}
// An edit that brings a line for another head changes the words: the ask is made again, still saying not reviewed.
w.h.PullEdited(edited("Reviewed-by: reviewer@shanks at " + strings.Repeat("a", 40)))
w.settleAll()
if w.op.count() != 2 || len(w.op.open()) != 1 {
t.Fatalf("a line for another head: %d asks, open %v", w.op.count(), w.op.open())
}
// Answered: a later edit changes nothing.
w.op.answer(w.op.open()[0], OptionApprove)
w.settleAll()
asked := w.op.count()
w.h.PullEdited(edited("Reviewed-by: reviewer@shanks at " + readyHead))
w.settleAll()
if w.op.count() != asked || len(w.op.open()) != 0 {
t.Fatalf("an edit after the Approve asked again: %d asks, open %v", w.op.count(), w.op.open())
}
if d := w.delivery(IDOf("novox/app", readyHead)); d.ReviewedBy != "reviewer@shanks" {
t.Fatalf("the record is still read: %q", d.ReviewedBy)
}
}
func TestAnEditOfAnUnknownOrMergedOrEmptyPullRequestChangesNothing(t *testing.T) {
w := askedUnreviewed(t)
other := pr("novox/other", 3, readyHead, "feat/y")
other.Body = "Reviewed-by: a@b at " + readyHead
w.h.PullEdited(other) // no delivery: nothing made
w.h.PullEdited(edited(""))
w.settleAll()
if w.op.count() != 1 || len(w.h.deliveries) != 1 {
t.Fatalf("asks %d, deliveries %d", w.op.count(), len(w.h.deliveries))
}
}
modules/mesh-delivery/cmd/mesh-delivery/issue1083_test.go
+89
View File
@@ -0,0 +1,89 @@
package main
import (
"strings"
"testing"
)
// novox/hq issue 1083: mesh-controller #375's delivery was stopped by a person, then #375 was closed and the same commit
// opened as #376 from another branch. The delivery is named repository@commit; it was Stopped before its pull request
// closed, so it was not ClosedUnmerged, and #376 got no delivery of its own: its Approve was never asked. A new pull
// request at a commit whose delivery is Stopped and unmerged, under another number, takes that record over (the key stays
// repository@commit); the take-over is said in its transitions, with who stopped it and why.
const (
head1083 = "e375e375e375e375"
branch1083 = "feat/backup-before-send"
)
func TestAPullRequestAtACommitWhoseDeliveryAPersonStoppedTakesItOver(t *testing.T) {
w := newWorld(t)
id := IDOf(catalog783, head1083)
w.h.PullUpdated(pr(catalog783, 375, head1083, branch1083, "src/a.go"))
w.h.Checked(verdict(catalog783, 375, head1083, "pass"))
if w.state(id) != Ready {
t.Fatalf("its passing verdict left it %s", w.state(id))
}
if _, err := w.h.Stop(id, "to break the loop", "jochen"); err != nil {
t.Fatal(err)
}
// The person stopped it first; the pull request closed afterwards: a final state is not moved by the close.
w.h.PullClosed(pr(catalog783, 375, head1083, branch1083))
if d := w.delivery(id); d.State != Stopped || d.ClosedUnmerged {
t.Fatalf("setup: %s, closed unmerged %v; wanted stopped by the person alone", d.State, d.ClosedUnmerged)
}
w.h.PullUpdated(pr(catalog783, 376, head1083, branch1083+"-2", "src/a.go"))
d := w.delivery(id)
if d.State != Proposed || d.Number != 376 {
t.Fatalf("#376 at the same commit left it %s, Number %d; wanted proposed under #376", d.State, d.Number)
}
why := d.Transitions[len(d.Transitions)-1].Why
if !strings.Contains(why, "#375 with the same head was stopped by jochen: to break the loop") {
t.Fatalf("the take-over is not said, with who stopped it and why: %q", why)
}
w.h.Checked(verdict(catalog783, 376, head1083, "pass"))
if w.state(id) != Ready {
t.Fatalf("a passing verdict for #376 left it %s", w.state(id))
}
}
// A stopped record that was merged is not taken over, and the same pull request announced again stays stopped.
func TestAStoppedRecordThatWasMergedIsNotTakenOverByAnotherPullRequest(t *testing.T) {
w := newWorld(t)
id := IDOf(catalog783, head783c+"1")
w.h.PullUpdated(pr(catalog783, 400, head783c+"1", branch1083, "src/a.go"))
w.h.PullMerged(merged(catalog783, 400, head783c+"1", "f400f400f400f400"))
w.h.PullUpdated(pr(catalog783, 401, head783c+"1", branch1083+"-2", "src/a.go"))
if d := w.delivery(id); d.Number != 400 || d.MergedAs == "" {
t.Fatalf("a merged record was taken over: Number %d, merged as %q", d.Number, d.MergedAs)
}
}
// The take-over leaves the stopped record's group: the new record joins its own group by its branch.
func TestATakenOverRecordLeavesTheGroupItWasStoppedIn(t *testing.T) {
w := newWorld(t)
w.h.PullUpdated(pr(gCtl, 1, gHc, "feat/g", "internal/a.go"))
w.h.PullUpdated(pr(gCat, 2, gHk, "feat/g", "modules/app/module.json"))
id := IDOf(gCtl, gHc)
if _, err := w.h.Stop(id, "no", "jochen"); err != nil {
t.Fatal(err)
}
w.h.PullClosed(pr(gCtl, 1, gHc, "feat/g"))
w.h.PullUpdated(pr(gCtl, 9, gHc, "feat/other", "internal/a.go"))
w.h.mu.Lock()
defer w.h.mu.Unlock()
for _, g := range w.h.groups {
if g.ID == "feat/g" {
for _, m := range g.Members {
if m == id {
t.Fatalf("group feat/g still lists %s, now #9 on feat/other: %v", id, g.Members)
}
}
}
}
if d := w.h.deliveries[id]; d.Number != 9 || d.State != Proposed || d.Group == "feat/g" {
t.Fatalf("taken over: Number %d, %s, group %q", d.Number, d.State, d.Group)
}
}
modules/mesh-delivery/cmd/mesh-delivery/main.go
+9 -1
View File
@@ -34,7 +34,7 @@ func (l *listening) get() string { l.mu.Lock(); defer l.mu.Unlock(); return l.n
// The events this owner follows: the forge's pull requests, and the controller's verdicts and walks.
// The forge's status events say that a check answered (events.go).
var followed = []string{"gitea.pull.updated", "gitea.pull.merged", "gitea.pull.closed",
"gitea.forge.status", ControllerSeat + ".checked", ControllerSeat + ".plan-moved"}
"gitea.forge.status", "gitea.forge.pull-request.edited", ControllerSeat + ".checked", ControllerSeat + ".plan-moved"}
func main() {
operator := &asker.Client{Module: Module, State: stdio.State("asked")}
@@ -149,6 +149,14 @@ func take(h *Holder, e stdio.Envelope) error {
default:
h.PullClosed(p)
}
case "gitea.forge.pull-request.edited":
// The description edited after a head was announced (issue 1079): the review record in it is read again.
var p PullEvent
if err := json.Unmarshal(e.Body, &p); err != nil {
h.Refuse(fmt.Errorf("%s is not readable: %v", e.Key, err))
return nil
}
h.PullEdited(p)
case "gitea.forge.status":
var s ForgeStatusEvent
if err := json.Unmarshal(e.Body, &s); err != nil {
modules/mesh-delivery/cmd/mesh-delivery/ready.go
+23 -1
View File
@@ -41,6 +41,16 @@ const mergeRetryAfter = time.Minute
// check takes (the controller's checkTimeout is 45 minutes), so a check on its way is never said.
const readyRefusedFor = time.Hour
// groupRefusedFor is how long the merge tool may refuse a closed group's member before it is said as stalled: the merge
// window of ADR 0418 §3, not the hour of a lone head, because the group's other members are already on the trunk.
const groupRefusedFor = 10 * time.Minute
// groupClosed is whether a group is closed: a member reached the trunk. Under the lock.
func (h *Holder) groupClosed(id string) bool {
g := h.groups[id]
return g != nil && g.Closed
}
// ReadyRun is what this owner did with a delivery's Approve and merge: kept on the delivery and shown whole by `show`.
type ReadyRun struct {
// Head is the head the run is for.
@@ -235,7 +245,10 @@ func (h *Holder) askFresh(d *Delivery, now time.Time) func() {
d.Number == 0 || d.TrunkNow == "" {
return nil
}
if !d.FreshWanted && h.covering(d) == nil {
// A group's member too (issue 1047, design 47: a move of the trunk asks every check again): the group's one Approve is
// asked after its composed check, so a member ready before that has no Approve to carry; its check is asked at once
// all the same, and the composed check follows on the new verdict. The rest of ADR 0395 §1 stays with issue 888.
if !d.FreshWanted && h.covering(d) == nil && h.groups[d.Group] == nil {
return nil
}
if tried, was := h.askTried[d.ID]; was && now.Sub(tried) < time.Minute {
@@ -407,6 +420,15 @@ func (h *Holder) readyLines(now time.Time) []StalledLine {
Bound: "0s", H2: "none: a new head, which asks the operator again — the operator's",
Says: fmt.Sprintf("%s#%d at %s is not merged: the operator's answer %s", d.Repository,
d.Number, shortOf(r.Head, 12), r.Refused)})
case r.RefusedSince != nil && d.Group != "" && h.groupClosed(d.Group) && now.Sub(*r.RefusedSince) >= groupRefusedFor:
// A closed group's member (novox/hq ADR 0418 §3, issue 1082): its sibling merged, and its own merge is refused
// again after each composed check. Said after the merge window, with the group and the tool's last answer.
out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: groupRefusedFor.String(),
H2: "none: the group's composed check is asked again after each refusal, and the merge each minute — the operator's",
Says: fmt.Sprintf("%s#%d is approved at %s with its group %s closed (a sibling merged), and the merge tool has "+
"refused it since %s: %s", d.Repository, d.Number, shortOf(r.Head, 12), d.Group,
r.RefusedSince.UTC().Format(time.RFC3339), r.Answer)})
case r.RefusedSince != nil && now.Sub(*r.RefusedSince) >= readyRefusedFor:
out = append(out, StalledLine{ID: d.ID, Number: d.Number, State: d.State,
For: now.Sub(*r.RefusedSince).Round(time.Second).String(), Bound: readyRefusedFor.String(),
modules/mesh-delivery/module.json
+1
View File
@@ -28,6 +28,7 @@
"gitea.pull.merged",
"gitea.pull.closed",
"gitea.forge.status",
"gitea.forge.pull-request.edited",
"mesh-controller.checked",
"mesh-controller.plan-moved"
],