review: rebuild a file the mesh once wrote whole, keep links, give back a missing line end, and release a hold only after the write (hq issue 128)

internal/apply/apply.go

@@ -52,6 +52,8 @@ type Outcome struct {
 	wrote string
 	// into is what a file written into held before the mesh's keys (novox/hq ADR 0102).
 	into *store.Into
+	// kept is where this apply kept the original of a file it wrote over (novox/hq ADR 0100).
+	kept string
 	// reads is, for a container, the digest of each file it was created reading, by path — so
 	// the next apply can say which one changed (novox/hq 04-ISSUES/103).
 	reads map[string]string
@@ -441,6 +443,16 @@ func ApplyKeeping(
 			continue
 		}
 
+		// Where the original of what this file replaced was kept, carried for as long as the
+		// resource is recorded: kept by this apply, by a hold its module's cutover ends, or before.
+		held, wasHeld := known.HeldAt(resource.Identity())
+		kept := outcome.kept
+		if kept == "" && wasHeld {
+			kept = held.Kept
+		}
+		if kept == "" {
+			kept = was.Kept
+		}
 		// Only now. The record follows the fact, never leads it.
 		known.Record(store.Applied{
 			Origin: origin,
@@ -448,13 +460,19 @@ func ApplyKeeping(
 			Target: outcome.Target, AppliedAt: time.Now().UTC(),
 			Wrote: outcome.wrote,
 			Into:  outcome.into,
+			Kept:  kept,
 			Reads: outcome.reads,
 			Holds: holds(resource),
 		})
-		// Its module has been taken, and what was held for it is now the mesh's.
-		if held, wasHeld := known.HeldAt(resource.Identity()); wasHeld {
+		// Its module has been taken, and what was held for it is now the mesh's. A file written
+		// into replaced nothing that was found, so its outcome says what the write did, not that
+		// a cutover happened; its hold from when it was declared whole goes all the same — here,
+		// after the write worked, so a failed one keeps the hold and where its original is.
+		if wasHeld {
 			known.Release(held.ID)
-			outcome.Detail = takenDetail(held)
+			if f, isFile := resource.(*declaration.File); !isFile || f.Into == "" {
+				outcome.Detail = takenDetail(held)
+			}
 		}
 		if svc, ok := resource.(*declaration.Service); ok && svc.TakesOver != nil && report.Tunnel != nil {
 			// The found interface is down and the mesh's is up in its place: the tunnel changed
@@ -840,6 +858,7 @@ func applyFile(r *declaration.File, previous store.Applied, unseal Unseal, keepF
 	default:
 		out.Action = "unchanged"
 	}
+	out.kept = kept
 	if kept != "" {
 		if out.Detail != "" {
 			out.Detail += "; "

internal/apply/block.go

@@ -3,6 +3,7 @@ package apply
 import (
 	"errors"
 	"fmt"
+	"net"
 	"os"
 	"path/filepath"
 	"strings"
@@ -28,17 +29,71 @@ import (
 // mesh created goes only if nothing but whitespace is left.
 
 // applyBlock writes a file's declared lines into its region of the file already at its path.
+//
+// **A link stays a link.** Where the path is a symbolic link — a hosts file some distributions keep
+// elsewhere and link into /etc — the file read, written and renamed over is the one it points to,
+// so the link and whatever manages it are left as they were. A file written whole, or into JSON,
+// still replaces a link with a file; that is unchanged here.
 func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 	out := begin(r)
 	opening, closing := declaration.BlockMarkers(r.ID)
 	want := blockBody(r.Content)
 
-	raw, err := os.ReadFile(r.Path)
+	real, err := realPath(r.Path)
+	if err != nil {
+		return out, err
+	}
+	raw, err := os.ReadFile(real)
 	existed := err == nil
 	if err != nil && !errors.Is(err, os.ErrNotExist) {
 		return out, err
 	}
+	// What the file is, taken once with what it holds: its mode and owner are the machine's and
+	// go back onto what is written. A file read and then not there to stat is a failure, never a
+	// file with no owner.
+	var info os.FileInfo
+	if existed {
+		if info, err = os.Stat(real); err != nil {
+			return out, fmt.Errorf("read %s and cannot see it: %w", r.Path, err)
+		}
+	}
 	existing := string(raw)
+
+	rec := store.Into{Format: declaration.IntoBlock}
+	var note string
+	rebuilt := false
+
+	// **A file the mesh once wrote whole** (novox/hq issue 128). The resource keeps its id when its
+	// module moves from writing the file whole to writing into it, and the file on the machine is
+	// then the mesh's own old write — its header, its loopback lines, its names. Adding the region
+	// after that would leave the old names above the new ones, and a resolver takes the first
+	// line that answers: the region would be shadowed by what it replaced. So the file is rebuilt:
+	// the original the mesh kept before its first write, with the region in it; or, where the mesh
+	// made the file itself, the loopback lines every machine needs, kept as the machine's, with the
+	// region beside them. Changed since the mesh wrote it, the file is somebody's again and is
+	// written into as it stands, and the outcome says so.
+	if existed && previous.Into == nil && previous.Wrote != "" {
+		if digestOf(existing) == previous.Wrote {
+			if previous.Kept != "" {
+				original, err := os.ReadFile(previous.Kept)
+				if err != nil {
+					return out, fmt.Errorf("%s was written whole by the mesh over an original kept at %s, "+
+						"which cannot be read to give it back: %w; it was left as it is", r.Path, previous.Kept, err)
+				}
+				existing = string(original)
+				note = "the mesh's old whole file replaced by the original kept at " + previous.Kept + ", with the region in it"
+			} else {
+				existing = loopbackOf(existing)
+				rec.Created = true
+				note = "the mesh's old whole file replaced by its loopback lines and the region"
+			}
+			// Not what was read: the whole of it was the mesh's, and the file is written afresh.
+			rebuilt = true
+		} else {
+			note = "a file the mesh once wrote whole, changed since; its old lines were kept"
+		}
+	}
+
 	lines := linesOf(existing)
 	at, found, err := regionIn(lines, opening, closing)
 	if err != nil {
@@ -49,14 +104,14 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 
 	// A record of a block is carried; anything else — no record, a file once written whole, one
 	// once written into as JSON — is a file the host is seeing for the first time as a block.
-	rec := store.Into{Format: declaration.IntoBlock}
 	recorded := previous.Into != nil && previous.Into.Format == declaration.IntoBlock
 	if recorded && existed {
 		rec.Created = previous.Into.Created
 		rec.Region = previous.Into.Region
 		rec.Separated = previous.Into.Separated
 		rec.At = previous.Into.At
-	} else {
+		rec.Ended = previous.Into.Ended
+	} else if !rebuilt {
 		// A file gone since the last apply is made again, and made by the mesh: what it held
 		// before went with it, so there is nothing to give back but the file's absence.
 		rec.Created = !existed
@@ -89,7 +144,7 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 	case r.At == declaration.AtStart:
 		// Above everything, and one blank line between the region and the machine's first line
 		// unless there is one already — a line in some files means what the lines above it say.
-		rec.At, rec.Separated = declaration.AtStart, false
+		rec.At, rec.Separated, rec.Ended = declaration.AtStart, false, false
 		next = regionOf(opening, closing, want)
 		if existing != "" && !strings.HasPrefix(existing, "\n") {
 			next += "\n"
@@ -99,10 +154,11 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 	default:
 		// At the end, apart from whatever is there: the file's last line is ended if it was not,
 		// and one blank line separates the region from the machine's lines unless there is one.
-		rec.At, rec.Separated = "", false
+		rec.At, rec.Separated, rec.Ended = "", false, false
 		next = existing
 		if next != "" && !strings.HasSuffix(next, "\n") {
 			next += "\n"
+			rec.Ended = true
 		}
 		if next != "" && next != "\n" && !strings.HasSuffix(next, "\n\n") {
 			next += "\n"
@@ -110,45 +166,60 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 		}
 		next += regionOf(opening, closing, want)
 	}
+	// What was not the mesh's is what it was. By construction — and checked, because a slip in
+	// splicing lines is exactly the fault this mode exists to prevent, and it must never be written.
+	if found {
+		after := linesOf(next)
+		if where, ok, err := regionIn(after, opening, closing); err != nil || !ok || outside(after, where) != outside(lines, at) {
+			return out, fmt.Errorf("%s: writing the region would change lines outside it; it was left as it is", r.Path)
+		}
+	}
 
-	same := existed && next == existing
+	same := existed && next == string(raw)
 	if !same {
-		var info os.FileInfo
 		mode := os.FileMode(0o644)
-		if info, err = os.Stat(r.Path); err == nil {
+		if info != nil {
 			mode = info.Mode().Perm() // the machine's file keeps the machine's mode
 		} else if mode, err = modeOf(r.Mode, mode); err != nil {
 			return out, err
 		}
-		if err := os.MkdirAll(filepath.Dir(r.Path), 0o755); err != nil {
+		if err := os.MkdirAll(filepath.Dir(real), 0o755); err != nil {
 			return out, err
 		}
-		if err := writeAtomically(r.Path, []byte(next), mode); err != nil {
+		if err := writeAtomically(real, []byte(next), mode); err != nil {
 			return out, err
 		}
-		if existed {
+		if info != nil {
 			// The write is a new file renamed over the old, so it belongs to whoever wrote it. The
 			// machine's file keeps the machine's owner, as it keeps its mode.
-			if err := keepOwner(r.Path, info); err != nil {
+			if err := keepOwner(real, info); err != nil {
 				return out, err
 			}
-		} else if err := own(r.Path, r.Owner); err != nil {
+		} else if err := own(real, r.Owner); err != nil {
 			return out, err
 		}
 	}
 
-	// Read back: the region holds what was declared, and nothing outside it moved.
-	written, err := os.ReadFile(r.Path)
+	// Read back: the region holds what was declared. Only the region — another tool writing its
+	// own lines in the moment after the rename is not a failed write. What remains is the moment
+	// between reading the file and renaming over it: a line another tool writes there is lost, and
+	// found again at its next write. Nothing short of a lock every writer honours closes that, and
+	// the other writers of a hosts file honour none.
+	written, err := os.ReadFile(real)
 	if err != nil {
 		return out, fmt.Errorf("wrote into %s and cannot read it back: %w", r.Path, err)
 	}
-	if string(written) != next {
-		return out, fmt.Errorf("%s does not hold the mesh's region as written after writing into it", r.Path)
+	back := linesOf(string(written))
+	if where, ok, err := regionIn(back, opening, closing); err != nil || !ok || where.body(back) != want {
+		return out, fmt.Errorf("%s does not hold the mesh's region after writing into it", r.Path)
 	}
 
 	out.into = &rec
 	out.wrote = digestOf(want)
 	switch {
+	case note != "" && !same:
+		out.Action = "updated"
+		out.Detail = note
 	case !existed:
 		out.Action = "created"
 		out.Detail = "written into; the file was not there"
@@ -173,16 +244,20 @@ func applyBlock(r *declaration.File, previous store.Applied) (Outcome, error) {
 
 // removeBlock gives back what a file written into a block held before the mesh's region.
 func removeBlock(a store.Applied) (string, string, error) {
-	raw, err := os.ReadFile(a.Target)
+	real, err := realPath(a.Target)
+	if err != nil {
+		return "", "", err
+	}
+	raw, err := os.ReadFile(real)
 	if errors.Is(err, os.ErrNotExist) {
 		return "forgotten", "no longer there", nil
 	}
 	if err != nil {
 		return "", "", err
 	}
-	info, err := os.Stat(a.Target)
+	info, err := os.Stat(real)
 	if err != nil {
-		return "", "", err
+		return "", "", fmt.Errorf("read %s and cannot see it: %w", a.Target, err)
 	}
 	opening, closing := declaration.BlockMarkers(a.ID)
 	lines := linesOf(string(raw))
@@ -198,8 +273,10 @@ func removeBlock(a store.Applied) (string, string, error) {
 		action, detail = "restored", "no longer declared; the region was given back what it held"
 	case found:
 		from, to := at.begin, at.end+1
-		// The blank line the host put beside the region, and only that one: if what stands there
-		// now is not blank, it is somebody's, and it stays.
+		// The blank line the host added beside the region, when a blank line still stands there.
+		// Whether it is the same one the host added cannot be known from the file; a blank line
+		// is the one line whose going changes nothing any program reads, so it is taken. A line
+		// that is not blank is never taken, whoever put it there.
 		if a.Into.Separated {
 			if a.Into.At == declaration.AtStart {
 				if to < len(lines) && lines[to] == "\n" {
@@ -210,11 +287,15 @@ func removeBlock(a store.Applied) (string, string, error) {
 			}
 		}
 		next = strings.Join(lines[:from], "") + strings.Join(lines[to:], "")
+		// And the line end the host gave the machine's last line, if that line is still last.
+		if a.Into.Ended && strings.Join(lines[to:], "") == "" {
+			next = strings.TrimSuffix(next, "\n")
+		}
 		action, detail = "restored", "no longer declared; the mesh's region was taken out and every other line kept"
 	}
 
-	if a.Into.Created && strings.TrimSpace(next) == "" {
-		if err := os.Remove(a.Target); err != nil {
+	if a.Into.Created && strings.TrimSpace(next) == "" && real == a.Target {
+		if err := os.Remove(real); err != nil {
 			return "", "", err
 		}
 		return "removed", "no longer declared; the mesh had created it and nothing else was in it", nil
@@ -222,15 +303,54 @@ func removeBlock(a store.Applied) (string, string, error) {
 	if next == string(raw) {
 		return action, detail, nil
 	}
-	if err := writeAtomically(a.Target, []byte(next), info.Mode().Perm()); err != nil {
+	if err := writeAtomically(real, []byte(next), info.Mode().Perm()); err != nil {
 		return "", "", err
 	}
-	if err := keepOwner(a.Target, info); err != nil {
+	if err := keepOwner(real, info); err != nil {
 		return "", "", err
 	}
 	return action, detail, nil
 }
 
+// realPath is the file a path names, through any links; a path that is not there yet is itself.
+// A link to nothing is refused: writing through it would replace the link with a file.
+func realPath(path string) (string, error) {
+	real, err := filepath.EvalSymlinks(path)
+	if err == nil {
+		return real, nil
+	}
+	if _, lerr := os.Lstat(path); errors.Is(lerr, os.ErrNotExist) {
+		return path, nil
+	}
+	return "", fmt.Errorf("%s is a link the host cannot follow to a file: %w; it was left as it is", path, err)
+}
+
+// loopbackOf is the lines of a file that answer for the machine itself — localhost, its own name on
+// 127.0.1.1, ::1 — and nothing else: what the mesh's old whole hosts file carried that the machine
+// needs, without the mesh's header or its names.
+func loopbackOf(text string) string {
+	var b strings.Builder
+	for _, line := range linesOf(text) {
+		fields := strings.Fields(line)
+		if len(fields) < 2 {
+			continue
+		}
+		if ip := net.ParseIP(fields[0]); ip != nil && ip.IsLoopback() {
+			b.WriteString(strings.TrimSuffix(line, "\n") + "\n")
+		}
+	}
+	return b.String()
+}
+
+// outside is every line of a file but the mesh's region, markers included, as one string.
+func outside(lines []string, at region) string {
+	end := at.end + 1
+	if end > len(lines) {
+		end = len(lines)
+	}
+	return strings.Join(lines[:at.begin], "") + "\x00" + strings.Join(lines[end:], "")
+}
+
 // blockBody is the declared lines as they stand in the region: ending in exactly one line end, or
 // nothing at all when there are no lines.
 func blockBody(content string) string {

internal/apply/block_test.go

@@ -469,3 +469,150 @@ func TestTheRecordOfABlockSurvivesTheStateFile(t *testing.T) {
 		t.Errorf("undeclaring from a saved state left %q", got)
 	}
 }
+
+// The mesh's old whole hosts file, as the controller composed it before issue 128.
+const oldWholeHosts = "# Generated by the mesh. Do not edit — this file is replaced whenever a machine\n" +
+	"# joins or leaves, and an edit would survive until then and vanish.\n\n" +
+	"127.0.0.1\tlocalhost\n" +
+	"::1\t\tlocalhost ip6-localhost ip6-loopback\n" +
+	"127.0.1.1\tg14\n" +
+	"\n" +
+	"10.42.0.1\tace.internal\tace\n" +
+	"10.42.0.9\tg14.internal\tg14\t# this machine\n"
+
+func wholeDecl(path, content string) string {
+	return fmt.Sprintf(`{"declaration":1,"resources":[
+	  {"id":%q,"type":"file","path":%q,"content":%q}
+	]}`, namesID, path, content)
+}
+
+func applyKeepingIn(t *testing.T, raw string, known store.State, keepDir string) (Report, store.State) {
+	t.Helper()
+	report, state, err := ApplyKeeping(context.Background(), archHost(t), parse(t, raw), known,
+		store.OriginDeclared, (&machine{}).run, nil, nil, KeepIn(keepDir))
+	if err != nil {
+		t.Fatalf("apply failed: %v", err)
+	}
+	return report, state
+}
+
+func TestAFileTheMeshWroteWholeAndMadeItselfKeepsOnlyItsLoopbackLines(t *testing.T) {
+	// Written whole into a file that was not there, then declared as a block under the same id:
+	// the old names must not stay above the region, where a resolver would answer from them first.
+	path := filepath.Join(t.TempDir(), "hosts")
+	_, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, t.TempDir())
+	report, state := applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir())
+	floor := "127.0.0.1\tlocalhost\n::1\t\tlocalhost ip6-localhost ip6-loopback\n127.0.1.1\tg14\n"
+	if got := readText(t, path); got != floor+"\n"+marked(namesID, meshNames) {
+		t.Fatalf("the old whole file became:\n%q", got)
+	}
+	o := outcomeOf(report, namesID)
+	if o.Action != "updated" || !strings.Contains(o.Detail, "loopback lines") {
+		t.Errorf("the rebuild was reported as %q: %s", o.Action, o.Detail)
+	}
+	if rec, _ := state.Find(namesID); !rec.Into.Created {
+		t.Error("a file the mesh made itself was not recorded as the mesh's")
+	}
+	report, _ = applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir())
+	if got := outcomeOf(report, namesID).Action; got != "unchanged" {
+		t.Errorf("applied again, the rebuilt file was %q", got)
+	}
+	undeclare(t, state)
+	if got := readText(t, path); got != floor {
+		t.Errorf("undeclared, the file holds %q", got)
+	}
+}
+
+func TestAFileTheMeshWroteWholeOverAnOriginalGetsTheOriginalBack(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "hosts")
+	_ = os.WriteFile(path, []byte(workstationHosts), 0o644)
+	keep := t.TempDir()
+	_, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, keep)
+	if rec, _ := state.Find(namesID); rec.Kept == "" {
+		t.Fatal("where the original was kept was not recorded")
+	}
+	report, state := applyKeepingIn(t, blockDecl(t, path, meshNames), state, keep)
+	if got := readText(t, path); got != workstationHosts+"\n"+marked(namesID, meshNames) {
+		t.Fatalf("the old whole file became:\n%q", got)
+	}
+	if d := outcomeOf(report, namesID).Detail; !strings.Contains(d, "original kept at") {
+		t.Errorf("the rebuild was reported as: %s", d)
+	}
+	undeclare(t, state)
+	if got := readText(t, path); got != workstationHosts {
+		t.Errorf("undeclared, the machine did not get its original back: %q", got)
+	}
+}
+
+func TestAFileTheMeshWroteWholeAndSomebodyChangedIsWrittenIntoAsItStands(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "hosts")
+	_, state := applyKeepingIn(t, wholeDecl(path, oldWholeHosts), store.State{}, t.TempDir())
+	edited := oldWholeHosts + "192.168.1.20 printer\n"
+	_ = os.WriteFile(path, []byte(edited), 0o644)
+	report, _ := applyKeepingIn(t, blockDecl(t, path, meshNames), state, t.TempDir())
+	if got := readText(t, path); got != edited+"\n"+marked(namesID, meshNames) {
+		t.Fatalf("an edited whole file became:\n%q", got)
+	}
+	if d := outcomeOf(report, namesID).Detail; !strings.Contains(d, "changed since; its old lines were kept") {
+		t.Errorf("the outcome does not say so: %s", d)
+	}
+}
+
+func TestALinkedFileStaysALink(t *testing.T) {
+	dir := t.TempDir()
+	real := filepath.Join(dir, "static", "hosts")
+	_ = os.MkdirAll(filepath.Dir(real), 0o755)
+	_ = os.WriteFile(real, []byte(workstationHosts), 0o644)
+	link := filepath.Join(dir, "hosts")
+	if err := os.Symlink(real, link); err != nil {
+		t.Fatal(err)
+	}
+	_, state := applyBlockDecl(t, blockDecl(t, link, meshNames), store.State{})
+	if info, err := os.Lstat(link); err != nil || info.Mode()&os.ModeSymlink == 0 {
+		t.Fatalf("the link was replaced by a file")
+	}
+	if got := readText(t, real); got != workstationHosts+"\n"+marked(namesID, meshNames) {
+		t.Errorf("the file the link names holds %q", got)
+	}
+	undeclare(t, state)
+	if info, err := os.Lstat(link); err != nil || info.Mode()&os.ModeSymlink == 0 {
+		t.Fatalf("undeclaring replaced the link with a file")
+	}
+	if got := readText(t, real); got != workstationHosts {
+		t.Errorf("undeclared, the file the link names holds %q", got)
+	}
+}
+
+func TestALastLineWithNoEndIsGivenBackWithNone(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "hosts")
+	_ = os.WriteFile(path, []byte("x"), 0o644)
+	_, state := applyBlockDecl(t, blockDecl(t, path, meshNames), store.State{})
+	if got := readText(t, path); got != "x\n\n"+marked(namesID, meshNames) {
+		t.Fatalf("got %q", got)
+	}
+	undeclare(t, state)
+	if got := readText(t, path); got != "x" {
+		t.Errorf("undeclaring left %q, the machine had %q", got, "x")
+	}
+}
+
+func TestAFailedBlockWriteKeepsItsHold(t *testing.T) {
+	// Held from when it was declared whole, then declared as a block into a file whose markers do
+	// not pair: the write is refused, and the hold — with where its original is — stays.
+	path := filepath.Join(t.TempDir(), "hosts")
+	broken := "a\n# BEGIN mesh " + namesID + "\n"
+	_ = os.WriteFile(path, []byte(broken), 0o644)
+	known := store.State{Held: []store.Held{{ID: namesID, Module: "mesh-wireguard", Kind: "file",
+		Target: path, Kept: "/var/lib/mesh/kept/hosts"}}}
+	resource := fmt.Sprintf(`{"id":%q,"type":"file","path":%q,"into":"block","content":%q}`, namesID, path, meshNames)
+	d := adopted(t, `{"taken":[],"untaken":{"mesh-wireguard":["`+namesID+`"]}}`, resource)
+	_, state, err := ApplyKeeping(context.Background(), archHost(t), d, known,
+		store.OriginDeclared, (&machine{}).run, nil, nil, KeepIn(t.TempDir()))
+	if err == nil {
+		t.Fatal("a write into unpaired markers was not refused")
+	}
+	h, held := state.HeldAt(namesID)
+	if !held || h.Kept != "/var/lib/mesh/kept/hosts" {
+		t.Errorf("a failed write released the hold: %+v", state.Held)
+	}
+}

internal/apply/hold.go

@@ -295,11 +295,10 @@ func holdOnAdopted(ctx context.Context, sys system.System, r declaration.Resourc
 	}
 
 	// A file written into replaces nothing that was found, so it is never held (novox/hq ADR
-	// 0102) — and a hold from when it was declared whole must not keep the mesh's keys out.
+	// 0102) — and a hold from when it was declared whole must not keep the mesh's keys out. That
+	// hold is released by the apply once the write has worked, not here: a write that fails keeps
+	// it, and with it where the original was kept.
 	if f, ok := r.(*declaration.File); ok && f.Into != "" {
-		if already {
-			known.Release(r.Identity())
-		}
 		return false, false, out, nil
 	}
 

internal/declaration/block_test.go

@@ -20,6 +20,7 @@ func TestAFileWrittenIntoABlockIsRefusedUnlessItIsOnlyItsLines(t *testing.T) {
 		"at on a whole file":        {`{"id":"f","type":"file","path":"/etc/hosts","content":"a","at":"start"}`, `at "start"`},
 		"at on a JSON file":         {`{"id":"f","type":"file","path":"/etc/x.json","into":"json","content":"{}","at":"end"}`, `at "end"`},
 		"at somewhere else":         {`{"id":"f","type":"file","path":"/etc/hosts","into":"block","content":"a","at":"middle"}`, `"start" or "end"`},
+		"an id ending in a space":   {`{"id":"f ","type":"file","path":"/etc/hosts","into":"block","content":"a"}`, "whitespace"},
 		"an unknown format":         {`{"id":"f","type":"file","path":"/etc/hosts","into":"lines","content":"a"}`, `"json" or "block"`},
 	} {
 		_, err := Parse([]byte(`{"declaration":1,"resources":[` + c.resource + `]}`))

internal/declaration/declaration.go

@@ -169,6 +169,9 @@ type File struct {
 	// the operator all write into it, and the mesh writing it whole took their lines away at the
 	// next change to the mesh's names, silently. Marked blocks are the shape the other tools in
 	// that file already use, and `#` is the comment character of every file this serves.
+	//
+	// Written into, in either format, the file's mode and owner are the machine's: a declared mode
+	// and owner apply only to a file the host creates, and a file that was there keeps its own.
 	Into string `json:"into,omitempty"`
 
 	// At is where a file written into a block has its region added when the file does not hold
@@ -272,10 +275,15 @@ func (f *File) validate(where string, _ bool) []string {
 				break
 			}
 		}
-		// The id is written into the markers, so it has to stay on one line.
+		// The id is written into the markers, so it has to stay on one line — and whitespace at
+		// either end of it is whitespace the host would have to match exactly in a line some
+		// editor may trim.
 		if strings.ContainsAny(f.ID, "\r\n") {
 			problems = append(problems, where+
 				": a file written into a block names its region by its id, and this id spans lines")
+		} else if strings.TrimSpace(f.ID) != f.ID {
+			problems = append(problems, where+
+				": a file written into a block names its region by its id, and this id begins or ends in whitespace")
 		}
 	default:
 		problems = append(problems, fmt.Sprintf(

internal/store/store.go

@@ -70,6 +70,13 @@ type Applied struct {
 	// anywhere saying why.
 	Wrote string `json:"wrote,omitempty"`
 
+	// Kept is where the original of a file this host wrote over was kept (novox/hq ADR 0100):
+	// by the keep on its first write, or by the hold that was released when its module was taken.
+	// Recorded rather than only reported, because a file once written whole and now written into
+	// (novox/hq issue 128) is given back its original with the mesh's region in it — and a path
+	// said once in a log line is not a path the host can find again.
+	Kept string `json:"kept,omitempty"`
+
 	// Into is set for a file written into rather than over (novox/hq ADR 0102): the format, what
 	// each of the mesh's keys held before it set them, which of them were absent, and whether the
 	// file itself was — so undeclaring it gives the machine back exactly what it had.
@@ -107,6 +114,9 @@ type Into struct {
 	Separated bool `json:"separated,omitempty"`
 	// At is where the host added the region: "start", or empty for the end.
 	At string `json:"at,omitempty"`
+	// Ended says the machine's last line had no line end and the host gave it one to add the
+	// region after it, so taking the region out takes that line end too.
+	Ended bool `json:"ended,omitempty"`
 }
 
 // State is the whole of what a node knows about what it has done.