Judge a recorded build, not build-agent's outcome, and read a registration's warrant from the merge a key names, so a done key registers under the same rules (issue 1851, A4a)

cmd/mesh-controller/build.go

@@ -679,13 +679,6 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu
 			recorded.Ref = followedBranch(was.Ref)
 		}
 	}
-	if err := namesNoInstallation(manifest); err != nil {
-		return manifest, kept, fmt.Errorf("%s built %s (%s), and the mesh does not register it: %w",
-			result.On, result.Repository, short(result.Commit), err)
-	}
-	// **Only from the repository the catalogue builds the module from** (novox/hq ADR 0266): else the trunk
-	// below is the trunk of whatever repository was built, which may be one an agent made — and a module named
-	// `sudo` from it would be what the next push sends. Another repository is the operator's, at the terminal.
 	trunk := result.Trunk
 	if was, err := inv.SourceOf(ctx, manifest.Module); err == nil && followedBranch(was.Ref) != "" {
 		trunk = followedBranch(was.Ref)
@@ -693,55 +686,111 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu
 	if trunk == "" {
 		trunk = "main"
 	}
-	repoID, err := mayRegisterFrom(ctx, inv, manifest.Module, recorded, result.ID, result.Path, trunk)
+	j := takenBuild{kept: kept, manifest: manifest, recorded: recorded, ask: byBuild(result.ID), path: result.Path,
+		trunk: trunk, by: result.On, what: result.Repository, commit: result.Commit}
+	// The checks only an outcome of build-agent needs: a key is a trunk build by construction (ADR 0501 §2), in no
+	// scratch namespace, and carries no list of its own.
+	j.published = func() error {
+		// **Only a commit on the trunk is published** (novox/hq ADR 0238): a commit off its repository's
+		// default branch — a pull request's head, a feature branch built by hand, a `rebuild` or `replay
+		// --register` of one — is for checking, and is never a module's version; nothing could then send it.
+		// The branch the module already follows is its trunk — never the branch a build was asked at, or a
+		// build of a feature branch by name would make that branch its trunk.
+		follows := ""
+		if was, err := inv.SourceOf(ctx, manifest.Module); err == nil {
+			follows = followedBranch(was.Ref)
+		}
+		if err := publishable(result, follows); err != nil {
+			return err
+		}
+		// **Never a build in the scratch namespace** (novox/hq ADR 0239 §9, ADR 0350 §2): what a check builds is
+		// registered only as the walk's copy in the module's own namespace.
+		if scratch := inScratch(result.Made); scratch != "" {
+			return fmt.Errorf("%s is in the scratch namespace, which only a walk's copy leaves (ADR 0350)", scratch)
+		}
+		return nil
+	}
+	j.carried = func() error {
+		// **The newest trunk build's list of predefined keys is published** (novox/hq ADR 0348 §7): a build of the
+		// catalogue that may be registered keeps its list when its commit is newer than the one kept.
+		carried, err := carriedList(result)
+		if err != nil {
+			return err
+		}
+		if carried != nil {
+			if err := publishPredefined(ctx, inv, result, carried); err != nil {
+				fmt.Printf("%s: the list of predefined keys it carried is not published: %v\n", result.ID, err)
+			}
+		}
+		return nil
+	}
+	manifest, err = registerJudged(ctx, inv, j)
+	return manifest, kept, err
+}
+
+// takenBuild is a build the controller judges and registers, whatever told it of the build: build-agent's outcome (the
+// builder's own module's, until R4g, ADR 0453 §5) or the builder's done key (ADR 0501 §1). **One judging for both**:
+// the rules of who may register, the gate a build failed, the predefined keys and the rule that an unchanged build
+// source moves nothing are the same whichever part built it (ADR 0501 §1, §3).
+type takenBuild struct {
+	kept     inventory.Build
+	manifest catalogue.Manifest
+	recorded inventory.Source
+	ask      registrationAsk
+	path     string
+	trunk    string
+	// by and what name the build in what is said: who built it, and what was built ("ace", "novox/mesh-catalog").
+	by, what, commit string
+	// published refuses a build that may not become a module's version (off the trunk, in the scratch namespace);
+	// nil for a key, which is a trunk fingerprint's by construction (ADR 0501 §2).
+	published func() error
+	// carried publishes a list of predefined keys the build carried; nil when it carried none.
+	carried func() error
+}
+
+// registerJudged judges a recorded build and registers its module, or says why not.
+func registerJudged(ctx context.Context, inv *inventory.Inventory, j takenBuild) (catalogue.Manifest, error) {
+	manifest, kept := j.manifest, j.kept
+	notRegistered := func(err error) (catalogue.Manifest, error) {
+		return manifest, fmt.Errorf("%s built %s (%s), recorded and not registered: %w", j.by, manifest.Module,
+			short(j.commit), err)
+	}
+	if err := namesNoInstallation(manifest); err != nil {
+		return manifest, fmt.Errorf("%s built %s (%s), and the mesh does not register it: %w",
+			j.by, j.what, short(j.commit), err)
+	}
+	// **Only from the repository the catalogue builds the module from** (novox/hq ADR 0266): else the trunk
+	// below is the trunk of whatever repository was built, which may be one an agent made — and a module named
+	// `sudo` from it would be what the next push sends. Another repository is the operator's, at the terminal.
+	repoID, err := mayRegisterFrom(ctx, inv, manifest.Module, j.recorded, j.ask, j.path, j.trunk)
 	if err != nil {
-		return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: %w", result.On,
-			manifest.Module, short(result.Commit), err)
+		return notRegistered(err)
 	}
