Carry the Approve's heads of every group member in a merge so the bridge can hand them to the forge (issue 1321)

go/delivery/gate.go

@@ -183,12 +183,20 @@ func ParseOwedKey(key string) (repository string, number int, head, act string,
 }
 
 // Merge is the value of a merges key: the ask approved and, for each pull request, the head the approval expects
-// and how far its merge has come.
+// and how far its merge has come. Approved is the Approve's heads of every member of the group, ApprovedKey of
+// the member to its full head sha: the forge's merge tool wants them for every sibling whichever member it is
+// handed (ADR 0450 §3). A merge written before Approved existed has none and is not checked against it.
 type Merge struct {
-	V     int         `json:"v"`
-	Group string      `json:"group"`
-	Ask   string      `json:"ask"`
-	Pulls []MergePull `json:"pulls"`
+	V        int               `json:"v"`
+	Group    string            `json:"group"`
+	Ask      string            `json:"ask"`
+	Approved map[string]string `json:"approved,omitempty"`
+	Pulls    []MergePull       `json:"pulls"`
+}
+
+// ApprovedKey is the key of a member in Merge.Approved: `<repository>@<number>`.
+func ApprovedKey(repository string, number int) string {
+	return repository + "@" + strconv.Itoa(number)
 }
 
 // MergePull is one pull request of a merge: its expected head, its state, the merge commit once merged, the
@@ -210,8 +218,22 @@ func (m Merge) check() error {
 	if len(m.Pulls) == 0 {
 		return fmt.Errorf("delivery: a merge names no pull request")
 	}
+	for key, sha := range m.Approved {
+		if sha == "" {
+			return fmt.Errorf("delivery: the approved head of %s is empty", key)
+		}
+	}
 	for _, p := range m.Pulls {
 		who := fmt.Sprintf("%s#%d", p.Repository, p.Number)
+		if len(m.Approved) > 0 {
+			sha, ok := m.Approved[ApprovedKey(p.Repository, p.Number)]
+			if !ok {
+				return fmt.Errorf("delivery: the pull request %s is handed to the merge but is not among the approved heads", who)
+			}
+			if sha != p.Head.Head {
+				return fmt.Errorf("delivery: the pull request %s is handed at head %q but the Approve was for %q", who, p.Head.Head, sha)
+			}
+		}
 		switch p.State {
 		case MergeMerging:
 		case MergeMerged:

go/delivery/gate_test.go

@@ -3,6 +3,7 @@ package delivery
 import (
 	"encoding/json"
 	"errors"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -379,3 +380,71 @@ func TestKeysEscapeAndRoundTrip(t *testing.T) { // 1323/AC1
 		t.Errorf("decode merge: %v %v", back.Group, err)
 	}
 }
+
+func approvedMerge() Merge {
+	m := sampleMerge()
+	m.Approved = map[string]string{
+		ApprovedKey("novox/mesh-sdk", 36):    "aaaa",
+		ApprovedKey("novox/mesh-control", 7): "bbbb",
+		ApprovedKey("novox/other", 1):        "dddd",
+	}
+	return m
+}
+
+func TestMergeCarriesApprovedHeads(t *testing.T) { // 1321/AC1
+	m := approvedMerge()
+	m.Pulls = m.Pulls[:1] // two members approved besides the one handed so far
+	m.Pulls[0].State = MergeMerging
+	m.Pulls[0].Commit = ""
+	data, err := EncodeMerge(m)
+	if err != nil {
+		t.Fatal(err)
+	}
+	back, err := DecodeMerge(data)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m.V = Version
+	if !SameMerge(back, m) || len(back.Approved) != 3 || back.Approved["novox/mesh-control@7"] != "bbbb" {
+		t.Errorf("decoded %+v, want %+v", back, m)
+	}
+}
+
+func TestMergeRefusesAPullNotApproved(t *testing.T) { // 1321/AC2
+	m := approvedMerge()
+	delete(m.Approved, ApprovedKey("novox/other", 1))
+	if _, err := EncodeMerge(m); err == nil || !strings.Contains(err.Error(), "novox/other#1") {
+		t.Errorf("a handed pull missing from the approved heads: %v", err)
+	}
+	m = approvedMerge()
+	m.Approved[ApprovedKey("novox/other", 1)] = "eeee"
+	if _, err := EncodeMerge(m); err == nil {
+		t.Error("a handed pull at another head than the Approve's accepted")
+	}
+	m = approvedMerge()
+	m.Approved[ApprovedKey("novox/other", 1)] = ""
+	if _, err := EncodeMerge(m); err == nil {
+		t.Error("an empty approved head accepted")
+	}
+	data, _ := json.Marshal(approvedMerge())
+	var raw map[string]any
+	_ = json.Unmarshal(data, &raw)
+	raw["v"] = Version
+	raw["approved"] = map[string]string{"novox/mesh-sdk@36": "aaaa"}
+	bad, _ := json.Marshal(raw)
+	if _, err := DecodeMerge(bad); err == nil {
+		t.Error("decoding a merge whose pulls are not all approved accepted")
+	}
+}
+
+func TestMergeWithoutApprovedHeadsDecodes(t *testing.T) { // 1321/AC3
+	old := `{"v":` + strconv.Itoa(Version) + `,"group":"feat/x","ask":"merge.feat/x.0123456789ab","pulls":[{"repository":"novox/mesh-sdk","number":36,"head":"aaaa","state":"merging"}]}`
+	m, err := DecodeMerge([]byte(old))
+	if err != nil || len(m.Approved) != 0 || len(m.Pulls) != 1 {
+		t.Fatalf("old merge = %+v, %v", m, err)
+	}
+	data, err := EncodeMerge(sampleMerge())
+	if err != nil || strings.Contains(string(data), "approved") {
+		t.Errorf("a merge without approved heads wrote them: %s %v", data, err)
+	}
+}