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

@@ -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:

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)
+	}
+}

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")

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)
+	}
+}