From bcd815a6bbaf2972d4c6286e6816e4c426676554 Mon Sep 17 00:00:00 2001 From: jochen Date: Mon, 9 Nov 2026 22:49:20 +0100 Subject: [PATCH] A tool runner is no longer restarted by a build of the engine: owned paths name the directory of every build, and a restart names its cause (issue 1966) The tool runner's unit carries the paths modules own, for the holder of node-files. The engine's own archive is unpacked into a place named for its build, so every engine replacement changed that word and restarted the tool runner with nothing of it changed (novox 21:16, 22:25, 22:38 on 2026-11-09). A resource unpacked into a place named for its build now owns the directory holding every build. The restart line now names the unit's directive, word and key that differ, never a value. Claude-Session: https://claude.ai/code/session_01JG73KEq4zoDfM9WtYpcQgZ --- internal/apply/process.go | 114 ++++++++++++++++++ internal/apply/process_test.go | 45 +++++++ internal/compose/resolution/seat_paths.go | 17 ++- .../compose/resolution/seat_paths_test.go | 24 ++++ 4 files changed, 199 insertions(+), 1 deletion(-) diff --git a/internal/apply/process.go b/internal/apply/process.go index 5e05d99..d572664 100644 --- a/internal/apply/process.go +++ b/internal/apply/process.go @@ -4,10 +4,13 @@ import ( "context" "crypto/sha256" "encoding/hex" + "encoding/json" "fmt" + "maps" "os" "path/filepath" "runtime" + "slices" "strings" "time" @@ -260,6 +263,9 @@ func applyProcess(ctx context.Context, r *declaration.Process, run Runner, } if because != "" { out.Detail += ", restarted because " + because + " changed" + } else if what := unitDifference(previous.Wrote, want); what != "" { + // Said, so a restart nobody asked for names its cause (issue 1966): the file and the input of it, never a value. + out.Detail += ", restarted because " + what + " changed" } if placing.Detail != "" { out.Detail += "; " + placing.Detail @@ -553,3 +559,111 @@ func digestWritten(wrote string) string { // DaemonRoot is where unpacked daemons live — and what a witness keeps beside them. func DaemonRoot() string { return daemonRoot } + +// unitDifference names what differs between what a process's record says was written and what is wanted now, both +// "sha256: ": its bundle when the digest differs, and in the unit file each directive and each environment +// word that differs, with inside a word the keys of a JSON object (and, one level down, of its objects) or the names of +// the name=value entries of a list that differ. Names only, never a value, so a secret in a word is never said. Empty +// when nothing differs or there was no record. +func unitDifference(was, want string) string { + if was == "" { + return "" + } + wasDigest, wasUnit, _ := strings.Cut(was, " ") + wantDigest, wantUnit, _ := strings.Cut(want, " ") + var names []string + if wasDigest != wantDigest { + names = append(names, "its bundle") + } + before, after := unitDirectives(wasUnit), unitDirectives(wantUnit) + keys := map[string]bool{} + for k := range before { + keys[k] = true + } + for k := range after { + keys[k] = true + } + for _, k := range slices.Sorted(maps.Keys(keys)) { + if before[k] != after[k] { + names = append(names, "its unit's "+k+valueDifference(before[k], after[k], 2)) + } + } + return strings.Join(names, ", ") +} + +// unitDirectives is a unit's directives by name: "Environment " for an environment word, the directive's own +// name for the rest, each with its value as written (an environment word's value unescaped, as unitValue escapes it). +func unitDirectives(unit string) map[string]string { + out := map[string]string{} + for _, line := range strings.Split(unit, "\n") { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "#") || strings.HasPrefix(line, "[") { + continue + } + key, value, _ := strings.Cut(line, "=") + if key == "Environment" && strings.HasPrefix(value, `"`) && strings.HasSuffix(value, `"`) { + word, rest, _ := strings.Cut(value[1:len(value)-1], "=") + rest = strings.NewReplacer(`\\`, `\`, `\"`, `"`, "%%", "%").Replace(rest) + out["Environment "+word] = rest + continue + } + out[key] += value + } + return out +} + +// valueDifference is what differs inside one directive's value, as " (a, b)": the keys of a JSON object whose value +// differs (going down depth levels), else the names of the name=value entries of a comma-separated list that were +// added, removed or changed. Empty when the value says nothing more than that it differs. +func valueDifference(was, now string, depth int) string { + var a, b map[string]json.RawMessage + if depth > 0 && json.Unmarshal([]byte(was), &a) == nil && json.Unmarshal([]byte(now), &b) == nil && (a != nil || b != nil) { + all := map[string]bool{} + for k := range a { + all[k] = true + } + for k := range b { + all[k] = true + } + var names []string + for _, k := range slices.Sorted(maps.Keys(all)) { + if string(a[k]) == string(b[k]) { + continue + } + inner := "" + var x, y string + if json.Unmarshal(a[k], &x) == nil && json.Unmarshal(b[k], &y) == nil { + inner = valueDifference(x, y, 0) + } else { + inner = valueDifference(string(a[k]), string(b[k]), depth-1) + } + names = append(names, k+inner) + } + return " (" + strings.Join(names, ", ") + ")" + } + entries := func(list string) map[string]string { + m := map[string]string{} + for _, e := range strings.Split(list, ",") { + if name, _, ok := strings.Cut(e, "="); ok { + m[e] = name + } + } + return m + } + before, after := entries(was), entries(now) + named := map[string]bool{} + for e, name := range before { + if _, ok := after[e]; !ok { + named[name] = true + } + } + for e, name := range after { + if _, ok := before[e]; !ok { + named[name] = true + } + } + if len(named) == 0 { + return "" + } + return " (" + strings.Join(slices.Sorted(maps.Keys(named)), ", ") + ")" +} diff --git a/internal/apply/process_test.go b/internal/apply/process_test.go index 124d8ca..6428667 100644 --- a/internal/apply/process_test.go +++ b/internal/apply/process_test.go @@ -357,6 +357,51 @@ func TestAProcessAppliedAgainIsUnchangedAndNotRestarted(t *testing.T) { if o := outcomeOf(third, "node-tools.runtime"); o.Action != "unchanged" { t.Errorf("third apply: %+v, want unchanged", o) } + // An input of its unit that differs restarts it, and the line says which (issue 1966). + changed := declare(t, `{"id":"node-tools.runtime","type":"process","name":"node-tools","source":"`+serving(t, body)+ + `","digest":"`+digest+`","run":["node","main.js"],"env":{"MESH_TOOL_MODULES":"a=/x/index.js,b=/y/index.js"}}`) + fourth, _, err := Apply(context.Background(), archHost(t), changed, state, store.OriginDeclared, run, nil, nil) + if err != nil { + t.Fatal(err) + } + if o := outcomeOf(fourth, "node-tools.runtime"); o.Action != "updated" || !strings.Contains(o.Detail, "its unit's Environment MESH_TOOL_MODULES (b)") { + t.Errorf("fourth apply: %+v, want updated and the word and entry that changed named", o) + } +} + +// A restart nobody asked for names its cause (issue 1966): the unit's own words that differ, the key inside a word that +// is a JSON object and the entry of a list, never a value. +func TestAProcessRestartedForAnInputSaysWhichInputChanged(t *testing.T) { + for _, row := range []struct { + name, was, now, says string + }{ + {"a word of a module inside a JSON word", `{"files":{"OWNED":"/a"},"mail":{"X":"1"}}`, `{"files":{"OWNED":"/b"},"mail":{"X":"1"}}`, + "Environment TOOL_ENV (files (OWNED))"}, + {"an entry of a list", "a=/x/1,b=/y/2", "a=/x/1,b=/y/3", "Environment TOOL_MODULES (b)"}, + {"nothing changed", "a=/x/1", "a=/x/1", ""}, + } { + t.Run(row.name, func(t *testing.T) { + word := "TOOL_MODULES" + if strings.HasPrefix(row.was, "{") { + word = "TOOL_ENV" + } + mk := func(v string) string { + r := aProcess() + r.Env = map[string]string{word: v, "SECRET": "never-said-" + v} + return "sha256:" + strings.Repeat("a", 64) + " " + unitFor(r) + } + got := unitDifference(mk(row.was), mk(row.now)) + if row.says == "" { + if got != "" { + t.Fatalf("said %q for nothing changed", got) + } + return + } + if !strings.Contains(got, row.says) || strings.Contains(got, "never-said") || strings.Contains(got, "/b") { + t.Fatalf("said %q, want it to name %q and no value", got, row.says) + } + }) + } } // novox/hq ADR 0193: a bundle compiled to a binary runs itself — `./name` is that file in the diff --git a/internal/compose/resolution/seat_paths.go b/internal/compose/resolution/seat_paths.go index d1c41f9..5f8ece6 100644 --- a/internal/compose/resolution/seat_paths.go +++ b/internal/compose/resolution/seat_paths.go @@ -26,7 +26,7 @@ func declaredPaths(m mcat.Manifest) map[string]bool { } for _, r := range m.Resources { if p, ok := r["path"].(string); ok && strings.HasPrefix(p, "/") { - out[path.Clean(p)] = true + out[path.Clean(versionedPlaceOwner(r, p))] = true } } for _, f := range m.Facts { @@ -123,3 +123,18 @@ func seatPathsOnMachine(modules []mcat.Manifest) []string { } return problems } + +// versionedPlaceOwner is the path a resource owns: its own, unless the resource is an archive unpacked into a place named +// for its build (the artifact's digest, short: `.../versions/`, catalogue `${version}`), whose owner is the +// directory holding every such place, because the place changes with each build and the owned paths are an input of the +// tool runner's unit (the holder of node-files reads them at its start): a build of the engine itself renamed its place +// and restarted the tool runner, with nothing of the tool runner changed (issue 1966). +func versionedPlaceOwner(r map[string]any, p string) string { + digest, _ := r["digest"].(string) + version := strings.TrimPrefix(digest, "sha256:") + base := path.Base(path.Clean(p)) + if r["type"] == "archive" && len(base) >= 8 && strings.HasPrefix(version, base) { + return path.Dir(path.Clean(p)) + } + return p +} diff --git a/internal/compose/resolution/seat_paths_test.go b/internal/compose/resolution/seat_paths_test.go index 38bfa8f..ece92a3 100644 --- a/internal/compose/resolution/seat_paths_test.go +++ b/internal/compose/resolution/seat_paths_test.go @@ -54,3 +54,27 @@ func TestAnUplinkDeclaringTheResolverFileAsAResourceIsRefusedBesideTheResolver(t t.Errorf("the uplink's holder renders %v beside the resolver", got.Facts) } } + +// The paths a module owns do not change with its build (issue 1966): the holder of node-files is told them in an +// environment word of the tool runner's unit, so a place named for each build moved with every build of its module and +// restarted the tool runner each time. +func TestAPathNamedForABuildIsOwnedAsTheDirectoryHoldingEveryBuild(t *testing.T) { + digest := "sha256:" + strings.Repeat("ab12", 16) + for _, row := range []struct { + name string + res map[string]any + owns string + }{ + {"a build's own place", map[string]any{"id": "next", "type": "archive", "digest": digest, "path": "/usr/lib/x/versions/ab12ab12ab12"}, "/usr/lib/x/versions"}, + {"another build's place", map[string]any{"id": "next", "type": "archive", "digest": "sha256:" + strings.Repeat("cd34", 16), "path": "/usr/lib/x/versions/cd34cd34cd34"}, "/usr/lib/x/versions"}, + {"a fixed place", map[string]any{"id": "app", "type": "archive", "digest": digest, "path": "/opt/app"}, "/opt/app"}, + {"a file", map[string]any{"id": "f", "type": "file", "path": "/etc/f"}, "/etc/f"}, + } { + t.Run(row.name, func(t *testing.T) { + got := DeclaredPaths(mcat.Manifest{Module: "m", Resources: []map[string]any{row.res}}) + if len(got) != 1 || got[0] != row.owns { + t.Fatalf("owns %v, want [%s]", got, row.owns) + } + }) + } +}