The copy of sent_builds never revives an ended key; a send empties its node's column (issue 1109)

A found ended or pending key is skipped and a pulled-in key is created only where no key exists. RecordSentWith and RecordSentUnder set sent_builds = null in their own update of the node row, so a machine delivery 2 has sent is never copied afterwards. These two updates and the copy's own null are the only writes to the column.

internal/inventory/chain_links.go

@@ -309,8 +309,8 @@ func (i *Inventory) CopySentBuilds(ctx context.Context) (int, error) {
 }
 
 // copySentBuildsOf is CopySentBuilds for one node, over a chain: for each module the node was sent, a standing key that
-// names no commit gets it (and the build, when one is known; no rollout), and where no key stands a standing pulled-in key is
-// made. A key with a commit, a hand send's unknown and a wait for a build are left as they are.
+// names no commit gets it (and the build, when one is known; no rollout), and where no key exists a standing pulled-in key is
+// made. A key with a commit, an ended key, a hand send's unknown and a wait for a build are left as they are.
 func (i *Inventory) copySentBuildsOf(ctx context.Context, c *chain.Chain, node string, sent map[string]string, at time.Time) error {
 	for _, module := range slices.Sorted(maps.Keys(sent)) {
 		commit := sent[module]
@@ -330,11 +330,11 @@ func (i *Inventory) copySentBuildsOf(ctx context.Context, c *chain.Chain, node s
 			if err != nil {
 				return err
 			}
-			if found && (cur.Value.State != "" || cur.Value.Commit != "" || !cur.Value.SentUnknown.IsZero()) {
-				break
+			if found && (cur.Value.State != "" || !cur.Value.Ended.IsZero() || cur.Value.Commit != "" || !cur.Value.SentUnknown.IsZero()) {
+				break // a wait, an ended key (never revived), one that names a commit, or a hand send's unknown
 			}
 			next := cur.Value
-			if !found || !cur.Value.Stands() {
+			if !found {
 				next = chain.Assignment{Node: node, Module: module, PulledIn: true, Since: at}
 			}
 			next.Commit, next.Build = commit, build

internal/inventory/chain_sent_copy_test.go

@@ -59,8 +59,8 @@ func TestWhatTheColumnRecordedIsCopiedOntoTheKeysAndOverwritesNothing(t *testing
 		if a := get("mesh-wireguard"); a.Commit != "c-wg" || a.Build != "b-wg" || !a.PulledIn || !a.Stands() || a.Rollout != "" {
 			t.Errorf("the pulled-in module has no standing key of its own: %+v", a)
 		}
-		if a := get("ended"); a.Commit != "c-ended" || !a.Stands() || !a.PulledIn {
-			t.Errorf("an ended key is not made to stand again as pulled in: %+v", a)
+		if a := get("ended"); a.Commit != "" || a.Stands() {
+			t.Errorf("an ended key was revived or given a commit: %+v", a)
 		}
 	}
 }
@@ -88,3 +88,24 @@ func TestTheColumnOfASentMachineIsCopiedOntoItsKeysAndEmptied(t *testing.T) {
 		t.Fatalf("a second copy did %d nodes (%v)", n, err)
 	}
 }
+
+// A machine delivery 2 has sent has its column emptied by the send itself, so a copy after it leaves a key the send cleared
+// without a commit.
+func TestASendEmptiesTheColumnSoALaterCopyLeavesAClearedKeyBare(t *testing.T) {
+	inv := ForTest(t)
+	ctx := t.Context()
+	record, err := inv.AddNode(ctx, "laptop")
+	if err != nil {
+		t.Fatal(err)
+	}
+	inv.ExecForTest(t, `update node set sent_builds = '{"notes": "c-old"}'::jsonb where name = 'laptop'`)
+	if err := inv.RecordSentUnder(ctx, record.ID, "d1", map[string]string{}, 1); err != nil {
+		t.Fatal(err)
+	}
+	if rows := inv.QueryForTest(t, `select count(*) from node where sent_builds is not null`); rows[0][0] != "0" {
+		t.Fatalf("a send left the column: %v", rows)
+	}
+	if n, err := inv.CopySentBuilds(ctx); err != nil || n != 0 {
+		t.Fatalf("the copy did %d nodes (%v)", n, err)
+	}
+}

internal/inventory/nodes.go

@@ -1263,7 +1263,7 @@ func (i *Inventory) RecordSentUnder(ctx context.Context, node, digest string, bu
 		sentEpoch = &e
 	}
 	if _, err := i.store.Pool().Exec(ctx,
-		`update node set sent = $2, sent_at = now(), sent_epoch = $3 where id = $1`, node, digest, sentEpoch); err != nil {
+		`update node set sent = $2, sent_at = now(), sent_builds = null, sent_epoch = $3 where id = $1`, node, digest, sentEpoch); err != nil {
 		return err
 	}
 	// What it was sent of each module is the assignments' (SentBuilds); a write of it that fails fails this record.

internal/inventory/sends.go

@@ -155,7 +155,7 @@ func (i *Inventory) RecordSentWith(ctx context.Context, node, digest string, bui
 	}
 	defer func() { _ = tx.Rollback(ctx) }()
 	if _, err := tx.Exec(ctx,
-		`update node set sent = $2, sent_at = now(), sent_epoch = $3, sent_generation = $4,
+		`update node set sent = $2, sent_at = now(), sent_builds = null, sent_epoch = $3, sent_generation = $4,
 		        sent_provisioning = $5::jsonb
 		  where id = $1`, node, digest, sentEpoch, nullIfZero(generation), nullIfEmpty(s.Provisioning)); err != nil {
 		return err

testdata/store-tests

@@ -160,3 +160,4 @@
 ./internal/inventory TestMigration0105PassesAStoreThatNeverHeldARow
 ./cmd/mesh-controller TestNoTableOfTheControllersStoreHoldsBuildsRolloutsOrAssignments
 ./internal/inventory TestTheColumnOfASentMachineIsCopiedOntoItsKeysAndEmptied
+./internal/inventory TestASendEmptiesTheColumnSoALaterCopyLeavesAClearedKeyBare