Say a re-apply whose result changed, refuse whole when the package manager is not there, and install the packages in the apply that made it answer (issue 233 review)

cmd/mesh-host/main.go

@@ -650,12 +650,17 @@ func runApply(ctx context.Context, opts options, d *declaration.Declaration, raw
 	// Minding the windows the daemon's scheduler has open: this runs as its own process, and a
 	// `reconcile` by hand at 03:31 is exactly the apply that must not restart a server a collector
 	// is holding still (novox/hq issue 224).
-	report, updated, applyErr := apply.ApplyMindingWindows(ctx, sys, d, known, origin,
-		apply.ExecRunner, func(line string) {
-			if !opts.json {
-				fmt.Fprintln(out, line)
-			}
-		}, sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)), apply.WindowsIn(filepath.Dir(opts.state)))
+	// Once more when the package manager did not answer and this apply wrote something: what it wrote may be
+	// what makes it answer, and then the packages go in now rather than in five minutes (issue 233, probe.go).
+	tell := func(line string) {
+		if !opts.json {
+			fmt.Fprintln(out, line)
+		}
+	}
+	report, updated, applyErr, probe, _ := applyMindingProbe(ctx, sys, probe, known, func(known store.State) (apply.Report, store.State, error) {
+		return apply.ApplyMindingWindows(ctx, sys, d, known, origin, apply.ExecRunner, tell, sealOpener(opts.state),
+			apply.KeepIn(filepath.Dir(opts.state)), apply.WindowsIn(filepath.Dir(opts.state)))
+	}, tell)
 
 	// What this apply settles about the node, whichever way it went. The mode is what the
 	// declaration said and the check above agreed with; the bundle, once applied, is consumed.
