From a6dd698114821e13b5426ece8e66e01e7c9561fb Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Mon, 19 Oct 2026 21:31:09 +0000 Subject: [PATCH 1/3] Escape group and repository names in every delivery key so a dotted branch keys (issue 1323) --- go/delivery/delivery.go | 44 +++++++++++++++++++ go/delivery/gate.go | 32 +++++++------- go/delivery/gate_test.go | 86 ++++++++++++++++++++++++++++++++++++-- go/delivery/source.go | 56 ++++++++++++++++++++++--- go/delivery/source_test.go | 6 +-- 5 files changed, 198 insertions(+), 26 deletions(-) diff --git a/go/delivery/delivery.go b/go/delivery/delivery.go index fe99528..ff6625e 100644 --- a/go/delivery/delivery.go +++ b/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: `.`. A module named `node` is // refused, since `.node` is the node's own facts. func HaveKey(node, module string) (string, error) { diff --git a/go/delivery/gate.go b/go/delivery/gate.go index cd85b3f..fe9cf56 100644 --- a/go/delivery/gate.go +++ b/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: ``. func MergesKey(group string) (string, error) { - if err := checkName("group", group); err != nil { - return "", err - } - return group, nil + return groupKey(group) } // MergeAskKey is the key, in AsksBucket, of the ask for merging a group at these heads: `merge..`. 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..", 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. @@ -147,7 +146,8 @@ func ParseOwedAct(act string) (kind, context string, err error) { return checkOw // `...`, the act being `status=` (mesh/merge-gate), `view`, `note` or // `comment`. 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 { @@ -159,7 +159,7 @@ 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=` for a status. @@ -172,10 +172,14 @@ 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 @@ -197,8 +201,8 @@ type MergePull struct { } func (m Merge) check() error { - if err := checkName("group", m.Group); err != nil { - return err + if m.Group == "" { + return fmt.Errorf("delivery: a merge names no group") } if m.Ask == "" { return fmt.Errorf("delivery: a merge names no ask") diff --git a/go/delivery/gate_test.go b/go/delivery/gate_test.go index d8306a1..437d27b 100644 --- a/go/delivery/gate_test.go +++ b/go/delivery/gate_test.go @@ -21,8 +21,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 { - t.Error("a group with a dot accepted") + if _, err := MergesKey(""); err == nil { + t.Error("an empty group accepted") } hh, _ := HeadsHash(gateHeads()) key, err := MergeAskKey("feat/x", gateHeads()) @@ -42,7 +42,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) } @@ -299,3 +299,83 @@ 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) + } +} diff --git a/go/delivery/source.go b/go/delivery/source.go index 0ec2568..25bbe5e 100644 --- a/go/delivery/source.go +++ b/go/delivery/source.go @@ -67,18 +67,62 @@ const SourcePrefix = "src1:" // CandidatesKey is the key, in CandidatesBucket, of a 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. -func ParseCandidatesKey(key string) (group string, err error) { - if group, err = CandidatesKey(key); err != nil { - return "", fmt.Errorf("delivery: %q is not a candidates key: %w", key, err) +// escapeRepository is `/` as a key part, each part escaped; the repository must have exactly one slash +// and neither part empty. +func escapeRepository(repository string) (string, error) { + owner, name, ok := strings.Cut(repository, "/") + if !ok || owner == "" || name == "" || strings.Contains(name, "/") { + return "", fmt.Errorf("delivery: the repository %q is not /, 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 /, 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: ``. diff --git a/go/delivery/source_test.go b/go/delivery/source_test.go index 8969545..c269277 100644 --- a/go/delivery/source_test.go +++ b/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) -- 2.54.0 From 44d15fa0be845a763895dff08b96cefdaad0020e Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Mon, 19 Oct 2026 21:31:50 +0000 Subject: [PATCH 2/3] 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 | 32 ++++++++++++++++--- go/delivery/gate_test.go | 69 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/go/delivery/gate.go b/go/delivery/gate.go index fe9cf56..eff609e 100644 --- a/go/delivery/gate.go +++ b/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: `@`. +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: diff --git a/go/delivery/gate_test.go b/go/delivery/gate_test.go index 437d27b..e1d1e3a 100644 --- a/go/delivery/gate_test.go +++ b/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) + } +} -- 2.54.0 From 2f1454699a49b3f0af8f79dc4dc9d3fcaba24c2d Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Mon, 19 Oct 2026 21:32:55 +0000 Subject: [PATCH 3/3] Let an owed act carry a status target, a commit without a pull request and a numbered comment (issue 1322) --- go/delivery/gate.go | 39 ++++++++++++++++----- go/delivery/gate_test.go | 76 +++++++++++++++++++++++++++++++++++----- 2 files changed, 98 insertions(+), 17 deletions(-) diff --git a/go/delivery/gate.go b/go/delivery/gate.go index eff609e..6996b5c 100644 --- a/go/delivery/gate.go +++ b/go/delivery/gate.go @@ -130,28 +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=, 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. with seq of 1 or more", act) + } + return OwedComment, seq, nil + } + return "", "", fmt.Errorf("delivery: the owed act %q is not status=, view, note or comment.", 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 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: // `...`, the act being `status=` (mesh/merge-gate), `view`, `note` or -// `comment`. +// `comment.`. 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) { repo, err := escapeRepository(repository) if err != nil { return "", err } - if number <= 0 { - return "", fmt.Errorf("delivery: an owed key needs the pull request number, not %d", number) + if number < 0 { + 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 @@ -164,7 +180,7 @@ func OwedKey(repository string, number int, head, act string) (string, error) { // ParseOwedKey is the inverse of OwedKey; act is whole, as `status=` 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, ...", key) } @@ -291,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 -// comment need Text, a note needs Text and Ref (the notes ref, mesh-plan). +// Owed is the value of an owed key: what the forge bridge performs. A status needs Context, State and Target (the +// 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"` @@ -308,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 diff --git a/go/delivery/gate_test.go b/go/delivery/gate_test.go index e1d1e3a..2d00c09 100644 --- a/go/delivery/gate_test.go +++ b/go/delivery/gate_test.go @@ -80,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) @@ -94,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) } @@ -102,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 { - t.Error("pull request number 0 accepted") + if _, err := OwedKey("novox/mesh-sdk", -1, "aaaa", "view"); err == nil { + 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) } @@ -134,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"}, @@ -252,8 +252,9 @@ func TestGateValuesRefused(t *testing.T) { // 1201/AC5 } for name, o := range map[string]Owed{ - "context": {Act: OwedStatus, State: "success"}, - "state": {Act: OwedStatus, Context: "mesh/merge-gate", State: "green"}, + "context": {Act: OwedStatus, State: "success", Target: "t"}, + "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}, @@ -448,3 +449,62 @@ func TestMergeWithoutApprovedHeadsDecodes(t *testing.T) { // 1321/AC3 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) + } + } +} -- 2.54.0