Write down what an archive unpacked as soon as it is unpacked, so a lost record never makes the host's own files foreign (issue 1585)

Claude-Session: https://claude.ai/code/session_01JG73KG7AfSdh6FP8VrBqCn

internal/apply/archive.go

@@ -38,6 +38,8 @@ const maxArchive = 512 << 20
 func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Applied, away *storeAway) (Outcome, error) {
 	out := begin(r)
 	out.Action = "unchanged"
+	recorded := previous.Wrote != ""
+	previous = withJournal(previous, r.Path)
 
 	body, err := fetchMinding(ctx, r.Source, away)
 	if err != nil {
@@ -78,8 +80,12 @@ func applyArchive(ctx context.Context, r *declaration.Archive, previous store.Ap
 		return out, err
 	}
 	out.unpacked = &unpacked
+	// At once, before the rest of the apply can fail or the engine stop (novox/hq issue 1585).
+	if err := keepJournal(r.Path, got, &unpacked); err != nil {
+		return out, fmt.Errorf("%s was unpacked, and what it unpacked could not be written down: %w", r.Path, err)
+	}
 	out.Action = "updated"
-	if previous.Wrote == "" {
+	if !recorded {
 		out.Action = "created"
 	}
 	out.Detail = fmt.Sprintf("%d file(s)", written)

internal/apply/archive_journal.go

@@ -0,0 +1,64 @@
+package apply
+
+import (
+	"encoding/json"
+	"os"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// What an archive unpacked is written down the moment it is unpacked (novox/hq issue 1585).
+//
+// The node's state is saved at the end of a module's apply, so an engine that stopped, was replaced or lost its
+// state between the unpack and that save held files on the machine it had no record of. The next apply then found
+// its own files and refused them as ones "the mesh did not put there" (issue 162), and a module could not move to
+// its new build until a person removed them. So beside the directory, at once and atomically, the host keeps the
+// digest it unpacked and the files and directories it placed; an apply that holds no record of the archive reads
+// this instead of treating everything there as somebody's. The record in the state stays the authority: the
+// journal is read only when there is none.
+
+// journalSuffix is appended to an archive's path to name its journal.
+const journalSuffix = ".unpacked.json"
+
+type unpackJournal struct {
+	Wrote    string          `json:"wrote"`
+	Unpacked *store.Unpacked `json:"unpacked"`
+}
+
+func journalPath(path string) string { return path + journalSuffix }
+
+// keepJournal writes down what an archive unpacked at path.
+func keepJournal(path, wrote string, u *store.Unpacked) error {
+	raw, err := json.Marshal(unpackJournal{Wrote: wrote, Unpacked: u})
+	if err != nil {
+		return err
+	}
+	return writeAtomicallyByPath(journalPath(path), raw, 0o600)
+}
+
+// journalOf is what the journal says of path, or false when there is none or it cannot be read.
+func journalOf(path string) (unpackJournal, bool) {
+	raw, err := os.ReadFile(journalPath(path))
+	if err != nil {
+		return unpackJournal{}, false
+	}
+	var j unpackJournal
+	if json.Unmarshal(raw, &j) != nil || j.Wrote == "" || j.Unpacked == nil {
+		return unpackJournal{}, false
+	}
+	return j, true
+}
+
+// withJournal is previous, or — when the state holds no record of the archive — what the journal says the host
+// unpacked at path.
+func withJournal(previous store.Applied, path string) store.Applied {
+	if previous.Wrote != "" {
+		return previous
+	}
+	j, ok := journalOf(path)
+	if !ok {
+		return previous
+	}
+	previous.Wrote, previous.Target, previous.Unpacked = j.Wrote, path, j.Unpacked
+	return previous
+}

internal/apply/archive_journal_test.go

@@ -0,0 +1,115 @@
+package apply
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/novox/mesh-host/internal/store"
+)
+
+// Defends novox/hq issue 1585: an archive's files were refused as "not put there by the mesh" when the state held
+// no record of them, though the host had unpacked them itself.
+
+// An archive the host unpacked, whose record is gone, is its own: the next version replaces its files, drops the
+// ones it no longer has, and leaves a file somebody added; a file the host never unpacked, or an unrecorded file
+// with other bytes where no unpack was journaled, still refuses the archive.
+func TestAnArchiveWhoseRecordIsGoneIsStillTheHosts(t *testing.T) {
+	rows := []struct {
+		name       string
+		journaled  bool // the first version was unpacked by the host
+		foreign    string
+		wantRefuse bool
+	}{
+		{"unpacked by the host, record lost", true, "", false},
+		{"unpacked by the host, somebody's file beside", true, "mine.txt", false},
+		{"never unpacked by the host", false, "", true},
+	}
+	for _, row := range rows {
+		t.Run(row.name, func(t *testing.T) {
+			dir := t.TempDir()
+			target := filepath.Join(dir, "tools")
+			first, _ := archiveDecl(t, "m.tools", target, map[string]string{"bin": "one", "old": "o"}, "")
+			if row.journaled {
+				if _, _, err := Apply(context.Background(), archHost(t), declare(t, first), store.State{},
+					store.OriginDeclared, noServices, nil, nil); err != nil {
+					t.Fatal(err)
+				}
+			} else {
+				os.MkdirAll(target, 0o755)
+				os.WriteFile(filepath.Join(target, "bin"), []byte("one"), 0o644)
+			}
+			if row.foreign != "" {
+				os.WriteFile(filepath.Join(target, row.foreign), []byte("x"), 0o644)
+			}
+			second, _ := archiveDecl(t, "m.tools", target, map[string]string{"bin": "two"}, "")
+			// The state is empty: the record of the first unpack was lost.
+			_, _, err := Apply(context.Background(), archHost(t), declare(t, second), store.State{},
+				store.OriginDeclared, noServices, nil, nil)
+			if row.wantRefuse {
+				if err == nil || !strings.Contains(err.Error(), "did not put there") {
+					t.Fatalf("refused? %v", err)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got, _ := os.ReadFile(filepath.Join(target, "bin")); string(got) != "two" {
+				t.Fatalf("bin is %q", got)
+			}
+			if exists(filepath.Join(target, "old")) {
+				t.Fatal("a file the new version no longer has is still there")
+			}
+			if row.foreign != "" && !exists(filepath.Join(target, row.foreign)) {
+				t.Fatal("somebody's file went")
+			}
+		})
+	}
+}
+
+// An unrecorded file already holding exactly the archive's bytes is not a collision; one with other bytes is.
+func TestAnUnrecordedFileWithTheArchivesBytesIsNotACollision(t *testing.T) {
+	for _, row := range []struct {
+		name, has string
+		refused   bool
+	}{{"same bytes", "theme", false}, {"other bytes", "somebody's", true}} {
+		t.Run(row.name, func(t *testing.T) {
+			dir := t.TempDir()
+			target := filepath.Join(dir, "theme")
+			os.MkdirAll(target, 0o755)
+			os.WriteFile(filepath.Join(target, "p10k.zsh"), []byte(row.has), 0o644)
+			archive, _ := archiveDecl(t, "shell.theme", target, map[string]string{"p10k.zsh": "theme"}, "")
+			_, _, err := Apply(context.Background(), archHost(t), declare(t, archive), store.State{},
+				store.OriginDeclared, noServices, nil, nil)
+			if (err != nil) != row.refused {
+				t.Fatalf("err = %v", err)
+			}
+		})
+	}
+}
+
+// Undeclaring an archive takes its journal with it, and a journal alone never makes a file the host's.
+func TestTheJournalGoesWithTheArchive(t *testing.T) {
+	dir := t.TempDir()
+	target := filepath.Join(dir, "theme")
+	archive, _ := archiveDecl(t, "shell.theme", target, map[string]string{"a": "a"}, "")
+	_, state, err := Apply(context.Background(), archHost(t), declare(t, archive), store.State{},
+		store.OriginDeclared, noServices, nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !exists(journalPath(target)) {
+		t.Fatal("no journal after the unpack")
+	}
+	if _, _, err := Apply(context.Background(), archHost(t),
+		declare(t, `{"id":"other","type":"file","path":"`+dir+`/other","content":"o"}`), state,
+		store.OriginDeclared, noServices, nil, nil); err != nil {
+		t.Fatal(err)
+	}
+	if exists(journalPath(target)) {
+		t.Fatal("the journal outlived the archive")
+	}
+}

internal/apply/archive_replace_test.go

@@ -3,6 +3,7 @@ package apply
 import (
 	"context"
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/novox/mesh-host/internal/store"
@@ -32,7 +33,7 @@ func TestAnArchiveReplacesTheTreeItWasUnpackedOver(t *testing.T) {
 	if _, err := os.Stat(dir + "/tools/node_modules"); err == nil {
 		t.Fatal("the previous archive's directory is still there")
 	}
-	entries, _ := os.ReadDir(dir)
+	entries := besideTheTree(dir)
 	if len(entries) != 1 {
 		names := []string{}
 		for _, e := range entries {
@@ -62,7 +63,19 @@ func TestARefusedArchiveLeavesTheTreeItWouldHaveReplaced(t *testing.T) {
 	if got, _ := os.ReadFile(dir + "/tools/index.js"); string(got) != "old" {
 		t.Fatalf("index.js is %q after a refused archive", got)
 	}
-	if entries, _ := os.ReadDir(dir); len(entries) != 1 {
+	if entries := besideTheTree(dir); len(entries) != 1 {
 		t.Fatalf("%d entries beside the tree after a refused archive", len(entries))
 	}
 }
+
+// besideTheTree is what sits in dir, but for the journals of what archives unpacked (novox/hq issue 1585).
+func besideTheTree(dir string) []os.DirEntry {
+	all, _ := os.ReadDir(dir)
+	var out []os.DirEntry
+	for _, e := range all {
+		if !strings.HasSuffix(e.Name(), journalSuffix) {
+			out = append(out, e)
+		}
+	}
+	return out
+}

internal/apply/archive_unpacked.go

@@ -365,6 +365,14 @@ func mergeInto(fresh, path, owner string, files, dirs []string, o ours) (store.U
 // open stays readable until it closes it. The runtime is told the module went by its own membership,
 // not by the files disappearing.
 func removeArchive(a store.Applied) (string, string, error) {
+	// First, so the parents the host made are empty by the time they are looked at.
+	if !store.IsFormer(a.ID) && a.Unpacked != nil {
+		os.Remove(journalPath(filepath.Clean(a.Target)))
+	}
+	return removeArchiveFiles(a)
+}
+
+func removeArchiveFiles(a store.Applied) (string, string, error) {
 	if store.IsFormer(a.ID) {
 		// The version before is what a rollback starts (ADR 0141) and what a reader may still have
 		// open; the launcher retires the host's own versions, not the apply (issue 194).