Keep a group member of the other kind's batch from being overtaken (hq issue 525)

A merge's group member counted as merged when its merge waited in any batch,
but kinds never share a batch: the later member got a batch of its own, cut
at once, and walked before the earlier member's batch was cut, breaking the
group's order (ADR 0276 §5). Only a member kept in a batch of the merge's own
kind counts as merged now; one kept in the other kind's keeps the merge
waiting, and its batch names that member, so the two are cut in the order
their windows close (review of #250).

cmd/mesh-controller/batches.go

@@ -179,7 +179,8 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T
 		return nil
 	}
 	// **Its delivery group's word, before the plans are held** (ADR 0306 decision 2): a slow answer holds up no walk.
-	waits, err := groupWait(ctx, inv, m, deliverySeatHeld(entries))
+	own := ownPath(touches)
+	waits, err := groupWait(ctx, inv, m, deliverySeatHeld(entries), own)
 	if err != nil {
 		return notNow(err)
 	}
@@ -208,7 +209,6 @@ func (f following) hearMerge(ctx context.Context, m link.SourceMoved, now time.T
 	// **A merge on the controller's own path never shares a batch with one that waits for the delivery's
 	// word** (decided during the build, 2026-10-10): batched together, the catalogue's deliveries would start
 	// with the controller's and skip their turn. Each kind has a batch of its own.
-	own := ownPath(touches)
 	if p, ok, err := answeredByALaterMerge(ctx, inv, heard, touches, own, deliverySeatHeld(entries)); err != nil {
 		return notNow(err)
 	} else if ok {

cmd/mesh-controller/group_wait.go

@@ -84,14 +84,29 @@ func memberOf(s string) groupMember {
 }
 
 // groupWait says whether a merge heard now waits for the window for its delivery group, and why in words; nothing
-// when it names no head, its head names no group, or every other member is merged or closed. A member of another
-// repository whose merge this controller already keeps in a batch not yet cut is merged, whatever mesh-delivery
-// says yet (it marks a member merged once its walk opened). Asked before the plans are held: a slow answer holds
-// up no walk.
-func groupWait(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, held bool) (string, error) {
+// when it names no head, its head names no group, or every other member is merged or closed. own is the merge's
+// kind (ownPath).
+//
+// A member of another repository whose merge this controller already keeps in a batch not yet cut of the same
+// kind is merged, whatever mesh-delivery says yet (it marks a member merged once its walk opened): the merge joins
+// that batch by their head. One kept in a batch of the other kind keeps the merge waiting, whatever mesh-delivery
+// says (review of #250): kinds never share a batch, so the merge's batch is a new one, and cut at once it would
+// walk before that member's batch is cut and break the group's order (ADR 0276 decision 5). Waiting, the two
+// batches are cut in the order their windows close, an own-path one first at one look, as cutBatchesHeld cuts
+// every pair of kinds.
+//
+// Asked before the plans are held: a slow answer holds up no walk.
+func groupWait(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved, held, own bool) (string, error) {
 	if m.Head == "" {
 		return "", nil
 	}
+	same, other, err := keptFromHead(ctx, inv, m.Head, own)
+	if err != nil {
+		return "", err
+	}
+	if len(other) > 0 {
+		return "group " + m.Head + " has a member waiting in a batch of the other kind: " + strings.Join(other, ", "), nil
+	}
 	members, isGroup, err := deliveryGroupOf(ctx, held, m.Head)
 	if err != nil {
 		return fmt.Sprintf("%s did not say whether group %s has a member still to merge (%v): it waits for the "+
@@ -100,13 +115,9 @@ func groupWait(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved
 	if !isGroup {
 		return "", nil
 	}
-	kept, err := keptFromHead(ctx, inv, m.Head)
-	if err != nil {
-		return "", err
-	}
 	var open []string
 	for _, g := range members {
-		if strings.EqualFold(g.Repository, m.Owner+"/"+m.Repo) || kept[strings.ToLower(g.Repository)] ||
+		if strings.EqualFold(g.Repository, m.Owner+"/"+m.Repo) || same[strings.ToLower(g.Repository)] ||
 			!openPullStates[g.State] {
 			continue
 		}
@@ -118,23 +129,30 @@ func groupWait(ctx context.Context, inv *inventory.Inventory, m link.SourceMoved
 	return "group " + m.Head + " has a member still to merge: " + strings.Join(open, ", "), nil
 }
 
-// keptFromHead are the repositories (lower case) of the merges from a head that batches not yet cut hold.
-func keptFromHead(ctx context.Context, inv *inventory.Inventory, head string) (map[string]bool, error) {
+// keptFromHead are the merges from a head that batches not yet cut hold: the repositories (lower case) of those in
+// a batch of the kind own says, and, in words, each of the other kind with its batch.
+func keptFromHead(ctx context.Context, inv *inventory.Inventory, head string, own bool) (map[string]bool, []string, error) {
 	batches, err := inv.Batches(ctx)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	out := map[string]bool{}
+	same := map[string]bool{}
+	var other []string
 	for _, b := range batches {
 		merges, err := inv.MergesOf(ctx, b.ID)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		for _, k := range merges {
-			if announcedOf(k).Head == head {
-				out[strings.ToLower(k.Repository)] = true
+			if announcedOf(k).Head != head {
+				continue
+			}
+			if b.OwnPath() == own {
+				same[strings.ToLower(k.Repository)] = true
+			} else {
+				other = append(other, fmt.Sprintf("%s@%s in %s", k.Repository, short(k.Commit), b.ID))
 			}
 		}
 	}
-	return out, nil
+	return same, other, nil
 }

cmd/mesh-controller/lone_merge_test.go

@@ -167,17 +167,17 @@ func TestAGroupsMergedOrClosedMembersMakeNobodyWait(t *testing.T) {
 	deliveryGroupOf = func(context.Context, bool, string) ([]groupMember, bool, error) { return members, true, nil }
 	t.Cleanup(func() { deliveryGroupOf = was })
 	m := fromHead(repoMerge("one", "c1", t0), "feat/x")
-	if why, err := groupWait(ctx, open.inventory, m, true); err != nil || why != "" {
+	if why, err := groupWait(ctx, open.inventory, m, true, false); err != nil || why != "" {
 		t.Fatalf("the group's other members are merged or closed, yet: %q %v", why, err)
 	}
 	members = append(members, groupMember{Repository: "novox/four", State: "checked"})
-	if why, _ := groupWait(ctx, open.inventory, m, true); !strings.Contains(why, "novox/four (checked)") {
+	if why, _ := groupWait(ctx, open.inventory, m, true, false); !strings.Contains(why, "novox/four (checked)") {
 		t.Fatalf("an open member: %q", why)
 	}
 	// novox/four's merge is kept in a batch already: merged, whatever mesh-delivery says yet.
 	hear(t, open, fromHead(repoMerge("two", "c4", t0), "feat/x"), t0)
 	members[3].Repository = "novox/two"
-	if why, _ := groupWait(ctx, open.inventory, m, true); why != "" {
+	if why, _ := groupWait(ctx, open.inventory, m, true, false); why != "" {
 		t.Fatalf("a member this controller keeps the merge of still waits: %q", why)
 	}
 	if got := memberOf("novox/mesh-sdk@d41d255f28ad (held)"); got != (groupMember{Repository: "novox/mesh-sdk", State: "held"}) {
@@ -329,3 +329,70 @@ func TestALoneMergeRelatedToAWalkTheCutFoldsJoinsTheBatch(t *testing.T) {
 		t.Fatalf("a lone merge related to the waiting walk: batches %+v", bs)
 	}
 }
+
+// A delivery group with a member of each kind (review of #250): a merge on the controller's own path and a
+// catalogue merge never share a batch, so the later member cannot join the earlier one's. Kept in a batch of the
+// other kind, the earlier member keeps the later one waiting, whatever mesh-delivery says yet — never cut at once
+// to walk before it. The two batches are then cut in the order their windows close (cutBatchesHeld: the oldest
+// closed batch first, an own-path one first at one look, and the other queued behind a walk that started): the
+// earlier member's window closes first, so it walks first, in both orders.
+func TestAGroupAcrossBothKindsKeepsItsOrder(t *testing.T) {
+	for _, ownFirst := range []bool{true, false} {
+		name := "catalogue member first"
+		if ownFirst {
+			name = "controller member first"
+		}
+		t.Run(name, func(t *testing.T) {
+			open := windowed(t)
+			asksWithPaths(t)
+			ctx := t.Context()
+			was := deliveryGroupOf
+			// mesh-delivery still says both members ready: it marks one merged only once its walk opened.
+			deliveryGroupOf = func(context.Context, bool, string) ([]groupMember, bool, error) {
+				return []groupMember{{Repository: "novox/mesh-controller", State: "ready"},
+					{Repository: "novox/mesh-catalog", State: "ready"}}, true, nil
+			}
+			t.Cleanup(func() { deliveryGroupOf = was })
+			own := fromHead(repoMerge("mesh-controller", "k1", t0), "feat/both-kinds")
+			cat := fromHead(catalogueMerge("cat1aaaaaaaa", "app", t0), "feat/both-kinds")
+			first, second := cat, own
+			if ownFirst {
+				first, second = own, cat
+			}
+			second.MergedAt = t0.Add(10 * time.Second).Format(time.RFC3339Nano)
+			hear(t, open, first, t0)
+			hear(t, open, second, t0.Add(10*time.Second))
+			ws, bs := walks(t, open)
+			if len(ws) != 0 || len(bs) != 2 {
+				t.Fatalf("both members heard: %d walk(s), %d batch(es); want two batches, nothing cut", len(ws), len(bs))
+			}
+			later := bs[0] // newest first
+			if later.Delivery.Batch.CutAtOnce || !strings.Contains(later.Delivery.Batch.Waits,
+				"has a member waiting in a batch of the other kind: novox/"+first.Repo+"@") {
+				t.Fatalf("the later member's batch reads %+v; want it waiting for the earlier member", later.Delivery.Batch)
+			}
+			// The earlier member's window closes first: its batch alone is cut.
+			cutAt(t, open, t0.Add(90*time.Second))
+			ws, bs = walks(t, open)
+			if len(ws) != 1 || !ws[0].Open() || ws[0].CommitOf("novox/"+first.Repo) != first.Commit || len(bs) != 1 ||
+				bs[0].ID != later.ID {
+				t.Fatalf("at the earlier window: walks %+v, batches %+v; want the earlier member alone cut", ws, bs)
+			}
+			earlier := ws[0]
+			// The later member's window closes while the earlier walk runs: it queues behind it.
+			cutAt(t, open, t0.Add(100*time.Second))
+			if _, bs := walks(t, open); len(bs) != 1 || bs[0].State != inventory.PlanQueued || bs[0].Delivery.Batch.Behind != earlier.ID {
+				t.Fatalf("the later member's batch does not queue behind the earlier walk: %+v", bs)
+			}
+			earlier.State = inventory.PlanDone
+			if err := open.inventory.SavePlan(ctx, &earlier); err != nil {
+				t.Fatal(err)
+			}
+			cutAt(t, open, t0.Add(101*time.Second))
+			got, err := open.inventory.PlanByID(ctx, later.ID)
+			if err != nil || got.Batch() || got.CommitOf("novox/"+second.Repo) != second.Commit {
+				t.Fatalf("the later member was not walked after the earlier: %s %v", got.State, err)
+			}
+		})
+	}
+}