Merge pull request 'A step gates its module, not the machine' (#41) from fix/a-step-gates-its-module-not-the-machine into main

internal/apply/apply.go

@@ -320,6 +320,9 @@ func ApplyKeeping(
 	// is a different thing — one is "this machine could not do it", the other is "this was never
 	// a declaration", and they are fixed in different places.
 	var failures []*Error
+	// Modules whose own step did not complete. What follows *within such a module* is not attempted;
+	// the rest of the machine is (novox/hq ADR 0136).
+	gated := map[string]bool{}
 	for i, resource := range ordered {
 		if !orphansRemoved && i == guardFirst {
 			// **Only a guard that is up may let the filter go.** Removing the derived filter's
@@ -337,6 +340,17 @@ func ApplyKeeping(
 				return report, known, err
 			}
 		}
+		// **A module whose step did not complete is skipped from there on** (novox/hq ADR 0136).
+		// Reported rather than passed over in silence: "not attempted" and "nothing to do" are
+		// different answers, and only one of them is somebody's to fix.
+		if module, ours := moduleOf(resource.Identity()); ours && gated[module] {
+			report.Outcomes = append(report.Outcomes, Outcome{
+				ID: resource.Identity(), Type: string(resource.Kind()), Target: resource.Target(),
+				Action: "skipped", Detail: "a step this module declares did not complete",
+			})
+			continue
+		}
+
 		// **On an adopted node, what is found is kept until its module is taken** (novox/hq ADR
 		// 0100, ADR 0103). Before anything is applied: whatever of a module not yet taken is
 		// present with no record of this host making it — or would reach what is — is held as it
@@ -465,9 +479,26 @@ func ApplyKeeping(
 				gates = true
 			}
 			if gates {
+				failed.Gated = true
+				// **A module's step gates that module, not the machine** (novox/hq ADR 0136).
+				//
+				// Stopping the whole apply is what this loop's own comment above calls holding a
+				// machine hostage, and it was already rejected for every other shape (04-ISSUES/011).
+				// A step exists to make something true before the next thing in *its module* needs it
+				// — a store seeded before the broker starts, a schema prepared before the version
+				// that needs it runs — so that is exactly how far the gate reaches. Everything else
+				// on the machine is independent state and is attempted.
+				//
+				// An action still gates the machine: the bootstrap is a row of them, each making the
+				// next possible, and they belong to no module.
+				if module, ours := moduleOf(resource.Identity()); ours {
+					gated[module] = true
+					log(fmt.Sprintf("  gated %s.*: a step it declares did not complete, so the rest "+
+						"of it was not attempted", module))
+					continue
+				}
 				failed.Done = report
 				failed.Others = len(failures) - 1
-				failed.Gated = true
 				return report, known, failed
 			}
 			continue
@@ -2247,3 +2278,19 @@ func meshMadeUnits(known store.State) map[string]bool {
 	}
 	return made
 }
+
+// moduleOf is the module a declared resource belongs to.
+//
+// The mesh composes a module's resource ids as `<module>.<id>`, so the part before the first dot is
+// the module. False for what the mesh declares in its own right — a guard, an opening, the adoption's
+// own resources — which belongs to no module, and whose gate is therefore the machine's.
+func moduleOf(identity string) (string, bool) {
+	if strings.HasPrefix(identity, declaration.AdoptionPrefix) {
+		return "", false
+	}
+	module, _, ok := strings.Cut(identity, ".")
+	if !ok || module == "" {
+		return "", false
+	}
+	return module, true
+}

internal/apply/runonce_test.go

@@ -316,3 +316,61 @@ func TestAContainerNamingARunOnceStepIsRecreatedWhenItRan(t *testing.T) {
 		t.Errorf("the recreation did not name the step as its reason: %+v", server)
 	}
 }
+
+func TestAFailedStepGatesItsModuleAndNotTheMachine(t *testing.T) {
+	// **The blast radius of a step is its module** (novox/hq ADR 0136). A step exists to make
+	// something true before the next thing in its own module needs it — a store seeded before the
+	// broker starts, a schema prepared before the version that needs it runs. Stopping the whole
+	// apply is what this host's own loop calls holding a machine hostage, and it was already
+	// rejected for every other shape (04-ISSUES/011): a module whose database is briefly
+	// unreachable must not stop every module declared after it.
+	var startedNames []string
+	run := func(ctx context.Context, name string, args ...string) (string, error) {
+		switch args[0] {
+		case "info":
+			return "27.0\n", nil
+		case "container":
+			return "false\t\n", errors.New("no such container")
+		case "run":
+			startedNames = append(startedNames, nameOf(args))
+			if nameOf(args) == "catalogue-prepare" {
+				return "", errors.New("exit status 1") // the schema could not be reached
+			}
+			return "deadbeef\n", nil
+		case "rm":
+			return "", nil
+		}
+		return "", nil
+	}
+	d := parseTrusted(t, `{"declaration":1,"resources":[
+	  {"id":"mesh-catalog.runtime.prepare","type":"container","name":"catalogue-prepare","image":"`+pinned+`","run-once":true},
+	  {"id":"mesh-catalog.runtime","type":"container","name":"catalogue","image":"`+pinned+`"},
+	  {"id":"gitea.server","type":"container","name":"forge","image":"`+pinned+`"}
+	]}`)
+
+	report, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginCarried, run, nil, nil)
+	if err == nil {
+		t.Fatal("a failed step was not reported as a failure")
+	}
+	started := map[string]bool{}
+	for _, n := range startedNames {
+		started[n] = true
+	}
+	if started["catalogue"] {
+		t.Error("the module's own workload ran although its step did not complete")
+	}
+	if !started["forge"] {
+		t.Error("another module was not attempted, so one module's step held the machine hostage")
+	}
+	// And the machine's own account says which was not attempted, rather than leaving it to be
+	// inferred from silence.
+	var skipped string
+	for _, o := range report.Outcomes {
+		if o.Action == "skipped" {
+			skipped = o.ID
+		}
+	}
+	if skipped != "mesh-catalog.runtime" {
+		t.Errorf("the report does not say what was not attempted: %q", skipped)
+	}
+}