Take a key or list member the host may have written itself as the mesh's, so undeclaring gives the file back (hq ADR 0102)

internal/apply/into.go

@@ -93,7 +93,7 @@ func applyInto(r *declaration.File, previous store.Applied) (Outcome, error) {
 			if !tracked(k) && !had {
 				rec.Absent = append(rec.Absent, k)
 			}
-			merged, added, err := addMembers(current, declared[k], rec.Added[k])
+			merged, added, err := addMembers(current, declared[k], rec.Added[k], previous.Into == nil)
 			if err != nil {
 				return out, fmt.Errorf("%s: %q: %w", r.Path, k, err)
 			}
@@ -102,7 +102,14 @@ func applyInto(r *declaration.File, previous store.Applied) (Outcome, error) {
 			continue
 		}
 		if !tracked(k) {
-			if v, had := object[k]; had {
+			v, had := object[k]
+			// **What the host may have written itself is not the machine's.** With no record of
+			// this file — the first apply, or a host that wrote and died before saving its state —
+			// a key already holding exactly what the mesh declares cannot be told from one the
+			// mesh set a moment ago. Remembered as the machine's, it would never be given back:
+			// undeclaring would leave the mesh's own value behind for ever. So it is the mesh's,
+			// and undeclaring takes it out (novox/hq ADR 0102).
+			if had && !(previous.Into == nil && canonical(v) == canonical(declared[k])) {
 				rec.Before[k] = v
 			} else {
 				rec.Absent = append(rec.Absent, k)
@@ -320,8 +327,11 @@ func listOf(members []json.RawMessage) json.RawMessage {
 // addMembers adds the declared members to the machine's list, dropping only members the mesh
 // added before and no longer declares. It returns the list and exactly which members the mesh
 // added — a declared member the machine already had is the machine's, and is never recorded.
-func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage) (json.RawMessage,
-	[]json.RawMessage, error) {
+// unrecorded says there is no record of this file yet, in which case a declared member already in
+// the list may be one the host itself wrote before it could save its state, and is taken as the
+// mesh's.
+func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage,
+	unrecorded bool) (json.RawMessage, []json.RawMessage, error) {
 	have, err := membersOf(current)
 	if err != nil {
 		return nil, nil, err
@@ -341,9 +351,11 @@ func addMembers(current, declared json.RawMessage, addedBefore []json.RawMessage
 	for _, m := range want {
 		if !hasMember(have, m) {
 			have = append(have, m)
-			if !hasMember(added, m) {
-				added = append(added, m)
-			}
+		} else if !unrecorded {
+			continue // the machine's own, and never the mesh's to take out
+		}
+		if !hasMember(added, m) {
+			added = append(added, m)
 		}
 	}
 	return listOf(have), added, nil

internal/apply/into_test.go

@@ -217,9 +217,15 @@ func TestAListIsAddedToNeverReplaced(t *testing.T) {
 	// takes back only what it added (novox/hq ADR 0102).
 	path := filepath.Join(t.TempDir(), "daemon.json")
 	_ = os.WriteFile(path, []byte(`{"insecure-registries":["192.0.2.7:5000","10.42.0.9:5000"]}`), 0o644)
-	// 10.42.0.9 is declared too, and was already the machine's: it is never the mesh's to remove.
+	// First the mesh's own member alone, so there is a record of this file.
+	first := parse(t, intoDecl(t, path, `{"insecure-registries":["10.42.0.1:5000"]}`))
+	_, state, err := Apply(context.Background(), archHost(t), first, store.State{}, store.OriginDeclared, nil, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Now 10.42.0.9 is declared too, and was already the machine's: it is never the mesh's to remove.
 	d := parse(t, intoDecl(t, path, `{"insecure-registries":["10.42.0.1:5000","10.42.0.9:5000"]}`))
-	_, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil)
+	_, state, err = Apply(context.Background(), archHost(t), d, state, store.OriginDeclared, nil, nil, nil)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -302,3 +308,35 @@ func TestAHoldFromAWholeFileDoesNotKeepOutAnIntoWrite(t *testing.T) {
 		t.Errorf("the mesh's member was not written in: %v", readObject(t, path))
 	}
 }
+
+func TestAWriteWithNoRecordOfItIsTheMeshsOwn(t *testing.T) {
+	// A host that wrote into the file and died before saving its state comes back with no record
+	// of it. What is there is then exactly what the mesh declares — and remembered as the
+	// machine's it would never be given back (novox/hq ADR 0102).
+	path := filepath.Join(t.TempDir(), "daemon.json")
+	_ = os.WriteFile(path, []byte(`{"data-root":"/srv/docker","insecure-registries":["192.0.2.7:5000"]}`), 0o644)
+	d := parse(t, intoDecl(t, path, `{"live-restore":true,"insecure-registries":["10.42.0.1:5000"]}`))
+	if _, _, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+
+	// The crash: the state was never saved, so the next apply knows nothing of this file.
+	_, state, err := Apply(context.Background(), archHost(t), d, store.State{}, store.OriginDeclared, nil, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	rec, _ := state.Find("networking.registry-trust")
+	if _, asTheMachines := rec.Into.Before["live-restore"]; asTheMachines {
+		t.Error("the mesh's own key was remembered as the machine's")
+	}
+	if _, _, err := Apply(context.Background(), archHost(t), somethingElse(t), state, store.OriginDeclared, nil, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	o := readObject(t, path)
+	if _, still := o["live-restore"]; still {
+		t.Errorf("undeclaring left the mesh's key behind: %v", o)
+	}
+	if fmt.Sprint(o["insecure-registries"]) != "[192.0.2.7:5000]" || o["data-root"] != "/srv/docker" {
+		t.Errorf("the machine did not get its file back: %v", o)
+	}
+}