-	// **Only a commit on the trunk is published** (novox/hq ADR 0238): a commit off its repository's
-	// default branch — a pull request's head, a feature branch built by hand, a `rebuild` or `replay
-	// --register` of one — is for checking, and is never a module's version; nothing could then send it.
-	// The branch the module already follows is its trunk — never the branch a build was asked at, or a
-	// build of a feature branch by name would make that branch its trunk.
-	follows := ""
-	if was, err := inv.SourceOf(ctx, manifest.Module); err == nil {
-		follows = followedBranch(was.Ref)
-	}
-	if err := publishable(result, follows); err != nil {
-		return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: %w", result.On,
-			manifest.Module, short(result.Commit), err)
-	}
-	// **Never a build in the scratch namespace** (novox/hq ADR 0239 §9, ADR 0350 §2): what a check builds is
-	// registered only as the walk's copy in the module's own namespace.
-	if scratch := inScratch(result.Made); scratch != "" {
-		return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: %s is in the scratch "+
-			"namespace, which only a walk's copy leaves (ADR 0350)", result.On, manifest.Module, short(result.Commit), scratch)
+	if j.published != nil {
+		if err := j.published(); err != nil {
+			return notRegistered(err)
+		}
 	}
 	// **A build that failed its gate is never registered again** (novox/hq ADR 0236): an outcome heard
 	// twice, or replayed, would otherwise make the build a rollback put back what the module is again,
 	// and the next push would send it.
 	if failed, err := inv.GateFailed(ctx, kept.ID); err != nil {
-		return manifest, kept, err
+		return manifest, err
 	} else if failed {
-		return manifest, kept, fmt.Errorf("%s built %s (%s), which failed its gate on its first machine and was put "+
-			"back: it is recorded and not registered again — a newer build is", result.On, manifest.Module,
-			short(result.Commit))
+		return manifest, fmt.Errorf("%s built %s (%s), which failed its gate on its first machine and was put "+
+			"back: it is recorded and not registered again — a newer build is", j.by, manifest.Module,
+			short(j.commit))
 	}
