From 2f1454699a49b3f0af8f79dc4dc9d3fcaba24c2d Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Mon, 19 Oct 2026 21:32:55 +0000 Subject: [PATCH] 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) + } + } +}