Take an ordered engine's report as a send's answer only by its epoch and sequence (novox/hq issue 485)

A periodic report about an identical earlier send names the same digest and
can arrive after a later send the machine never received, so the digest and
the time read it as applied. The digest-and-time rule stays for engines that
report no order. Tests now hold the digest condition and the sequence one.

internal/inventory/sends.go

@@ -244,15 +244,21 @@ func (i *Inventory) LastSends(ctx context.Context) (map[string]Send, error) {
 }
 
 // lastSends reads the newest recorded send of each machine matching where, beside the machine's last report
-// when that report is about the send: the same declaration (its digest), reported after it was sent — both
-// times the store's own, so no machine's clock decides it — so a machine sent back a declaration it once
-// applied does not read as having applied the new send before it said so (novox/hq issue 485).
+// when that report is about the send (novox/hq issue 485). It names the send's declaration (its digest), and:
+//   - from an ordered node-engine, one that reports the epoch and sequence it acted on, those are the send's
+//     own. A periodic report about an identical earlier send names the same digest and may arrive after a later
+//     send the machine never received; only the sequence tells them apart. It also holds for a fast machine
+//     whose report lands before the send's row is stamped.
+//   - from an engine that reports no order, the report came after the send — both times the store's own — so
+//     a machine sent again a declaration it once applied does not read as having applied the new send.
 func (i *Inventory) lastSends(ctx context.Context, where string, args ...any) (map[string]Send, error) {
 	rows, err := i.store.Pool().Query(ctx, `select distinct on (n.name) `+sendColumns+`,
 		       coalesce(r.outcome, ''), coalesce(r.refused, ''), case when jsonb_typeof(r.failed) = 'array' then jsonb_array_length(r.failed) else 0 end, r.at
 		  from declaration_send s join node n on n.id = s.node
 		  left join node_report r on r.node = s.node and r.declared <> '' and r.declared = s.digest
-		       and r.at >= s.sent_at
+		       and case when r.reported_sequence is not null
+		                 then r.reported_sequence = s.sequence and coalesce(r.reported_epoch, 0) = coalesce(s.epoch, 0)
+		                 else r.at >= s.sent_at end
 		 where s.recorded and `+where+`
 		 order by n.name, s.id desc`, args...)
 	if err != nil {

internal/inventory/sends_test.go

@@ -280,3 +280,69 @@ func TestTheLastSendSaysWhatTheMachineAnswered(t *testing.T) {
 		t.Error("a machine never sent anything has a last send")
 	}
 }
+
+// **A report about another declaration, or another send of the same one, is not the last send's answer**
+// (novox/hq issue 485, review of mesh-controller #234). A report naming an earlier send's digest that arrives
+// after a later send is not the later send's answer. And from an ordered node-engine, a periodic report about
+// an identical earlier send — the same digest, arriving after the later send — is not its answer either: only
+// the report's epoch and sequence say which send it is about.
+func TestAReportIsTheLastSendsAnswerOnlyWhenItIsAboutThatSend(t *testing.T) {
+	inv, node := aNodeWithModules(t)
+	ctx := t.Context()
+	record, err := inv.NodeByName(ctx, node)
+	if err != nil {
+		t.Fatal(err)
+	}
+	send := func(sequence int64, digest string) {
+		t.Helper()
+		if err := inv.RecordSentWith(ctx, record.ID, digest, nil, 57, 40, Send{Sequence: sequence, Epoch: 57,
+			Sender: "the controller's daemon (pid 7 on anchor)", Generation: 40, Digest: digest}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	ordered := func(epoch, sequence int64, digest string) {
+		t.Helper()
+		if _, err := inv.RecordOrderedDoing(ctx, record.ID, Doing{Outcome: OutcomeApplied, Declared: digest, Applied: 1},
+			ReportOrder{Epoch: epoch, Sequence: sequence}, func(ReportOrder) bool { return false }); err != nil {
+			t.Fatal(err)
+		}
+	}
+	answer := func() string {
+		t.Helper()
+		last, found, err := inv.LastSend(ctx, node)
+		if err != nil || !found {
+			t.Fatalf("the last send is not read back: %v, %v", found, err)
+		}
+		return last.Answer.Outcome
+	}
+
+	// An engine that reports no order: the digest decides.
+	send(11, "d11")
+	send(12, "d12")
+	if _, err := inv.RecordDoing(ctx, record.ID, Doing{Outcome: OutcomeApplied, Declared: "d11"}); err != nil {
+		t.Fatal(err)
+	}
+	if a := answer(); a != "" {
+		t.Fatalf("a report about d11, after d12 was sent, reads as d12's answer: %q", a)
+	}
+
+	// An ordered engine: the epoch and sequence decide, whatever the digest and the time.
+	send(20, "d20")
+	ordered(57, 20, "d20")
+	if a := answer(); a != OutcomeApplied {
+		t.Fatalf("an ordered report about sequence 20 is not its answer: %q", a)
+	}
+	send(21, "d20")
+	ordered(57, 20, "d20")
+	if a := answer(); a != "" {
+		t.Fatalf("a periodic report about sequence 20, after 21 was sent with the same digest, reads as 21's answer: %q", a)
+	}
+	ordered(56, 21, "d20")
+	if a := answer(); a != "" {
+		t.Fatalf("a report about sequence 21 of another epoch reads as this send's answer: %q", a)
+	}
+	ordered(57, 21, "d20")
+	if a := answer(); a != OutcomeApplied {
+		t.Fatalf("an ordered report about sequence 21 is not its answer: %q", a)
+	}
+}

testdata/store-tests

@@ -2,4 +2,5 @@
 # each one named here, by package, and fails unless every one passed — a test that needs the store skips without
 # one, and a skip passes unseen. One per line: <package> <TestName>. Lines starting with # are said nothing of.
 ./internal/inventory TestTheLastSendSaysWhatTheMachineAnswered
+./internal/inventory TestAReportIsTheLastSendsAnswerOnlyWhenItIsAboutThatSend
 ./cmd/mesh-controller TestNodeShowAndStatusReadTheLastSendFromTheStore