-	// **The newest trunk build's list of predefined keys is published** (novox/hq ADR 0348 §7): a build of the
-	// catalogue that may be registered keeps its list when its commit is newer than the one kept. Then the module
-	// is judged against the list the mesh publishes, as every manifest read again is: one naming a key that list
-	// does not hold, or of another type, is refused here and never enters the catalogue.
-	if carried, err := carriedList(result); err != nil {
-		return manifest, kept, err
-	} else if carried != nil {
-		if err := publishPredefined(ctx, inv, result, carried); err != nil {
-			fmt.Printf("%s: the list of predefined keys it carried is not published: %v\n", result.ID, err)
+	// Then the module is judged against the list the mesh publishes, as every manifest read again is: one naming a
+	// key that list does not hold, or of another type, is refused here and never enters the catalogue.
+	if j.carried != nil {
+		if err := j.carried(); err != nil {
+			return manifest, err
 		}
 	}
 	if manifest.NamesPredefined() {
 		if _, err := catalogue.ParseManifest(kept.Manifest); err != nil {
-			return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: judged against the "+
-				"list of predefined keys the mesh publishes, %w", result.On, manifest.Module, short(result.Commit), err)
+			return notRegistered(fmt.Errorf("judged against the list of predefined keys the mesh publishes, %w", err))
 		}
 	}
 	// **A build whose source is unchanged is never a move** (novox/hq issue 280): a rebuild made from
@@ -751,35 +800,34 @@ func takeIn(ctx context.Context, inv *inventory.Inventory, result link.BuildResu
 	if kept.SourceFingerprint != "" {
 		stands, raw, err := inv.StandingBuild(ctx, manifest.Module, kept.ID)
 		if err != nil {
-			return manifest, kept, err
+			return manifest, err
 		}
 		if stands != "" && stands != kept.ID && len(raw) > 0 {
-			if same, err := catalogue.ParseManifest(raw); err == nil && same.Module == manifest.Module {
+			if was, err := catalogue.ParseManifest(raw); err == nil && was.Module == manifest.Module {
 				fmt.Printf("%s at %s was made from the source %s was: registered with its artifacts, no move\n",
-					manifest.Module, short(result.Commit), stands)
-				manifest = same
+					manifest.Module, short(j.commit), stands)
+				manifest = was
 			}
 		}
 	}
-	if err := inv.RegisterModule(ctx, manifest, recorded); err != nil {
+	if err := inv.RegisterModule(ctx, manifest, j.recorded); err != nil {
 		if errors.Is(err, inventory.ErrSuperseded) {
-			return manifest, kept, fmt.Errorf("%s built %s (%s), recorded and not registered: %w",
-				result.On, manifest.Module, short(result.Commit), err)
+			return notRegistered(err)
 		}
-		return manifest, kept, err
+		return manifest, err
 	}
 	// A bus process the new definition gains gets its account now, on each machine assigned the module, so the
 	// next push composes a real account (issue 1226).
 	issueGainedProcessAccounts(ctx, inv, manifest)
 	// Which repository it is registered from, by the forge's own id (novox/hq ADR 0266).
 	if err := inv.SetSourceIdentity(ctx, manifest.Module, repoID); err != nil {
-		return manifest, kept, err
+		return manifest, err
 	}
 	// The keep set just moved, and new bytes just landed (novox/hq ADR 0189). Asked here rather
 	// than on a timer of its own: this is the only moment either is true. Never fatal — the build
 	// worked and the module is registered.
 	collect(ctx, inv)
-	return manifest, kept, nil
+	return manifest, nil
 }
 
 // inScratch is the first artifact a build made into the scratch namespace; empty when none is.

cmd/mesh-controller/build_source.go

@@ -469,8 +469,39 @@ func (f *sourceForms) onTheForge(repository, seat string) (owner, name string, o
 // repository and path; or, for the build of a merge, when mesh-delivery announced it on the operator's Approve of
 // that head (novox/hq ADR 0420 §7 for a moved module, ADR 0432 for a new one), the forge's checks still holding. path is the module's directory as built; branch the trunk it is registered from.
 func mayRegisterFrom(ctx context.Context, inv *inventory.Inventory, module string, built inventory.Source,
-	buildID, path, branch string) (int64, error) {
-	return mayRegisterIn(ctx, inv, newSourceForms(ctx, inv), module, built, buildID, path, branch)
+	ask registrationAsk, path, branch string) (int64, error) {
+	return mayRegisterIn(ctx, inv, newSourceForms(ctx, inv), module, built, ask, path, branch)
+}
+
+// registrationAsk is what a registration's warrant is read from (novox/hq ADR 0501 §3): the build an outcome answers,
+// whose kept request says whether it was asked at the terminal or for a merge (build-agent's outcome, the builder's own
+// module until R4g); or, for a take-in from the builder's done key, the merge the indexer's trunk value names and
+// whether the operator asked the registration at the controller's terminal (ADR 0501 §4). A key asks no build, so it
+// keeps no request.
+type registrationAsk struct {
+	// Build is the build whose kept request is read; empty for a take-in from a key.
+	Build string
+	// Merge is the commit of the merge a key's module is registered from (the indexer's trunk value); empty for an
+	// outcome, whose request names it.
+	Merge string
+	// Terminal is a key registration the operator asked at the controller's terminal for this repository and path.
+	Terminal bool
+}
+
+// byBuild is the warrant of a build's outcome: read from the request kept under its id.
+func byBuild(id string) registrationAsk { return registrationAsk{Build: id} }
+
+// String is the ask as the registration's words name it.
+func (a registrationAsk) String() string {
+	switch {
+	case a.Build != "":
+		return a.Build
+	case a.Terminal:
+		return "the registration asked at the terminal"
+	case a.Merge != "":
+		return fmt.Sprintf("the merge %.8s", a.Merge)
+	}
+	return "a key"
 }
 
 // catalogueReader is the catalogue's manifests, as the router's records are found in them.
@@ -490,15 +521,19 @@ type registrations interface {
 
 // mayRegisterIn is mayRegisterFrom on what it reads of the mesh handed in.
 func mayRegisterIn(ctx context.Context, inv registrations, forms *sourceForms, module string, built inventory.Source,
-	buildID, path, branch string) (int64, error) {
+	ask registrationAsk, path, branch string) (int64, error) {
+	buildID := ask.String()
 	was, err := inv.SourceOf(ctx, module)
 	isNew := errors.Is(err, inventory.ErrNoSuchModule)
 	if err != nil && !isNew {
 		return 0, err
 	}
-	terminal, err := askedHereFor(ctx, inv, forms, buildID, built, path)
-	if err != nil {
-		return 0, err
+	terminal := ask.Terminal
+	if ask.Build != "" {
+		var err error
+		if terminal, err = askedHereFor(ctx, inv, forms, ask.Build, built, path); err != nil {
+			return 0, err
+		}
 	}
 	refuse := func(why string) (int64, error) {
 		return 0, fmt.Errorf("%w: %s. A module is registered from such a source only by a build asked at the "+
@@ -542,7 +577,7 @@ func mayRegisterIn(ctx context.Context, inv registrations, forms *sourceForms, m
 	// The forge's protection and the repository's identity are checked below as for every registration.
 	movedWhy := ""
 	if why != "" && !terminal {
-		accepted, err := approvedMove(ctx, inv, forms, buildID, built)
+		accepted, err := approvedMove(ctx, inv, forms, ask, built)
 		switch {
 		case err == nil:
 			movedWhy, why = accepted, ""

cmd/mesh-controller/forge_address_test.go

@@ -64,7 +64,7 @@ func registerFrom(t *testing.T, forms *sourceForms) (int64, error) {
 	mesh := &aMeshRegisteredFromTheURL{anEmptyMesh{asked: true,
 		request: inventory.BuildRequest{ID: "build-1", Repository: aRepoOnTheForge, For: "build"}}, aRepoOnTheForge}
 	return mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-view", inventory.Source{Repository: aRepoOnTheForge},
-		"build-1", "", "main")
+		byBuild("build-1"), "", "main")
 }
 
 // 1277/AC2: an address unread twice, then read, is waited out and the forge is asked.
@@ -112,7 +112,7 @@ func TestAKnownAddressAndAnotherHostIsStillNotOnTheForge(t *testing.T) {
 	const elsewhere = "http://elsewhere.example/x/y.git"
 	mesh := &aMeshRegisteredFromTheURL{anEmptyMesh{asked: true, request: inventory.BuildRequest{ID: "build-1", For: "build"}}, elsewhere}
 	_, err := mayRegisterIn(t.Context(), mesh, sourceFormsRead(context.Background(), time.Now, read), "mesh-delivery-view",
-		inventory.Source{Repository: elsewhere}, "build-1", "", "main")
+		inventory.Source{Repository: elsewhere}, byBuild("build-1"), "", "main")
 	if !errors.Is(err, errNotItsSource) || !strings.Contains(err.Error(), "is not on the mesh's forge") {
 		t.Fatalf("1277/AC4: the refusal changed: %v", err)
 	}

cmd/mesh-controller/moved.go

@@ -265,25 +265,32 @@ var errNotAMerge = errors.New("the build is not of a merge")
 // (ADR 0420 §7): the build is of a merge, that merge is a pull request mesh-delivery announced it made, and the
 // ask it made it on holds an Approve by the operator of that head. Answers why it was accepted; an error says why
 // it was not, and the refusal of ADR 0266 stands.
-func approvedMove(ctx context.Context, inv registrations, forms *sourceForms, buildID string,
+func approvedMove(ctx context.Context, inv registrations, forms *sourceForms, ask registrationAsk,
 	built inventory.Source) (string, error) {
-	r, found, err := inv.BuildRequestByID(ctx, buildID)
-	if err != nil {
-		return "", err
+	commit := ask.Merge
+	if ask.Build != "" {
+		r, found, err := inv.BuildRequestByID(ctx, ask.Build)
+		if err != nil {
+			return "", err
+		}
+		if !found || r.For != "merge" || r.Commit == "" {
+			return "", errNotAMerge
+		}
+		commit = r.Commit
 	}
-	if !found || r.For != "merge" || r.Commit == "" {
+	if commit == "" {
 		return "", errNotAMerge
 	}
 	owner, name, onForge := forms.onTheForge(built.Repository, built.Seat)
 	if !onForge {
 		return "", errors.New("the repository is not on the mesh's forge")
 	}
-	merged, known, err := inv.MergeOf(ctx, owner+"/"+name, r.Commit)
+	merged, known, err := inv.MergeOf(ctx, owner+"/"+name, commit)
 	if err != nil {
 		return "", err
 	}
 	if !known {
-		return "", fmt.Errorf("the controller kept no merge of %s/%s at %.8s", owner, name, r.Commit)
+		return "", fmt.Errorf("the controller kept no merge of %s/%s at %.8s", owner, name, commit)
 	}
 	return approvedAnnounced(ctx, inv, owner, name, announcedOf(merged))
 }

cmd/mesh-controller/new_module_approved_test.go

@@ -116,7 +116,7 @@ func TestANewModuleIsRegisteredOnTheOperatorsApproveOfItsMergeAndRefusedOtherwis
 			if c.built != nil {
 				src = *c.built
 			}
-			id, err := mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-view", src, "build-1", "", "main")
+			id, err := mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-view", src, byBuild("build-1"), "", "main")
 			switch {
 			case c.accepted && err != nil:
 				t.Fatalf("%s: the Approved merge's new module was refused: %v", c.ac, err)
@@ -157,7 +157,7 @@ func TestASecondNewModuleFromARepositoryIsJudgedAlongsideAndAsksNoWarrant(t *tes
 		request: inventory.BuildRequest{ID: "build-2", Repository: "novox/mesh-delivery", Seat: "git", For: "build"}}}
 	forms := &sourceForms{bases: map[string]string{}, base: func(string) string { return "http://forge.internal:20000" }}
 	built := inventory.Source{Repository: "novox/mesh-delivery", Seat: "git"}
-	if id, err := mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-watchman", built, "build-2", "", "main"); err != nil || id == 0 {
+	if id, err := mayRegisterIn(t.Context(), mesh, forms, "mesh-delivery-watchman", built, byBuild("build-2"), "", "main"); err != nil || id == 0 {
 		t.Fatalf("1146/AC5: a second module from a repository the catalogue builds from was refused (id %d): %v", id, err)
 	}
 }