Merge pull request 'Give the gate's keys what the forge needs: approved heads, owed targets, escaped names (design 47 R3d, ADR 0450)' (#45) from feat/r3d-sdk-and-controller into main

go/delivery/delivery.go

@@ -35,6 +35,7 @@ package delivery
 
 import (
 	"bytes"
+	"encoding/hex"
 	"encoding/json"
 	"errors"
 	"fmt"
@@ -105,6 +106,49 @@ func checkName(kind, name string) error {
 	return nil
 }
 
+// EscapeName is name as a key part: a character outside letters, digits, `-`, `/` and `_` is written `=` and its
+// two lowercase hex digits per byte, and `=` itself is escaped, so any non-empty name keys and a name of allowed
+// characters only keys as it is. UnescapeName is its inverse.
+func EscapeName(name string) string {
+	var b strings.Builder
+	for i := 0; i < len(name); i++ {
+		c := name[i]
+		switch {
+		case c >= 'a' && c <= 'z', c >= 'A' && c <= 'Z', c >= '0' && c <= '9', c == '-', c == '/', c == '_':
+			b.WriteByte(c)
+		default:
+			fmt.Fprintf(&b, "=%02x", c)
+		}
+	}
+	return b.String()
+}
+
+// UnescapeName is the inverse of EscapeName; it refuses a part with a bare `=` or any character EscapeName leaves
+// escaped.
+func UnescapeName(part string) (string, error) {
+	var out []byte
+	for i := 0; i < len(part); i++ {
+		c := part[i]
+		if c != '=' {
+			if !nameOK.MatchString(string(c)) {
+				return "", fmt.Errorf("delivery: %q holds %q, which a key part escapes", part, c)
+			}
+			out = append(out, c)
+			continue
+		}
+		if i+3 > len(part) {
+			return "", fmt.Errorf("delivery: %q ends in an incomplete escape", part)
+		}
+		v, err := hex.DecodeString(part[i+1 : i+3])
+		if err != nil || part[i+1:i+3] != strings.ToLower(part[i+1:i+3]) {
+			return "", fmt.Errorf("delivery: %q holds a malformed escape %q", part, part[i:i+3])
+		}
+		out = append(out, v[0])
+		i += 2
+	}
+	return string(out), nil
+}
+
 // HaveKey is the key, in HaveBucket, of a module's have on a node: `<node>.<module>`. A module named `node` is
 // refused, since `<node>.node` is the node's own facts.
 func HaveKey(node, module string) (string, error) {

go/delivery/gate.go

@@ -56,23 +56,21 @@ var hashOK = regexp.MustCompile(`^[0-9a-f]{12}$`)
 
 // MergesKey is the key, in MergesBucket, of a group: `<group>`.
 func MergesKey(group string) (string, error) {
-	if err := checkName("group", group); err != nil {
+	return groupKey(group)
-		return "", err
-	}
-	return group, nil
 }
 
 // MergeAskKey is the key, in AsksBucket, of the ask for merging a group at these heads: `merge.<group>.<heads
 // hash>`. The teller's answer has the same key in AnswersBucket (AnswerKey).
 func MergeAskKey(group string, heads []Head) (string, error) {
-	if err := checkName("group", group); err != nil {
+	part, err := groupKey(group)
+	if err != nil {
 		return "", err
 	}
 	hash, err := HeadsHash(heads)
 	if err != nil {
 		return "", err
 	}
-	return AskKindMerge + "." + group + "." + hash, nil
+	return AskKindMerge + "." + part + "." + hash, nil
 }
 
 // ParseAskKey is the inverse of MergeAskKey: the kind, the group and the heads hash.
@@ -81,13 +79,14 @@ func ParseAskKey(key string) (kind, group, hash string, err error) {
 	if len(parts) != 3 || parts[0] != AskKindMerge {
 		return "", "", "", fmt.Errorf("delivery: %q is not an ask key, merge.<group>.<heads hash>", key)
 	}
-	if err := checkName("group", parts[1]); err != nil {
+	group, err = parseGroupKey(parts[1])
+	if err != nil {
 		return "", "", "", fmt.Errorf("delivery: %q is not an ask key: %w", key, err)
 	}
 	if !hashOK.MatchString(parts[2]) {
 		return "", "", "", fmt.Errorf("delivery: %q is not an ask key: %q is not 12 hex digits", key, parts[2])
 	}
-	return parts[0], parts[1], parts[2], nil
+	return parts[0], group, parts[2], nil
 }
 
 // AnswerKey is the key, in AnswersBucket, of the teller's answer to the ask at askKey: the same string.
@@ -131,27 +130,44 @@ func checkOwedAct(act string) (kind, context string, err error) {
 			return "", "", err
 		}
 		return kind, context, nil
-	case OwedView, OwedNote, OwedComment:
+	case OwedView, OwedNote:
 		if hasContext {
 			return "", "", fmt.Errorf("delivery: the owed act %q takes no context", act)
 		}
 		return kind, "", nil
 	}
-	return "", "", fmt.Errorf("delivery: the owed act %q is not status=<context>, view, note or comment", act)
+	if kind, seq, ok := strings.Cut(act, "."); ok && kind == OwedComment {
+		if n, err := strconv.Atoi(seq); err != nil || n < 1 || strconv.Itoa(n) != seq {
+			return "", "", fmt.Errorf("delivery: the owed act %q is a comment without its sequence, comment.<seq> with seq of 1 or more", act)
+		}
+		return OwedComment, seq, nil
+	}
+	return "", "", fmt.Errorf("delivery: the owed act %q is not status=<context>, view, note or comment.<seq>", act)
 }
 
-// ParseOwedAct splits an owed act into its kind and, for a status, its context.
+// OwedCommentAct is the act of the seq-th comment on a head: `comment.<seq>`, seq of 1 or more, so a second comment
+// has a key of its own and does not overwrite the first.
+func OwedCommentAct(seq int) (string, error) {
+	if seq < 1 {
+		return "", fmt.Errorf("delivery: a comment's sequence is 1 or more, not %d", seq)
+	}
+	return OwedComment + "." + strconv.Itoa(seq), nil
+}
+
+// ParseOwedAct splits an owed act into its kind and, for a status, its context, for a comment its sequence as
+// digits.
 func ParseOwedAct(act string) (kind, context string, err error) { return checkOwedAct(act) }
 
 // OwedKey is the key, in OwedBucket, of an act the gate owes the forge on a pull request at a head:
 // `<repository>.<number>.<head>.<act>`, the act being `status=<context>` (mesh/merge-gate), `view`, `note` or
-// `comment`.
+// `comment.<seq>`. The number is 0 for a commit no pull request names, and never negative.
 func OwedKey(repository string, number int, head, act string) (string, error) {
-	if err := checkRepository(repository); err != nil {
+	repo, err := escapeRepository(repository)
+	if err != nil {
 		return "", err
 	}
-	if number <= 0 {
+	if number < 0 {
-		return "", fmt.Errorf("delivery: an owed key needs the pull request number, not %d", number)
+		return "", fmt.Errorf("delivery: an owed key's pull request number is 0 (none) or more, not %d", number)
 	}
 	if err := checkName("head", head); err != nil {
 		return "", err
@@ -159,12 +175,12 @@ func OwedKey(repository string, number int, head, act string) (string, error) {
 	if _, _, err := checkOwedAct(act); err != nil {
 		return "", err
 	}
-	return repository + "." + strconv.Itoa(number) + "." + head + "." + act, nil
+	return repo + "." + strconv.Itoa(number) + "." + head + "." + act, nil
 }
 
 // ParseOwedKey is the inverse of OwedKey; act is whole, as `status=<context>` for a status.
 func ParseOwedKey(key string) (repository string, number int, head, act string, err error) {
-	parts := strings.Split(key, ".")
+	parts := strings.SplitN(key, ".", 4) // a comment's act holds a dot
 	if len(parts) != 4 {
 		return "", 0, "", "", fmt.Errorf("delivery: %q is not an owed key, <repository>.<number>.<head>.<act>", key)
 	}
@@ -172,21 +188,33 @@ func ParseOwedKey(key string) (repository string, number int, head, act string,
 	if convErr != nil {
 		return "", 0, "", "", fmt.Errorf("delivery: %q is not an owed key: %q is not a pull request number", key, parts[1])
 	}
-	if _, err := OwedKey(parts[0], number, parts[2], parts[3]); err != nil {
+	repository, err = unescapeRepository(parts[0])
+	if err != nil {
 		return "", 0, "", "", fmt.Errorf("delivery: %q is not an owed key: %w", key, err)
 	}
-	return parts[0], number, parts[2], parts[3], nil
+	if _, err := OwedKey(repository, number, parts[2], parts[3]); err != nil {
+		return "", 0, "", "", fmt.Errorf("delivery: %q is not an owed key: %w", key, err)
+	}
+	return repository, number, parts[2], parts[3], nil
 }
 
 // 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"`
+	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
 // forge's words when refused.
 type MergePull struct {
@@ -197,8 +225,8 @@ type MergePull struct {
 }
 
 func (m Merge) check() error {
-	if err := checkName("group", m.Group); err != nil {
+	if m.Group == "" {
-		return err
+		return fmt.Errorf("delivery: a merge names no group")
 	}
 	if m.Ask == "" {
 		return fmt.Errorf("delivery: a merge names no ask")
@@ -206,8 +234,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:
@@ -265,12 +307,14 @@ func (a Answer) check() error {
 	return nil
 }
 
-// Owed is the value of an owed key: what the forge bridge performs. A status needs Context and State, a view and a
+// Owed is the value of an owed key: what the forge bridge performs. A status needs Context, State and Target (the
-// comment need Text, a note needs Text and Ref (the notes ref, mesh-plan).
+// address the status points at), a view and a comment need Text, a note needs Text and Ref (the notes ref,
+// mesh-plan).
 type Owed struct {
 	V       int    `json:"v"`
 	Act     string `json:"act"`
 	Context string `json:"context,omitempty"`
+	Target  string `json:"target,omitempty"`
 	State   string `json:"state,omitempty"`
 	Text    string `json:"text,omitempty"`
 	Ref     string `json:"ref,omitempty"`
@@ -282,6 +326,9 @@ func (o Owed) check() error {
 		if o.Context == "" {
 			return fmt.Errorf("delivery: an owed status has no context")
 		}
+		if o.Target == "" {
+			return fmt.Errorf("delivery: an owed status has no target")
+		}
 		for _, s := range statusStates {
 			if o.State == s {
 				return nil

go/delivery/gate_test.go

@@ -3,6 +3,7 @@ package delivery
 import (
 	"encoding/json"
 	"errors"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -21,8 +22,8 @@ func TestMergeKeys(t *testing.T) { // 1201/AC1
 	if k, err := MergesKey("feat/x"); err != nil || k != "feat/x" {
 		t.Fatalf("merges key = %q, %v", k, err)
 	}
-	if _, err := MergesKey("a.b"); err == nil {
+	if _, err := MergesKey(""); err == nil {
-		t.Error("a group with a dot accepted")
+		t.Error("an empty group accepted")
 	}
 	hh, _ := HeadsHash(gateHeads())
 	key, err := MergeAskKey("feat/x", gateHeads())
@@ -42,7 +43,7 @@ func TestMergeKeys(t *testing.T) { // 1201/AC1
 	if _, err := MergeAskKey("", gateHeads()); err == nil {
 		t.Error("an ask key without a group accepted")
 	}
-	for _, bad := range []string{"", "merge", "merge.g", "merge.g.h.x", "other.g.0123456789ab", "merge..0123456789ab", "merge.g.nothex"} {
+	for _, bad := range []string{"", "merge", "merge.g", "merge.g.h.x", "other.g.0123456789ab", "merge..0123456789ab", "merge.g.nothex", "merge.a=zz.0123456789ab", "merge.a=4.0123456789ab", "merge.a=4A.0123456789ab", "merge.a+b.0123456789ab"} {
 		if _, _, _, err := ParseAskKey(bad); err == nil {
 			t.Errorf("parse ask key %q accepted", bad)
 		}
@@ -79,7 +80,7 @@ func TestAskID(t *testing.T) { // 1201/AC2
 }
 
 func TestOwedKey(t *testing.T) { // 1201/AC3
-	for _, act := range []string{"status=mesh/merge-gate", "view", "note", "comment"} {
+	for _, act := range []string{"status=mesh/merge-gate", "view", "note", "comment.1", "comment.12"} {
 		key, err := OwedKey("novox/mesh-sdk", 36, "aaaa", act)
 		if err != nil || key != "novox/mesh-sdk.36.aaaa."+act {
 			t.Fatalf("owed key for %s = %q, %v", act, key, err)
@@ -93,7 +94,7 @@ func TestOwedKey(t *testing.T) { // 1201/AC3
 	if err != nil || kind != OwedStatus || context != "mesh/merge-gate" {
 		t.Errorf("owed act = %q %q %v", kind, context, err)
 	}
-	for _, bad := range []string{"", "merge", "status", "status=", "view=x", "status=a.b", "statuses=x"} {
+	for _, bad := range []string{"", "merge", "status", "status=", "view=x", "status=a.b", "statuses=x", "comment", "comment.", "comment.0", "comment.-1", "comment.01", "comment.x", "comment=1"} {
 		if _, err := OwedKey("novox/mesh-sdk", 36, "aaaa", bad); err == nil {
 			t.Errorf("owed act %q accepted", bad)
 		}
@@ -101,13 +102,13 @@ func TestOwedKey(t *testing.T) { // 1201/AC3
 	if _, err := OwedKey("nope", 36, "aaaa", "view"); err == nil {
 		t.Error("a bad repository accepted")
 	}
-	if _, err := OwedKey("novox/mesh-sdk", 0, "aaaa", "view"); err == nil {
+	if _, err := OwedKey("novox/mesh-sdk", -1, "aaaa", "view"); err == nil {
-		t.Error("pull request number 0 accepted")
+		t.Error("a negative pull request number accepted")
 	}
 	if _, err := OwedKey("novox/mesh-sdk", 36, "", "view"); err == nil {
 		t.Error("an empty head accepted")
 	}
-	for _, bad := range []string{"", "novox/mesh-sdk.36.aaaa", "novox/mesh-sdk.x.aaaa.view", "novox/mesh-sdk.36.aaaa.merge", "a.b.36.aaaa.view"} {
+	for _, bad := range []string{"", "novox/mesh-sdk.36.aaaa", "novox/mesh-sdk.x.aaaa.view", "novox/mesh-sdk.-1.aaaa.view", "novox/mesh-sdk.36.aaaa.comment", "novox/mesh-sdk.36.aaaa.merge", "a.b.36.aaaa.view"} {
 		if _, _, _, _, err := ParseOwedKey(bad); err == nil {
 			t.Errorf("parse owed key %q accepted", bad)
 		}
@@ -133,7 +134,7 @@ func sampleAsk() Ask {
 
 func sampleOwed() []Owed {
 	return []Owed{
-		{Act: OwedStatus, Context: "mesh/merge-gate", State: "success"},
+		{Act: OwedStatus, Context: "mesh/merge-gate", State: "success", Target: "https://mesh.example/delivery/36"},
 		{Act: OwedView, Text: "the delivery reaches ace"},
 		{Act: OwedNote, Text: "plan: mesh-mail to ace", Ref: "mesh-plan"},
 		{Act: OwedComment, Text: "merged"},
@@ -251,8 +252,9 @@ func TestGateValuesRefused(t *testing.T) { // 1201/AC5
 	}
 
 	for name, o := range map[string]Owed{
-		"context": {Act: OwedStatus, State: "success"},
+		"context": {Act: OwedStatus, State: "success", Target: "t"},
-		"state":   {Act: OwedStatus, Context: "mesh/merge-gate", State: "green"},
+		"state":   {Act: OwedStatus, Context: "mesh/merge-gate", State: "green", Target: "t"},
+		"target":  {Act: OwedStatus, Context: "mesh/merge-gate", State: "success"},
 		"text":    {Act: OwedView},
 		"ref":     {Act: OwedNote, Text: "line"},
 		"comment": {Act: OwedComment},
@@ -299,3 +301,210 @@ func TestGateBuckets(t *testing.T) {
 		t.Error("a bucket or history differs from the decided layout")
 	}
 }
+
+// today's keys: names of letters, digits, - / and _ key exactly as before the escape.
+func TestKeysOfPlainNamesAreUnchanged(t *testing.T) { // 1323/AC2
+	hh, _ := HeadsHash(gateHeads())
+	for _, name := range []string{"feat/x", "fix/1323-escape", "feat/delivery_source-keys", "main", "A/B_c-9"} {
+		if k, err := MergesKey(name); err != nil || k != name {
+			t.Errorf("merges key %q = %q, %v", name, k, err)
+		}
+		if k, err := CandidatesKey(name); err != nil || k != name {
+			t.Errorf("candidates key %q = %q, %v", name, k, err)
+		}
+		if k, err := MergeAskKey(name, gateHeads()); err != nil || k != "merge."+name+"."+hh {
+			t.Errorf("ask key %q = %q, %v", name, k, err)
+		}
+	}
+	if k, err := OwedKey("novox/mesh-sdk", 36, "aaaa", "status=mesh/merge-gate"); err != nil || k != "novox/mesh-sdk.36.aaaa.status=mesh/merge-gate" {
+		t.Errorf("owed key = %q, %v", k, err)
+	}
+}
+
+func TestKeysEscapeAndRoundTrip(t *testing.T) { // 1323/AC1
+	hh, _ := HeadsHash(gateHeads())
+	for _, name := range []string{"fix/1.2", "feat/a+b", "feat/a=b", "a.b+c=d", "fix/ü", "x y"} {
+		esc := EscapeName(name)
+		if strings.ContainsAny(esc, ".+ ") {
+			t.Errorf("escape of %q = %q holds a dot, plus or space", name, esc)
+		}
+		if back, err := UnescapeName(esc); err != nil || back != name {
+			t.Errorf("unescape(%q) = %q, %v; want %q", esc, back, err, name)
+		}
+		if k, err := MergesKey(name); err != nil || k != esc {
+			t.Errorf("merges key %q = %q, %v", name, k, err)
+		}
+		ck, err := CandidatesKey(name)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if g, err := ParseCandidatesKey(ck); err != nil || g != name {
+			t.Errorf("candidates round trip %q = %q, %v", name, g, err)
+		}
+		ak, err := MergeAskKey(name, gateHeads())
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, g, h, err := ParseAskKey(ak); err != nil || g != name || h != hh {
+			t.Errorf("ask round trip %q = %q %q %v", name, g, h, err)
+		}
+		if AnswerKey(ak) != ak {
+			t.Error("answer key differs from the ask key")
+		}
+		// the ask and the answer of such a group encode and decode
+		ask := Ask{Group: name, Heads: gateHeads(), Ask: asks.Ask{ID: AskID(ak), Options: []asks.Option{{ID: "ok"}}}}
+		if data, err := EncodeAsk(ask); err != nil {
+			t.Errorf("encode ask for %q: %v", name, err)
+		} else if _, err := DecodeAsk(data); err != nil {
+			t.Errorf("decode ask for %q: %v", name, err)
+		}
+		repo := "novox/" + name[strings.Index(name, "/")+1:]
+		if !strings.Contains(name, "/") {
+			repo = "novox/" + name
+		}
+		ok, err := OwedKey(repo, 3, "aaaa", "view")
+		if err != nil {
+			t.Errorf("owed key for repository %q: %v", repo, err)
+			continue
+		}
+		if r, n, h, a, err := ParseOwedKey(ok); err != nil || r != repo || n != 3 || h != "aaaa" || a != "view" {
+			t.Errorf("owed round trip %q = %q %d %q %q %v", repo, r, n, h, a, err)
+		}
+	}
+	if EscapeName("a.b") != "a=2eb" || EscapeName("a=b") != "a=3db" || EscapeName("a+b") != "a=2bb" {
+		t.Errorf("escapes: %q %q %q", EscapeName("a.b"), EscapeName("a=b"), EscapeName("a+b"))
+	}
+	m := Merge{Group: "fix/1.2+x=y", Ask: "merge.x.0123456789ab", Pulls: []MergePull{{Head: gateHeads()[0], State: MergeMerging}}}
+	if data, err := EncodeMerge(m); err != nil {
+		t.Errorf("encode merge: %v", err)
+	} else if back, err := DecodeMerge(data); err != nil || back.Group != m.Group {
+		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)
+	}
+}
+
+func TestOwedStatusCarriesItsTarget(t *testing.T) { // 1322/AC1
+	o := Owed{Act: OwedStatus, Context: "mesh/delivery", State: "success", Target: "https://mesh.example/delivery/36"}
+	data, err := EncodeOwed(o)
+	if err != nil {
+		t.Fatal(err)
+	}
+	back, err := DecodeOwed(data)
+	o.V = Version
+	if err != nil || !SameOwed(back, o) || back.Target != o.Target {
+		t.Errorf("decoded %+v, %v; want %+v", back, err, o)
+	}
+	if _, err := EncodeOwed(Owed{Act: OwedStatus, Context: "mesh/delivery", State: "success"}); err == nil || !strings.Contains(err.Error(), "target") {
+		t.Errorf("a status without a target: %v", err)
+	}
+}
+
+func TestOwedKeyNumberZero(t *testing.T) { // 1322/AC2
+	key, err := OwedKey("novox/mesh-sdk", 0, "aaaa", "status=mesh/delivery")
+	if err != nil || key != "novox/mesh-sdk.0.aaaa.status=mesh/delivery" {
+		t.Fatalf("owed key for number 0 = %q, %v", key, err)
+	}
+	repo, number, head, act, err := ParseOwedKey(key)
+	if err != nil || repo != "novox/mesh-sdk" || number != 0 || head != "aaaa" || act != "status=mesh/delivery" {
+		t.Errorf("parse = %q %d %q %q %v", repo, number, head, act, err)
+	}
+	if _, err := OwedKey("novox/mesh-sdk", -1, "aaaa", "view"); err == nil {
+		t.Error("a negative number accepted by OwedKey")
+	}
+	if _, _, _, _, err := ParseOwedKey("novox/mesh-sdk.-1.aaaa.view"); err == nil {
+		t.Error("a negative number accepted by ParseOwedKey")
+	}
+}
+
+func TestOwedCommentsHaveASequence(t *testing.T) { // 1322/AC3
+	a1, err := OwedCommentAct(1)
+	a2, err2 := OwedCommentAct(2)
+	if err != nil || err2 != nil || a1 != "comment.1" || a2 != "comment.2" {
+		t.Fatalf("acts = %q %q %v %v", a1, a2, err, err2)
+	}
+	if _, err := OwedCommentAct(0); err == nil {
+		t.Error("comment sequence 0 accepted")
+	}
+	k1, err := OwedKey("novox/mesh-sdk", 36, "aaaa", a1)
+	k2, err2 := OwedKey("novox/mesh-sdk", 36, "aaaa", a2)
+	if err != nil || err2 != nil || k1 == k2 {
+		t.Fatalf("keys = %q %q %v %v", k1, k2, err, err2)
+	}
+	for key, seq := range map[string]string{k1: "1", k2: "2"} {
+		_, _, _, act, err := ParseOwedKey(key)
+		if err != nil {
+			t.Fatal(err)
+		}
+		kind, got, err := ParseOwedAct(act)
+		if err != nil || kind != OwedComment || got != seq {
+			t.Errorf("parse %q = %q %q %v; want comment %s", key, kind, got, err, seq)
+		}
+	}
+}

go/delivery/source.go

@@ -67,18 +67,62 @@ const SourcePrefix = "src1:"
 // CandidatesKey is the key, in CandidatesBucket, of a group: `<group>`, the branch name the group's pull requests
 // share (a slash is allowed); a lone pull request is a group of one under its branch name.
 func CandidatesKey(group string) (string, error) {
-	if err := checkName("group", group); err != nil {
+	return groupKey(group)
+}
+
+// ParseCandidatesKey is the inverse of CandidatesKey.
+func ParseCandidatesKey(key string) (group string, err error) {
+	if group, err = parseGroupKey(key); err != nil {
+		return "", fmt.Errorf("delivery: %q is not a candidates key: %w", key, err)
+	}
+	return group, nil
+}
+
+// groupKey is a group name as a key part: escaped (EscapeName), so a branch such as `fix/1.2` keys.
+func groupKey(group string) (string, error) {
+	if group == "" {
+		return "", fmt.Errorf("delivery: the group name is empty")
+	}
+	return EscapeName(group), nil
+}
+
+// parseGroupKey is the inverse of groupKey.
+func parseGroupKey(part string) (string, error) {
+	if part == "" {
+		return "", fmt.Errorf("delivery: the group name is empty")
+	}
+	group, err := UnescapeName(part)
+	if err != nil {
 		return "", err
 	}
 	return group, nil
 }
 
-// ParseCandidatesKey is the inverse of CandidatesKey.
+// escapeRepository is `<owner>/<name>` as a key part, each part escaped; the repository must have exactly one slash
-func ParseCandidatesKey(key string) (group string, err error) {
+// and neither part empty.
-	if group, err = CandidatesKey(key); err != nil {
+func escapeRepository(repository string) (string, error) {
-		return "", fmt.Errorf("delivery: %q is not a candidates key: %w", key, err)
+	owner, name, ok := strings.Cut(repository, "/")
+	if !ok || owner == "" || name == "" || strings.Contains(name, "/") {
+		return "", fmt.Errorf("delivery: the repository %q is not <owner>/<name>, with exactly one slash", repository)
 	}
-	return group, nil
+	return EscapeName(owner) + "/" + EscapeName(name), nil
+}
+
+// unescapeRepository is the inverse of escapeRepository.
+func unescapeRepository(part string) (string, error) {
+	owner, name, ok := strings.Cut(part, "/")
+	if !ok || owner == "" || name == "" || strings.Contains(name, "/") {
+		return "", fmt.Errorf("delivery: the repository %q is not <owner>/<name>, with exactly one slash", part)
+	}
+	o, err := UnescapeName(owner)
+	if err != nil {
+		return "", err
+	}
+	n, err := UnescapeName(name)
+	if err != nil {
+		return "", err
+	}
+	return o + "/" + n, nil
 }
 
 // ModulesKey is the key, in ModulesBucket, of a module: `<module>`.

go/delivery/source_test.go

@@ -22,9 +22,6 @@ func TestSourceKeys(t *testing.T) { // 1200/AC1
 		t.Fatalf("parse modules = %q, %v", m, err)
 	}
 	for _, bad := range []string{"", "a.b", "a*b", "a>b", "a b", "a\tb"} {
-		if _, err := CandidatesKey(bad); err == nil {
-			t.Errorf("candidates key %q accepted", bad)
-		}
 		if _, err := ModulesKey(bad); err == nil {
 			t.Errorf("modules key %q accepted", bad)
 		}
@@ -35,6 +32,9 @@ func TestSourceKeys(t *testing.T) { // 1200/AC1
 			t.Errorf("parse modules %q accepted", bad)
 		}
 	}
+	if _, err := CandidatesKey(""); err == nil {
+		t.Error("an empty candidates key accepted")
+	}
 	ev, err := ForgeEvent("novox/mesh-sdk", ForgeMerged)
 	if err != nil || ev != "forge.novox.mesh-sdk.merged" {
 		t.Fatalf("forge event = %q, %v", ev, err)