@@ -1204,7 +1209,7 @@ func runLink(ctx context.Context, opts options) error {
 		Membership: membership,
 		Apply:      applier,
 		Reconcile:  reconcileKept(opts, mine.Membership.Signer, sched, say),
-		News:       func(r link.Report) bool { return worthSaying(r) && watch.differs(r) },
+		News:       watch.news,
 		// Counted as said only once the broker has taken it: queued and lost — the link down, the
 		// publish refused — the change would never be said again (novox/hq ADR 0100).
 		// And the first account under this build is what proves it to the launcher (to-be 45 §8).
@@ -1345,6 +1350,8 @@ func (n numbersFile) Save(reportSequence, refusedOlder int64) error {
 type adoptionWatch struct {
 	mu   sync.Mutex
 	last string
+	// faulty is whether the last report the mesh heard said an apply refused or failed (novox/hq issue 233).
+	faulty bool
 }
 
 // fingerprint is what a report says about adoption: each hold and whether it changed, the
@@ -1371,6 +1378,12 @@ func adoptionFingerprint(r link.Report) string {
 	for _, v := range r.Rollbacks {
 		parts = append(parts, fmt.Sprintf("rollback %s %s %s %s", v.Component, v.From, v.To, v.Outcome))
 	}
+	// **And how the apply went** (novox/hq issue 233): refused, clean, or which resources it did not apply. A
+	// reconcile is the only apply a machine makes between deliveries, so a machine whose package manager stopped
+	// answering, or answers again, says so at the next reconcile — otherwise the mesh's record of what it did
+	// stays whatever the last delivery left, and a fault that began or ended between deliveries is never seen.
+	// Not the words: an error that carries a time would read as new on every reconcile.
+	parts = append(parts, applyState(r))
 	if r.FoundFirewall != nil {
 		parts = append(parts, fmt.Sprintf("found-firewall %s active=%v retired-by=%s", r.FoundFirewall.Kind,
 			r.FoundFirewall.Active, r.FoundFirewall.RetiredBy))
@@ -1383,6 +1396,40 @@ func adoptionFingerprint(r link.Report) string {
 	return strings.Join(parts, "\n")
 }
 
+// applyState is how an apply went, as the fingerprint keeps it: refused, clean, or failed with what it did not
+// apply and which keys its failure is said under.
+func applyState(r link.Report) string {
+	switch {
+	case r.Refused != "":
+		return "apply refused"
+	case len(r.Failed) == 0:
+		return "apply clean"
+	}
+	keys := make([]string, 0, len(r.Failed))
+	for k := range r.Failed {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return "apply failed " + strings.Join(keys, ",") + " unapplied " + strings.Join(r.Unapplied, ",")
+}
+
+// faulty is whether a report says an apply refused or failed.
+func faulty(r link.Report) bool { return r.Refused != "" || len(r.Failed) > 0 }
+
+// news is whether a reconcile's report is worth saying unasked: it says something the last one the mesh heard
+// did not, and it is about the machine (worthSaying) — or about how the apply went, refused or failed now, or
+// clean after the last one the mesh heard was not (novox/hq issue 233). A refusal says nothing about the
+// machine, but that the machine refuses is what the mesh raises a condition from, and a clean apply after it is
+// what clears one.
+func (w *adoptionWatch) news(r link.Report) bool {
+	w.mu.Lock()
+	defer w.mu.Unlock()
+	if adoptionFingerprint(r) == w.last {
+		return false
+	}
+	return worthSaying(r) || faulty(r) || w.faulty
+}
+
 // differs says whether a report says anything the last one that went out did not. It records
 // nothing: what was said is what reached the mesh, not what was written down to send.
 func (w *adoptionWatch) differs(r link.Report) bool {
@@ -1396,6 +1443,7 @@ func (w *adoptionWatch) said(r link.Report) {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 	w.last = adoptionFingerprint(r)
+	w.faulty = faulty(r)
 }
 
 // changed is differs and said together, for a report published as it is made.
@@ -1786,8 +1834,10 @@ func holdTheMachine(ctx context.Context, queue *link.Queue) {
 // node when it can say which links face outside (ADR 0140) — a converged node holds nothing and found
 // no firewall, so without that it could speak only in reply to a declaration, while the mesh composes
 // no declaration for a node that has not said which links face outside. Whether a report is news is
-// the queue's News (worthSaying and the watch), so an unchanged answer costs one comparison every
-// reconcile and nothing on the bus.
+// the queue's News (the watch's news), so an unchanged answer costs one comparison every reconcile and
+// nothing on the bus. And on any node when how the apply went changed — refused, clean, or which resources it
+// did not apply — since that is what the mesh raises a machine that does not apply from, and clears it by
+// (novox/hq issue 233).
 func reconcileKept(opts options, signer ed25519.PublicKey, sched *apply.Scheduler,
 	say link.Announce) link.Reconcile {
 	return func(ctx context.Context) (link.Report, bool) {
@@ -1973,9 +2023,13 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto
 	//
 	// `say` already reaches stdout, and the launcher's unit sends that to the journal, so this needs
 	// no new mechanism — only for the argument to be passed.
-	outcome, updated, applyErr := apply.ApplyMindingWindows(ctx, built, declared, known, store.OriginDeclared,
-		apply.ExecRunner, announceOr(say), sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)),
-		apply.WindowsIn(filepath.Dir(opts.state)))
+	// Once more when the package manager did not answer and this apply wrote something (issue 233, probe.go).
+	outcome, updated, applyErr, probe, firstPass := applyMindingProbe(ctx, built, probe, known,
+		func(known store.State) (apply.Report, store.State, error) {
+			return apply.ApplyMindingWindows(ctx, built, declared, known, store.OriginDeclared,
+				apply.ExecRunner, announceOr(say), sealOpener(opts.state), apply.KeepIn(filepath.Dir(opts.state)),
+				apply.WindowsIn(filepath.Dir(opts.state)))
+		}, announceOr(say))
 
 	// The mode the mesh said, recorded whichever way the apply went: the declaration is kept
 	// either way, and the node is held to it from the next reconcile (novox/hq ADR 0100).
@@ -1986,7 +2040,7 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto
 	// Root acted on what an agent account's judgement reads — an account, a package, an action, sudo's
 	// rules: the search for setuid programs runs again (novox/hq issue 361). A failed apply counts what it
 	// did before it stopped.
-	moved := outcome.Outcomes
+	moved := append(append([]apply.Outcome(nil), firstPass...), outcome.Outcomes...)
 	var partial *apply.Error
 	if errors.As(applyErr, &partial) {
 		moved = append(append([]apply.Outcome(nil), moved...), partial.Done.Outcomes...)
@@ -2132,6 +2186,8 @@ func applyAndKeepHeld(ctx context.Context, opts options, raw []byte, signed *sto
 		report.Failed = map[string]string{"apply": applyErr.Error()}
 	}
 	report.Failed = withUnanswered(report.Failed, probe)
+	// Which resources did not apply, for whether a reconcile's account is news (issue 233): not said on the wire.
+	report.Unapplied = unapplied(declared, outcome, applyErr)
 	return report
 }
 

cmd/mesh-host/probe.go

@@ -3,8 +3,12 @@ package main
 import (
 	"context"
 	"errors"
+	"os/exec"
+	"sort"
 
 	"github.com/novox/mesh-host/internal/apply"
+	"github.com/novox/mesh-host/internal/declaration"
+	"github.com/novox/mesh-host/internal/store"
 	"github.com/novox/mesh-host/internal/system"
 )
 
@@ -46,6 +50,11 @@ func (u *unanswered) partial() error {
 // undo what an earlier one did. Before the rest is applied, a declaration composed from an older view of the
 // assignments than this machine applied has already been refused (issue 234's generation).
 //
+// **A package manager that is not there at all still refuses whole.** That is the host installed on a
+// machine it was not built for — the arch host on Alpine — which is what the probe exists to catch, and no
+// declaration the mesh sends removes a package manager's program. Only one that is there and errors (a
+// configuration file gone, a database locked) is the mesh's own fault and applies in part.
+//
 // A system with no package manager (android) still refuses whole: what its probe asks is the system itself,
 // which no resource the mesh declares can remove, and nothing else stands between it and the wrong machine.
 func probePackageManager(ctx context.Context, sys system.System) (*unanswered, error) {
@@ -54,12 +63,84 @@ func probePackageManager(ctx context.Context, sys system.System) (*unanswered, e
 		return nil, nil
 	}
 	tool, ok := packageManagers[sys.Name()]
-	if !ok {
+	if !ok || errors.Is(err, exec.ErrNotFound) {
 		return nil, err
 	}
 	return &unanswered{tool: tool, words: err.Error()}, nil
 }
 
+// applyPass is one apply of the declaration over what the node's state says.
+type applyPass func(known store.State) (apply.Report, store.State, error)
+
+// applyMindingProbe applies, and **once more when the package manager did not answer and this apply wrote
+// something** (novox/hq issue 233). What it wrote may be what the package manager needed — the declaration
+// that names its module again writes its configuration file back — and then the packages go in now rather
+// than at the next reconcile. Asked again only then: an apply that wrote nothing changed nothing the package
+// manager reads. The second pass is the same declaration over the state the first left, so what the first
+// applied reads unchanged and only what needs the package manager moves.
+//
+// It returns the last pass's account, the probe as it stands after it (nil once the package manager
+// answers), and the first pass's outcomes when a second ran, for whoever counts what an apply moved.
+func applyMindingProbe(ctx context.Context, sys system.System, probe *unanswered, known store.State,
+	pass applyPass, say func(string)) (apply.Report, store.State, error, *unanswered, []apply.Outcome) {
+	report, updated, err := pass(known)
+	if probe == nil || !wrote(report, err) {
+		return report, updated, err, probe, nil
+	}
+	again, refused := probePackageManager(ctx, sys)
+	if refused != nil || again != nil {
+		return report, updated, err, probe, nil
+	}
+	say(probe.tool + " answers now that this apply wrote what it reads; applying again, so its packages are " +
+		"installed in this apply (novox/hq issue 233)")
+	first := append([]apply.Outcome(nil), report.Outcomes...)
+	var partial *apply.Error
+	if errors.As(err, &partial) {
+		first = append(first, partial.Done.Outcomes...)
+	}
+	report, updated, err = pass(updated)
+	return report, updated, err, nil, first
+}
+
+// wrote is whether an apply moved anything on the machine, failed or not.
+func wrote(report apply.Report, err error) bool {
+	if report.Changed() {
+		return true
+	}
+	var partial *apply.Error
+	return errors.As(err, &partial) && partial.Done.Changed()
+}
+
+// unapplied is which declared resources an apply did not apply, failed or not attempted, sorted; nil for a
+// clean one.
+func unapplied(d *declaration.Declaration, report apply.Report, err error) []string {
+	if err == nil {
+		return nil
+	}
+	done := map[string]bool{}
+	for _, o := range report.Outcomes {
+		done[o.ID] = o.Action != "skipped"
+	}
+	var partial *apply.Error
+	if errors.As(err, &partial) {
+		for _, o := range partial.Done.Outcomes {
+			done[o.ID] = done[o.ID] || o.Action != "skipped"
+		}
+	}
+	var out []string
+	for _, r := range d.Resources {
+		if !done[r.Identity()] {
+			out = append(out, r.Identity())
+		}
+	}
+	sort.Strings(out)
+	if len(out) == 0 {
+		// Failed, and every resource has an account: what failed is no resource (the firewall found, a removal).
+		out = []string{"(apply)"}
+	}
+	return out
+}
+
 // withUnanswered adds the probe to what a report says failed.
 func withUnanswered(failed map[string]string, u *unanswered) map[string]string {
 	if u == nil {

cmd/mesh-host/probe_test.go

@@ -4,10 +4,12 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 	"testing"
 
 	"github.com/novox/mesh-host/internal/declaration"
+	"github.com/novox/mesh-host/internal/link"
 	"github.com/novox/mesh-host/internal/system"
 )
 
@@ -56,3 +58,152 @@ func TestASystemWithNoPackageManagerStillRefusesWhole(t *testing.T) {
 		t.Fatalf("the android host's failed probe did not refuse: %v, %+v", err, probe)
 	}
 }
+
+// pacmanAnsweringOnce is a pacman that does not answer until conf exists, as the workstation's did until its
+// configuration file was written back, and installs into a directory of its own once it does.
+func pacmanAnsweringOnce(t *testing.T, conf string) {
+	t.Helper()
+	bin, installed := t.TempDir(), t.TempDir()
+	script := "#!/bin/sh\n" +
+		"[ -f '" + conf + "' ] || { echo \"error: config file " + conf + " could not be read: No such file or directory\" >&2; exit 1; }\n" +
+		"for a; do last=$a; done\n" +
+		"case \"$1\" in\n" +
+		"-Q) [ \"$2\" = pacman ] && exit 0; [ -f '" + installed + "'/\"$2\" ] && exit 0; exit 1;;\n" +
+		"-S) touch '" + installed + "'/\"$last\"; exit 0;;\n" +
+		"esac\nexit 1\n"
+	if err := os.WriteFile(filepath.Join(bin, "pacman"), []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
+	was := builtFor
+	builtFor = "arch"
+	t.Cleanup(func() { builtFor = was })
+}
+
+// Defends the review of novox/hq issue 233: **the apply that writes the package manager's configuration back
+// installs its packages too**. The probe is asked again once the partial apply wrote something, and when it
+// answers the declaration is applied again over what the first pass left — the packages in this apply, not five
+// minutes later, and the report clean.
+func TestThePackagesGoInWhenTheApplyThatRestoresThePackageManagerWroteIt(t *testing.T) {
+	dir := t.TempDir()
+	conf := filepath.Join(dir, "pacman.conf")
+	pacmanAnsweringOnce(t, conf)
+	opts := options{state: filepath.Join(dir, "state.json")}
+	// The package first: within one pass it is asked about before the file is written, which is the order that
+	// leaves it to a second pass.
+	raw := []byte(`{"declaration":1,"resources":[` +
+		`{"id":"pacman.base","type":"package","package":"base"},` +
+		`{"id":"pacman.conf","type":"file","path":"` + conf + `","content":"[options]\n"}]}`)
+
+	report := applyAndKeep(context.Background(), opts, raw, nil, nil, nil)
+
+	if report.Refused != "" || len(report.Failed) != 0 {
+		t.Fatalf("the apply that made the package manager answer did not install its packages: refused %q, failed %v",
+			report.Refused, report.Failed)
+	}
+	if !slices.Contains(report.Applied, "pacman.base") || !slices.Contains(report.Applied, "pacman.conf") {
+		t.Errorf("the report does not account for both: %v", report.Applied)
+	}
+}
+
+// Defends decision (1) of the review of novox/hq issue 233: **a package manager that is not there at all still
+// refuses whole**. That is the host installed on the wrong machine, which the probe exists to catch.
+func TestAPackageManagerThatIsNotThereRefusesWhole(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+	was := builtFor
+	builtFor = "arch"
+	t.Cleanup(func() { builtFor = was })
+	dir := t.TempDir()
+	conf := filepath.Join(dir, "pacman.conf")
+	raw := []byte(`{"declaration":1,"resources":[{"id":"pacman.conf","type":"file","path":"` + conf +
+		`","content":"[options]\n"}]}`)
+
+	report := applyAndKeep(context.Background(), options{state: filepath.Join(dir, "state.json")}, raw, nil, nil, nil)
+
+	if !strings.Contains(report.Refused, "pacman does not answer") {
+		t.Fatalf("a machine with no pacman at all was not refused whole: %+v", report)
+	}
+	if _, err := os.Stat(conf); err == nil {
+		t.Error("the refused declaration touched the machine")
+	}
+}
+
+// Defends the review of novox/hq issue 233: **a re-apply whose result changed is said unasked**. A machine
+// reconciles every five minutes and speaks only when its account is news; how the apply went was not part of
+// it, so a package manager that stopped answering between deliveries was never said, and one that answered
+// again never cleared. Now a failure is said once, the same failure again is not, and a clean re-apply after it
+// is — which is what clears the mesh's condition.
+func TestAReapplyWhoseResultChangedIsSaidUnasked(t *testing.T) {
+	dir := t.TempDir()
+	conf := filepath.Join(dir, "elsewhere", "pacman.conf") // not declared: the apply cannot make it answer
+	pacmanAnsweringOnce(t, conf)
+	opts := options{state: filepath.Join(dir, "state.json")}
+	raw := []byte(`{"declaration":1,"resources":[` +
+		`{"id":"motd","type":"file","path":"` + filepath.Join(dir, "motd") + `","content":"hello\n"},` +
+		`{"id":"pacman.base","type":"package","package":"base"}]}`)
+	w := &adoptionWatch{}
+
+	// Clean once, as it was before: base installed while pacman answered.
+	if err := os.MkdirAll(filepath.Dir(conf), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(conf, []byte("[options]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	clean := applyAndKeep(context.Background(), opts, raw, nil, nil, nil)
+	if faulty(clean) {
+		t.Fatalf("the first apply was not clean: %+v", clean)
+	}
+	w.said(clean)
+	if w.news(applyAndKeep(context.Background(), opts, raw, nil, nil, nil)) {
+		t.Error("a clean re-apply after a clean one was said, so every machine speaks every five minutes")
+	}
+
+	// The configuration goes: the re-apply fails, and that is news once.
+	if err := os.Remove(conf); err != nil {
+		t.Fatal(err)
+	}
+	failed := applyAndKeep(context.Background(), opts, raw, nil, nil, nil)
+	if !faulty(failed) {
+		t.Fatalf("the re-apply did not fail: %+v", failed)
+	}
+	if !w.news(failed) {
+		t.Fatal("a re-apply that failed after a clean one was not said, so the mesh never hears the machine fails")
+	}
+	w.said(failed)
+	if w.news(applyAndKeep(context.Background(), opts, raw, nil, nil, nil)) {
+		t.Error("the same failure was said again on the next re-apply")
+	}
+
+	// It comes back: the clean re-apply is news, and is what clears the mesh's condition.
+	if err := os.WriteFile(conf, []byte("[options]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	again := applyAndKeep(context.Background(), opts, raw, nil, nil, nil)
+	if faulty(again) {
+		t.Fatalf("the re-apply did not come back clean: %+v", again)
+	}
+	if !w.news(again) {
+		t.Fatal("a clean re-apply after a failed one was not said, so the mesh's condition never clears")
+	}
+}
+
+// A refusal is news too, once, and a reconcile with nothing to say after a clean one stays silent.
+func TestARefusalIsSaidOnceAndNothingIsNotSaid(t *testing.T) {
+	w := &adoptionWatch{}
+	w.said(link.Report{})
+	if w.news(link.Report{}) {
+		t.Error("nothing was said again")
+	}
+	refused := link.Report{Refused: "pacman does not answer here"}
+	if !w.news(refused) {
+		t.Fatal("a refusal was not said")
+	}
+	w.said(refused)
+	if w.news(link.Report{Refused: "pacman does not answer here, at another time"}) {
+		t.Error("the same refusal in other words was said again")
+	}
+	if !w.news(link.Report{}) {
+		t.Error("a clean apply after a refusal was not said")
+	}
+}

internal/link/messages.go

@@ -229,6 +229,11 @@ type Report struct {
 	// as "not known", never as healthy and never as a reason to raise anything. Fresh on every report
 	// this engine makes, and said between reports on HealthSubject when it changes.
 	Health *Health `json:"health,omitempty"`
+
+	// Unapplied is which declared resources this apply did not apply, by id — failed or not attempted. Not
+	// said on the wire (Failed says why, in words): it is what makes a reconcile whose result changed news to
+	// the node-engine, a failure that moved to another resource included (novox/hq issue 233).
+	Unapplied []string `json:"-"`
 }
 
 // LivenessContract is the version of the health statement this host keeps (ADR 0240 Phase A: liveness,