Hold a joining module at the build it ran, own its directories by its own group, and take a library only when its act is done (hq ADR 0328 review, ADR 0334)

A held module is composed exactly as before it joined, at the build its machine runs, so its port, resources and contributions stay. A joining module's directories belong to its user and its own group, and its tools' directories are refused closed to that group; the runner joins it where the engine can say so. A take records the root and is taken only when the engine states the act done. Libraries' paths and verdicts count modules pulled in by a requirement, and a unit takes the identity.
2026-10-13 00:03:18 +02:00
parent 7ea6d36eab
commit 96e15f8693
16 changed files with 763 additions and 106 deletions
@@ -3,6 +3,7 @@ package inventory
 import (
 	"encoding/json"
 	"fmt"
+	"sort"
 	"strings"
 	"testing"

@@ -157,17 +158,17 @@ func TestALibraryIsDefinedOnceForTheMeshAndNeverOverlaps(t *testing.T) {
 func TestALibraryIsTakenOnceUnderAWarrant(t *testing.T) {
 	inv := fresh(t)
 	ctx := t.Context()
-	if err := inv.RecordTaken(ctx, TakenLibrary{Library: "media", Home: "ace", By: "the operator"}); err == nil {
+	if err := inv.RecordTakeStarted(ctx, TakenLibrary{Library: "media", Home: "ace", By: "the operator"}); err == nil {
 		t.Fatal("a library was recorded taken with no warrant")
 	}
-	first := TakenLibrary{Library: "media", Home: "ace", Placed: "fs-000000000001", By: "the operator", Ask: "c01",
-		Warrant: "c01"}
-	if err := inv.RecordTaken(ctx, first); err != nil {
+	first := TakenLibrary{Library: "media", Home: "ace", Root: "/storage/media", Placed: "fs-000000000001",
+		By: "the operator", Ask: "c01", Warrant: "c01"}
+	if err := inv.RecordTakeStarted(ctx, first); err != nil {
 		t.Fatal(err)
 	}
 	again := first
 	again.Ask, again.Warrant, again.By = "c02", "c02", "someone else"
-	if err := inv.RecordTaken(ctx, again); err != nil {
+	if err := inv.RecordTakeStarted(ctx, again); err != nil {
 		t.Fatal(err)
 	}
 	taken, err := inv.TakenLibraries(ctx)
@@ -179,12 +180,25 @@ func TestALibraryIsTakenOnceUnderAWarrant(t *testing.T) {
 	}
 	moved := first
 	moved.Placed, moved.Ask, moved.Warrant = "fs-0000000000ff", "c03", "c03"
-	if err := inv.RecordTaken(ctx, moved); err != nil {
+	if err := inv.RecordTakeStarted(ctx, moved); err != nil {
 		t.Fatal(err)
 	}
 	if taken, err = inv.TakenLibraries(ctx); err != nil || taken["media"].Ask != "c03" {
 		t.Fatalf("a take on another filesystem: %+v (%v)", taken["media"], err)
 	}
+	// The review of 7ea6d36e, item 3: the root is part of what was taken — a library whose root changed is taken
+	// again, and the new take replaces the old.
+	if got := taken["media"].Root; got != "/storage/media" {
+		t.Fatalf("the take recorded the root %q", got)
+	}
+	rooted := moved
+	rooted.Root, rooted.Ask, rooted.Warrant = "/storage/media2", "c04", "c04"
+	if err := inv.RecordTakeStarted(ctx, rooted); err != nil {
+		t.Fatal(err)
+	}
+	if taken, err = inv.TakenLibraries(ctx); err != nil || taken["media"].Ask != "c04" || taken["media"].Root != "/storage/media2" {
+		t.Fatalf("a take at another root: %+v (%v)", taken["media"], err)
+	}
 }

 // novox/hq issue 575: a module registered without the access movies, whose newest build on its branch declares it —
@@ -298,3 +312,82 @@ func TestOnlyABuildOfTheFollowedBranchJudgesAPlacementAhead(t *testing.T) {
 		t.Fatalf("a module with no branch recorded: %v", err)
 	}
 }
+
+// The review of 7ea6d36e, item 4: every sub-path a module asks of a library is kept on its home, whether the module
+// was assigned or pulled in by a requirement (it is in what its machine was last sent).
+func TestTheSubPathsOfModulesPulledInByARequirementAreKept(t *testing.T) {
+	inv := fresh(t)
+	ctx := t.Context()
+	record, err := inv.AddNode(ctx, "ace")
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, m := range []catalogue.Manifest{
+		{Module: "radarr", Version: "1", Libraries: []catalogue.LibraryUse{{ID: "m", Library: "media", Path: "movies", Use: "read-write"}}},
+		{Module: "importer", Version: "1", Libraries: []catalogue.LibraryUse{{ID: "i", Library: "media", Path: "incoming", Use: "read-write"}}},
+	} {
+		if err := inv.RegisterModule(ctx, m, Source{}); err != nil {
+			t.Fatal(err)
+		}
+	}
+	if _, err := inv.Assign(ctx, "ace", "radarr"); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.RecordSentWith(ctx, record.ID, "d1", map[string]string{"radarr": "c1", "importer": "c2"}, 1, 1,
+		Send{Sequence: 1}); err != nil {
+		t.Fatal(err)
+	}
+	paths, err := inv.LibraryPaths(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	got := append([]string{}, paths["media"]...)
+	sort.Strings(got)
+	if strings.Join(got, ",") != "incoming,movies" {
+		t.Fatalf("the paths kept of media are %v; want the importer's, pulled in by a requirement, too", got)
+	}
+}
+
+// The review of 7ea6d36e, item 5: a take the engine says it started is recorded as started, not taken; it is taken when
+// the engine states the act done under that warrant, and forgotten when it states it failed, so the operator is asked
+// again.
+func TestALibraryIsTakenWhenTheActIsDone(t *testing.T) {
+	inv := fresh(t)
+	ctx := t.Context()
+	started := TakenLibrary{Library: "media", Home: "ace", Root: "/storage/media", Placed: "fs-000000000001",
+		By: "the operator", Ask: "c01", Warrant: "c01"}
+	if err := inv.RecordTakeStarted(ctx, started); err != nil {
+		t.Fatal(err)
+	}
+	taken, err := inv.TakenLibraries(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := taken["media"]; got.Done || got.Ask != "c01" {
+		t.Fatalf("a take only started reads %+v; want started, not done", got)
+	}
+	if err := inv.TakeDone(ctx, "media", "c99"); err != nil {
+		t.Fatal(err)
+	}
+	if taken, _ = inv.TakenLibraries(ctx); taken["media"].Done {
+		t.Fatal("another warrant's act made the take done")
+	}
+	if err := inv.TakeDone(ctx, "media", "c01"); err != nil {
+		t.Fatal(err)
+	}
+	if taken, _ = inv.TakenLibraries(ctx); !taken["media"].Done {
+		t.Fatal("the act done under the take's warrant did not make it taken")
+	}
+	// A failed act forgets a take not done, so it is asked again; never one done.
+	again := started
+	again.Root, again.Ask, again.Warrant = "/storage/media2", "c02", "c02"
+	if err := inv.RecordTakeStarted(ctx, again); err != nil {
+		t.Fatal(err)
+	}
+	if err := inv.TakeFailed(ctx, "media", "c02"); err != nil {
+		t.Fatal(err)
+	}
+	if taken, _ = inv.TakenLibraries(ctx); taken["media"].Ask == "c02" {
+		t.Fatalf("a failed take is still recorded: %+v", taken["media"])
+	}
+}