From ba684e54976bf4d68efb1d7ea5d8428dedd5edde Mon Sep 17 00:00:00 2001 From: mesh-tools Date: Tue, 20 Oct 2026 01:18:49 +0000 Subject: [PATCH] Give each asking of a merge ask its own id, and add the have phase removed (issues 1370, 1369) The router drops an ask under an id it keeps as a redelivery, so a gate that writes the same group's ask again at unchanged heads was never heard. Ask gains Asking; AskingID gives the first asking the old id and later ones their own. PhaseRemoved lets the node-engine say a module left its node. --- go/delivery/delivery.go | 8 +++-- go/delivery/delivery_test.go | 16 +++++++++ go/delivery/gate.go | 23 +++++++++++-- go/delivery/gate_test.go | 64 ++++++++++++++++++++++++++++++++++++ 4 files changed, 107 insertions(+), 4 deletions(-) diff --git a/go/delivery/delivery.go b/go/delivery/delivery.go index ff6625e..8b907ee 100644 --- a/go/delivery/delivery.go +++ b/go/delivery/delivery.go @@ -80,6 +80,10 @@ const ( PhaseChecking Phase = "checking" PhaseRunning Phase = "running" PhaseFailed Phase = "failed" + // PhaseRemoved is written by the node-engine, once, for a module its applied declaration no longer holds and + // for which it had written a have key; the key is never deleted (issue 1369, ADR 0408 ยง4), so a reader can tell + // that the module stopped. + PhaseRemoved Phase = "removed" ) // Health is what the module's own check says. @@ -338,9 +342,9 @@ func decode(data []byte, into any) error { func (h Have) check() error { switch h.Phase { - case PhaseBackingUp, PhaseApplying, PhaseChecking, PhaseRunning, PhaseFailed: + case PhaseBackingUp, PhaseApplying, PhaseChecking, PhaseRunning, PhaseFailed, PhaseRemoved: default: - return fmt.Errorf("delivery: phase %q is not backing-up, applying, checking, running or failed", h.Phase) + return fmt.Errorf("delivery: phase %q is not backing-up, applying, checking, running, failed or removed", h.Phase) } switch h.Health { case HealthOK, HealthFailed, HealthWaits, HealthUnknown: diff --git a/go/delivery/delivery_test.go b/go/delivery/delivery_test.go index dce825e..9d1ab21 100644 --- a/go/delivery/delivery_test.go +++ b/go/delivery/delivery_test.go @@ -292,3 +292,19 @@ func TestSame(t *testing.T) { t.Error("different pulses same") } } + +func TestHaveRemoved(t *testing.T) { // 1370/AC3 + in := Have{Build: "b1", Phase: PhaseRemoved, Health: HealthUnknown, Detail: "the declaration no longer holds it"} + b, err := EncodeHave(in) + if err != nil { + t.Fatal(err) + } + out, err := DecodeHave(b) + if err != nil { + t.Fatal(err) + } + in.V = Version + if out != in || out.Phase != "removed" { + t.Errorf("round trip: %+v", out) + } +} diff --git a/go/delivery/gate.go b/go/delivery/gate.go index 6996b5c..61ec74d 100644 --- a/go/delivery/gate.go +++ b/go/delivery/gate.go @@ -119,6 +119,18 @@ func AskID(askKey string) string { return string(readable) + "-" + hex.EncodeToString(sum[:])[:12] } +// AskingID is the id of the asking-th writing of the ask at askKey. Asking 0 and 1 give AskID(askKey), so the first +// asking's id is unchanged; asking n of 2 or more gives the AskID form of askKey + "." + n, so each asking has an id +// of its own (at most 64 bytes, ending in the 12 hex digits of the hash of that whole string) and the router, which +// drops an ask under an id it keeps as a redelivery, takes each as new and an answer to an earlier asking is never +// taken for a later one. +func AskingID(askKey string, asking int) string { + if asking < 2 { + return AskID(askKey) + } + return AskID(askKey + "." + strconv.Itoa(asking)) +} + func checkOwedAct(act string) (kind, context string, err error) { kind, context, hasContext := strings.Cut(act, "=") switch kind { @@ -274,15 +286,22 @@ type Ask struct { Group string `json:"group"` Heads []Head `json:"heads"` Ask asks.Ask `json:"ask"` + // Asking counts the times the gate wrote this group's ask at these heads: 0 or 1 for the first, n for the n-th. + // The router drops an ask whose id it has already kept as a redelivery, so each asking has its own id + // (AskingID); the first asking's id is unchanged and the field is omitted for it. + Asking int `json:"asking,omitempty"` } func (a Ask) check() error { + if a.Asking < 0 { + return fmt.Errorf("delivery: the ask's asking %d is negative", a.Asking) + } key, err := MergeAskKey(a.Group, a.Heads) if err != nil { return fmt.Errorf("delivery: an ask needs its group and at least one pull request's head: %w", err) } - if want := AskID(key); a.Ask.ID != want { - return fmt.Errorf("delivery: the ask's id %q is not %q, the id its group and heads give", a.Ask.ID, want) + if want := AskingID(key, a.Asking); a.Ask.ID != want { + return fmt.Errorf("delivery: the ask's id %q is not %q, the id its group, heads and asking %d give", a.Ask.ID, want, a.Asking) } if len(a.Ask.Options) == 0 { return fmt.Errorf("delivery: the ask has no option") diff --git a/go/delivery/gate_test.go b/go/delivery/gate_test.go index 2d00c09..fe36564 100644 --- a/go/delivery/gate_test.go +++ b/go/delivery/gate_test.go @@ -508,3 +508,67 @@ func TestOwedCommentsHaveASequence(t *testing.T) { // 1322/AC3 } } } + +func TestAskingID(t *testing.T) { // 1370/AC1 + k, _ := MergeAskKey("feat/x", gateHeads()) + long, _ := MergeAskKey("feat/"+strings.Repeat("long-", 20), gateHeads()) + for _, key := range []string{k, long} { + if AskingID(key, 1) != AskID(key) || AskingID(key, 0) != AskID(key) { + t.Errorf("the first asking's id is not AskID: %q %q %q", AskingID(key, 0), AskingID(key, 1), AskID(key)) + } + seen := map[string]int{AskID(key): 1} + for n := 2; n <= 12; n++ { + id := AskingID(key, n) + if other, dup := seen[id]; dup { + t.Errorf("askings %d and %d give the id %q", n, other, id) + } + seen[id] = n + if len(id) > 64 || !asks.UsableID(id) { + t.Errorf("id %q of asking %d is not usable by the router", id, n) + } + if id != AskID(key+"."+strconv.Itoa(n)) { + t.Errorf("id of asking %d is not the AskID form of key.%d: %q", n, n, id) + } + } + } +} + +func TestAskChecksItsAsking(t *testing.T) { // 1370/AC2 + key, _ := MergeAskKey("feat/x", gateHeads()) + a := sampleAsk() + if _, err := EncodeAsk(a); err != nil { + t.Fatalf("asking 0: %v", err) + } + a.Asking = 1 + if _, err := EncodeAsk(a); err != nil { + t.Fatalf("asking 1 under the first id: %v", err) + } + a.Asking = 3 + a.Ask.ID = AskingID(key, 3) + data, err := EncodeAsk(a) + if err != nil { + t.Fatalf("asking 3 under its id: %v", err) + } + a.V = Version + if got, err := DecodeAsk(data); err != nil || !SameAsk(got, a) || got.Asking != 3 { + t.Errorf("asking round trip: %+v %v", got, err) + } + a.Ask.ID = AskingID(key, 2) + if _, err := EncodeAsk(a); err == nil { + t.Error("asking 3 under asking 2's id was accepted") + } + a.Asking = 2 + a.Ask.ID = AskID(key) + if _, err := EncodeAsk(a); err == nil { + t.Error("asking 2 under the first asking's id was accepted") + } + a.Asking = -1 + if _, err := EncodeAsk(a); err == nil { + t.Error("a negative asking was accepted") + } + // the first asking is written without the field + data, _ = EncodeAsk(sampleAsk()) + if strings.Contains(string(data), `"asking"`) { + t.Errorf("asking 0 is written: %s", data) + } +}