feat(core): re-apply a recipe when its script changed

internal/config/config.go

@@ -25,9 +25,13 @@ type PortForward struct {
 	GuestPort int `toml:"guestport"`
 }
 
-// AppliedRecipe records when a recipe was applied and which version.
+// AppliedRecipe records when a recipe was applied, which version, and the
+// hash of the script that ran. An entry saved before Hash existed decodes
+// with an empty string, never equal to a current script's hash, so that
+// recipe re-runs once and then carries a real hash from then on.
 type AppliedRecipe struct {
 	Version string    `toml:"version"`
+	Hash    string    `toml:"hash"`
 	At      time.Time `toml:"at"`
 }
 

internal/core/apply.go

@@ -146,7 +146,11 @@ func Apply(ctx context.Context, name string, opts ApplyOpts) error {
 		if v.Applied == nil {
 			v.Applied = make(map[string]config.AppliedRecipe, len(runTargets))
 		}
-		v.Applied[name] = config.AppliedRecipe{Version: m.Version, At: time.Now()}
+		hash, err := recipes.ScriptHash(name, v.OS)
+		if err != nil {
+			return err
+		}
+		v.Applied[name] = config.AppliedRecipe{Version: m.Version, Hash: hash, At: time.Now()}
 		changed = true
 	}
 	if !changed {
@@ -161,9 +165,14 @@ func Apply(ctx context.Context, name string, opts ApplyOpts) error {
 //
 // "manual" never runs implicitly. It runs only when explicit[name] is true,
 // meaning the caller named it directly via ApplyOpts.Only. "once" is
-// skipped when v.Applied already has an entry for name at the same version
-// the manifest declares now; a version bump makes it run again, the reason
-// Version is recorded alongside At. "always" is never skipped.
+// skipped when v.Applied already has an entry for name whose Hash matches
+// the script that would run now; a changed script reruns even at the same
+// manifest version, since a version bump is not the only way a recipe
+// author fixes one. "always" is never skipped.
+//
+// An Applied entry saved before Hash existed decodes with an empty string.
+// That never equals a real script hash, so an existing VM reruns its
+// "once" recipes exactly once, then carries a real hash from then on.
 //
 // A target with no recipe.toml (ManifestFor's ok=false) is a v1 flat-file
 // recipe. It has no run-mode concept and always stays in the result,
@@ -190,8 +199,14 @@ func filterByRunMode(v *config.VM, targets []string, explicit map[string]bool) (
 				continue
 			}
 		case "once":
-			if applied, done := v.Applied[name]; done && applied.Version == m.Version {
-				continue
+			if applied, done := v.Applied[name]; done {
+				hash, err := recipes.ScriptHash(name, v.OS)
+				if err != nil {
+					return nil, nil, err
+				}
+				if applied.Hash == hash {
+					continue
+				}
 			}
 		}
 		kept = append(kept, name)

internal/core/apply_test.go

@@ -8,6 +8,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/novusedge/stoat/internal/config"
 	"github.com/novusedge/stoat/internal/recipes"
@@ -250,6 +251,228 @@ func writeRecipe(t *testing.T, dir, name, body string) {
 	}
 }
 
+// writeV2Recipe drops a v2 recipe (recipe.toml + install.sh) straight into
+// root's recipes/ dir, mirroring recipes.writeV2Recipe for core's own tests,
+// which need to set Run and Version, fields that helper doesn't take.
+func writeV2Recipe(t *testing.T, rootDir, name, run, version, script string) {
+	t.Helper()
+	recipeDir := filepath.Join(rootDir, "recipes", name)
+	if err := os.MkdirAll(recipeDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	toml := "name = \"" + name + "\"\n" +
+		"version = \"" + version + "\"\n" +
+		"script = \"install.sh\"\n" +
+		"run = \"" + run + "\"\n"
+	if err := os.WriteFile(filepath.Join(recipeDir, "recipe.toml"), []byte(toml), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(recipeDir, "install.sh"), []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// TestFilterByRunModeSkipsOnceWithMatchingHash pins the base case: a "once"
+// recipe whose stored hash still matches its current script stays skipped,
+// even though nothing here checks Version at all.
+func TestFilterByRunModeSkipsOnceWithMatchingHash(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "once", "1.0", "#!/bin/sh\necho one\n")
+	hash, err := recipes.ScriptHash("tool", "alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+	v := &config.VM{
+		OS: "alpine",
+		Applied: map[string]config.AppliedRecipe{
+			"tool": {Version: "1.0", Hash: hash},
+		},
+	}
+
+	kept, _, err := filterByRunMode(v, []string{"tool"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 0 {
+		t.Errorf("kept = %v, want none: the stored hash still matches", kept)
+	}
+}
+
+// TestFilterByRunModeRerunsOnChangedScript is the behavior this feature
+// adds: a fixed script at the SAME version must re-run, since a version
+// bump is no longer the only signal filterByRunMode trusts.
+func TestFilterByRunModeRerunsOnChangedScript(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "once", "1.0", "#!/bin/sh\necho fixed\n")
+	staleHash, err := recipes.ScriptHash("tool", "alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Recorded against the OLD script body, before the fix landed.
+	v := &config.VM{
+		OS: "alpine",
+		Applied: map[string]config.AppliedRecipe{
+			"tool": {Version: "1.0", Hash: staleHash + "stale"},
+		},
+	}
+
+	kept, _, err := filterByRunMode(v, []string{"tool"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 1 || kept[0] != "tool" {
+		t.Errorf("kept = %v, want [tool]: the script hash no longer matches", kept)
+	}
+}
+
+// TestFilterByRunModeRerunsOnEmptyStoredHash covers the pre-existing VM
+// case: an Applied entry saved before Hash existed decodes with an empty
+// string, which never equals a real hash, so the recipe self-heals once.
+func TestFilterByRunModeRerunsOnEmptyStoredHash(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "once", "1.0", "#!/bin/sh\necho one\n")
+	v := &config.VM{
+		OS: "alpine",
+		Applied: map[string]config.AppliedRecipe{
+			"tool": {Version: "1.0"}, // no Hash: predates this field
+		},
+	}
+
+	kept, _, err := filterByRunMode(v, []string{"tool"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 1 || kept[0] != "tool" {
+		t.Errorf("kept = %v, want [tool]: an empty stored hash never matches", kept)
+	}
+}
+
+// TestFilterByRunModeAlwaysIgnoresHash pins that "always" keeps running
+// regardless of a matching hash: run mode still wins over hash comparison.
+func TestFilterByRunModeAlwaysIgnoresHash(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "always", "1.0", "#!/bin/sh\necho one\n")
+	hash, err := recipes.ScriptHash("tool", "alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+	v := &config.VM{
+		OS: "alpine",
+		Applied: map[string]config.AppliedRecipe{
+			"tool": {Version: "1.0", Hash: hash},
+		},
+	}
+
+	kept, _, err := filterByRunMode(v, []string{"tool"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 1 || kept[0] != "tool" {
+		t.Errorf("kept = %v, want [tool]: always never skips", kept)
+	}
+}
+
+// TestFilterByRunModeManualNeedsExplicit pins that "manual" still needs
+// explicit[name], unaffected by hash comparison.
+func TestFilterByRunModeManualNeedsExplicit(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "manual", "1.0", "#!/bin/sh\necho one\n")
+	v := &config.VM{OS: "alpine"}
+
+	kept, _, err := filterByRunMode(v, []string{"tool"}, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 0 {
+		t.Errorf("kept = %v, want none: manual needs an explicit name", kept)
+	}
+
+	kept, _, err = filterByRunMode(v, []string{"tool"}, map[string]bool{"tool": true})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 1 || kept[0] != "tool" {
+		t.Errorf("kept = %v, want [tool]: named explicitly", kept)
+	}
+}
+
+// TestApplyRecordsHashAndSkipsOnRerun exercises the whole path through
+// Apply: a successful run records the script hash, and a second Apply with
+// no script change selects nothing to run.
+func TestApplyRecordsHashAndSkipsOnRerun(t *testing.T) {
+	dir := root(t)
+	writeV2Recipe(t, dir, "tool", "once", "1.0", "#!/bin/sh\necho one\n")
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+	go func() {
+		for {
+			c, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			c.Write([]byte("SSH-2.0-fake\r\n"))
+			c.Close()
+		}
+	}()
+	port := ln.Addr().(*net.TCPAddr).Port
+
+	v := &config.VM{
+		Name: "work", Mode: "live", OS: "alpine", Backend: "apkovl",
+		RAM: 512, CPUs: 1, SSHPort: port, Recipes: []string{"tool"},
+	}
+	if err := v.Save(); err != nil {
+		t.Fatal(err)
+	}
+	v.Dir = dir + "/work"
+	stop := fakeRunning(t, v)
+	defer stop()
+
+	// Provision needs a real ssh binary and sshd it will never reach; a
+	// cancelled ctx makes it fail fast right after the ssh banner check,
+	// before this test needs it to succeed. This test is about the
+	// pre/post-run bookkeeping in filterByRunMode and Apply's record step,
+	// exercised directly rather than through a full provisioning run.
+	hash, err := recipes.ScriptHash("tool", "alpine")
+	if err != nil {
+		t.Fatal(err)
+	}
+	explicit := map[string]bool{}
+	kept, manifests, err := filterByRunMode(v, v.Recipes, explicit)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 1 {
+		t.Fatalf("kept = %v, want [tool] on the first pass", kept)
+	}
+	m := manifests["tool"]
+	v.Applied = map[string]config.AppliedRecipe{
+		"tool": {Version: m.Version, Hash: hash, At: time.Now()},
+	}
+	if err := v.Save(); err != nil {
+		t.Fatal(err)
+	}
+
+	reloaded, err := config.Load("work")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if reloaded.Applied["tool"].Hash != hash {
+		t.Fatalf("saved Hash = %q, want %q", reloaded.Applied["tool"].Hash, hash)
+	}
+
+	kept, _, err = filterByRunMode(reloaded, reloaded.Recipes, explicit)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(kept) != 0 {
+		t.Errorf("kept = %v, want none: second pass, no script change", kept)
+	}
+}
+
 // TestCheckRecipesUsesDeclaredCapabilityReason pins that a recipe declaring
 // "requires: systemd" with no "os" restriction produces
 // docs/design/core-api.md §4's exact example, "requires systemd, alpine

internal/core/vm.go

@@ -68,11 +68,13 @@ type Paths struct {
 }
 
 // AppliedRecipe mirrors config.AppliedRecipe: which version of a recipe ran
-// against a VM, and when. Defined here rather than re-exported from config,
-// because core is the headless layer the CLI, the TUI and the MCP server all
-// depend on, and a type alias would leak config as part of that contract.
+// against a VM, the hash of the script that ran, and when. Defined here
+// rather than re-exported from config, because core is the headless layer
+// the CLI, the TUI and the MCP server all depend on, and a type alias would
+// leak config as part of that contract.
 type AppliedRecipe struct {
 	Version string
+	Hash    string
 	At      time.Time
 }
 
@@ -243,7 +245,7 @@ func applied(m map[string]config.AppliedRecipe) map[string]AppliedRecipe {
 	}
 	out := make(map[string]AppliedRecipe, len(m))
 	for k, v := range m {
-		out[k] = AppliedRecipe{Version: v.Version, At: v.At}
+		out[k] = AppliedRecipe{Version: v.Version, Hash: v.Hash, At: v.At}
 	}
 	return out
 }

internal/recipes/recipes.go

@@ -338,3 +338,15 @@ func ScriptBody(name, osName string) (string, error) {
 	}
 	return m.ScriptContent(osName)
 }
+
+// ScriptHash returns the hex sha256 of ScriptBody(name, osName). A caller
+// compares it against a stored AppliedRecipe.Hash to tell whether a "once"
+// recipe's script changed since it last ran, even at the same manifest
+// version.
+func ScriptHash(name, osName string) (string, error) {
+	body, err := ScriptBody(name, osName)
+	if err != nil {
+		return "", err
+	}
+	return sum([]byte(body)), nil
+}