From 84a14b08d5fee1d8f8f8a58fd1a90fe278199ca6 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 09:52:33 +0100 Subject: [PATCH] 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 | 8 +- internal/apply/archive_journal.go | 64 ++++++++++++++ internal/apply/archive_journal_test.go | 115 +++++++++++++++++++++++++ internal/apply/archive_replace_test.go | 17 +++- internal/apply/archive_unpacked.go | 8 ++ 5 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 internal/apply/archive_journal.go create mode 100644 internal/apply/archive_journal_test.go diff --git a/internal/apply/archive.go b/internal/apply/archive.go index fef2f12..1ec3983 100644 --- a/internal/apply/archive.go +++ b/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) diff --git a/internal/apply/archive_journal.go b/internal/apply/archive_journal.go new file mode 100644 index 0000000..3e49769 --- /dev/null +++ b/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 +} diff --git a/internal/apply/archive_journal_test.go b/internal/apply/archive_journal_test.go new file mode 100644 index 0000000..0a56203 --- /dev/null +++ b/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") + } +} diff --git a/internal/apply/archive_replace_test.go b/internal/apply/archive_replace_test.go index 2982f1f..8b3431b 100644 --- a/internal/apply/archive_replace_test.go +++ b/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 +} diff --git a/internal/apply/archive_unpacked.go b/internal/apply/archive_unpacked.go index cc032cd..5038b67 100644 --- a/internal/apply/archive_unpacked.go +++ b/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).