Write the hand act of register --reference after the registration was made, not before, and test the why and the record (issue 1888, C8b)

cmd/mesh-controller/register.go

@@ -79,6 +79,20 @@ func registerCommand(ctx context.Context, args []string) error {
 		return err
 	}
 	defer closeKeys()
+	return registerNow(ctx, open, keys, hand, module, repository, path, references, given)
+}
+
+// registerKeys is what a registration reads of the indexer and the builder (builderkeys.Reader, or a test's).
+type registerKeys interface {
+	keyReads
+	ModuleAt(ctx context.Context, module, commit string) (sdkdelivery.Module, error)
+}
+
+// registerNow is the registration once its arguments are read and the terminal is known: the module registered from its
+// key, or at the references given. **The hand act is written after the registration, and only when it was made** (ADR
+// 0526 clause 3): a take-in that waited or was refused did nothing by hand.
+func registerNow(ctx context.Context, open *stores, keys registerKeys, hand handActFlags, module, repository, path string,
+	references map[string]string, given []string) error {
 	trunk, err := keys.ModuleAt(ctx, module, "")
 	if errors.Is(err, builderkeys.ErrNoValue) {
 		return fmt.Errorf("the indexer holds no trunk value of %s: a module is registered from the builder's key of a "+
@@ -99,15 +113,6 @@ func registerCommand(ctx context.Context, args []string) error {
 		}
 		take.Hand = references
 	}
-	// And written down before it is done, with its why (novox/hq to-be 45 §7): a placement by hand is not undone by the
-	// next push, and the log says who put it there.
-	if take.Hand != nil {
-		act := []string{module, repository, positionalPath(path)}
-		for _, g := range given {
-			act = append(act, "--reference", g)
-		}
-		hand.record(ctx, "register", act)
-	}
 	manifest, kept, err := takeInKey(ctx, open.inventory, keys, take)
 	if errors.Is(err, errKeyWaits) {
 		return fmt.Errorf("%s is not registered yet, and nothing was asked: %v. Run register again once its key is done",
@@ -117,6 +122,13 @@ func registerCommand(ctx context.Context, args []string) error {
 		return err
 	}
 	if take.Hand != nil {
+		// Written down with its why (novox/hq to-be 45 §7) now that it is done: a placement by hand is not undone by the
+		// next push, and the log says who put it there.
+		act := []string{module, repository, positionalPath(path)}
+		for _, g := range given {
+			act = append(act, "--reference", g)
+		}
+		hand.record(ctx, "register", act)
 		fmt.Printf("%s %s registered at the references given (%s at %.8s), as asked at the controller's terminal, "+
 			"recorded as a build by hand; assigned nowhere\n", manifest.Module, manifest.Version, repository, kept.Commit)
 		return nil

cmd/mesh-controller/register_hand_integration_test.go

@@ -0,0 +1,65 @@
+//go:build integration
+
+package main
+
+import (
+	"context"
+	"flag"
+	"testing"
+	"time"
+
+	"github.com/nats-io/nats.go/jetstream"
+	"github.com/novox/mesh-controller/internal/builderkeys"
+	"github.com/novox/mesh-controller/internal/inventory"
+	"github.com/novox/mesh-controller/internal/link"
+)
+
+// C8b (ADR 0526 clause 3; issue 1888): `register --reference` writes its hand act, with the why, only once the module is
+// registered; a take-in that waits writes none.
+func TestARegistrationAtReferencesWritesItsHandActOnlyWhenItWasMade(t *testing.T) {
+	open := theCatalogue(t)
+	ctx := t.Context()
+	b := aBuilderOnTheBus(t)
+	if err := b.js.EnsureControllerBuckets(); err != nil {
+		t.Fatal(err)
+	}
+	api, err := jetstream.New(b.js.Conn())
+	if err != nil {
+		t.Fatal(err)
+	}
+	keys := builderkeys.Reader{JS: api}
+	fp := fpOf('8')
+	b.indexed(t, "sudo", "7", "novox/mesh-catalog", "modules/sudo", "c0ffee0123456789", fp)
+	ref := "artifact-store://sudo/a/blobs/" + shaOf("hand")
+	was := handStoreHolds
+	handStoreHolds = func(context.Context, *inventory.Inventory, string) error { return nil }
+	t.Cleanup(func() { handStoreHolds = was })
+	set := flag.NewFlagSet("register", flag.ContinueOnError)
+	hand := addHandActFlags(set)
+	if err := set.Parse([]string{"--why", "the builder cannot build it"}); err != nil {
+		t.Fatal(err)
+	}
+	acts := func() []link.HandAct {
+		got, err := link.HandActs(ctx, b.js.Conn(), time.Now().Add(-time.Minute))
+		if err != nil {
+			t.Fatal(err)
+		}
+		return got
+	}
+	given := []string{"a=" + ref}
+
+	withTrunkList(t, false) // the list at the catalogue's trunk head cannot be read: the take-in waits
+	err = registerNow(ctx, open, keys, hand, "sudo", "novox/mesh-catalog", "modules/sudo", map[string]string{"a": ref}, given)
+	if err == nil || len(acts()) != 0 {
+		t.Fatalf("a take-in that waited: %v, and %d hand act(s)", err, len(acts()))
+	}
+
+	withTrunkList(t, true)
+	if err := registerNow(ctx, open, keys, hand, "sudo", "novox/mesh-catalog", "modules/sudo", map[string]string{"a": ref}, given); err != nil {
+		t.Fatal(err)
+	}
+	got := acts()
+	if len(got) != 1 || got[0].Verb != "register" || got[0].Why != "the builder cannot build it" {
+		t.Fatalf("the hand act of a registration made: %+v", got)
+	}
+}

cmd/mesh-controller/register_test.go

@@ -118,3 +118,12 @@ func TestAHandReferenceReplacesOneArtifactOfTheBuildersKey(t *testing.T) {
 		t.Errorf("Made: %v", got)
 	}
 }
+
+// C8b (ADR 0526 clause 3): a registration at references refuses without a why, before anything is opened or done.
+func TestARegistrationAtReferencesRefusesWithoutAWhy(t *testing.T) {
+	ref := "artifact-store://x/tools/blobs/" + shaOf("t")
+	err := registerCommand(context.Background(), []string{"x", "someone/app", ".", "--reference", "tools=" + ref})
+	if err == nil || !strings.Contains(err.Error(), "--why") || !strings.Contains(err.Error(), "Nothing was done") {
+		t.Fatalf("a registration at references with no why: %v", err)
+	}
+}