feat(recipes): seed the index with caddy

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Changelog
 
+## Unreleased
+
+### Added
+
+- The curated recipe index has its first entry, `caddy`, which installs the
+  Caddy web server and serves one directory over HTTP on Alpine, Ubuntu,
+  Debian, Fedora and Arch. `stoat recipe add caddy` now resolves a name
+  instead of needing a repository URL. `docs/recipes/sharing.md` describes how
+  to publish an entry.
+
+### Fixes
+
+- `stoat update <vm> --agent-access exec` set the level without updating the
+  legacy `allow_exec` field beside it, so the VM reported `allow_exec: false`
+  in every JSON and MCP payload while exec worked. The two stay in step now.
+- Restoring or deleting a snapshot tag a VM does not have answered with qemu's
+  own wording under the error code `internal`, which reads as a fault in stoat.
+  It answers `not_found` now, and names the tags the VM does have.
+
 ## v0.6.1
 
 Fixes found by driving the MCP tools against a live VM. The JSON contract

docs/guides/project-workflow.md

@@ -49,15 +49,20 @@ fields and validation rules.
 Declare a recipe in `[recipes]`, then resolve it to a commit:
 
 ```sh
-stoat recipe add https://github.com/OWNER/REPOSITORY.git@TAG
+stoat recipe add caddy
 stoat recipe lock
 stoat recipe sync
 ```
 
-Replace the URL and tag with a repository that contains a valid `recipe.toml`.
-The repository's current `index.toml` has no published entries, so an index
-name cannot be resolved from this checkout. Bundled recipes such as `docker`
-are already available and do not need `recipe add`.
+`caddy` is an index name. Pass a Git URL with an optional `@tag` instead for a
+repository the index does not carry:
+
+```sh
+stoat recipe add https://github.com/OWNER/REPOSITORY.git@TAG
+```
+
+Bundled recipes such as `docker` are already available and do not need
+`recipe add`.
 
 An index name can be used when the configured index contains it. A Git URL can
 be passed instead, with an optional `@tag` or branch. `recipe add` writes the

docs/recipes/sharing.md

@@ -33,6 +33,27 @@ stoat recipe add https://github.com/example/stoat-my-tools@main -y
 name collision with a bundled, local, or remote recipe. Use it only when the
 replacement is intentional.
 
+## Publish a recipe to the index
+
+Put the recipe in its own repository, with `recipe.toml` at the root and the
+scripts beside it, then open a pull request against `index.toml` in the Stoat
+repository:
+
+```toml
+[recipes.caddy]
+source = "https://github.com/NovusEdge/stoat-recipe-caddy"
+description = "Caddy web server, serving a local directory over HTTP"
+os = ["alpine", "ubuntu", "debian", "fedora", "arch"]
+```
+
+Pick a name no bundled recipe uses. `stoat recipe add` refuses a bundled name
+without `--force`, and the MCP `add_recipe` tool passes no force, so an agent
+can never install an entry named after a bundled recipe.
+
+Tag the recipe repository so users can pin a ref. An entry changes what a new
+`recipe add` resolves; it never moves a recipe an existing VM already pinned,
+because the commit lives in the user's `stoat.lock`.
+
 List installed recipes, their scope, and the short commit pin:
 
 ```sh

index.toml

@@ -3,6 +3,15 @@
 # name to the git repository that holds that recipe: recipe.toml at its root,
 # scripts beside it, one recipe per repository. The user's stoat.lock pins the
 # commit, so a change here never moves an existing VM's recipe.
+#
+# A name here must not repeat a bundled recipe name. CheckCollision refuses
+# that name, and the MCP add_recipe tool passes no --force, so an agent could
+# never install the entry.
 schema = 1
 
 [recipes]
+
+[recipes.caddy]
+source = "https://github.com/NovusEdge/stoat-recipe-caddy"
+description = "Caddy web server, serving a local directory over HTTP"
+os = ["alpine", "ubuntu", "debian", "fedora", "arch"]

internal/core/remote_recipes_test.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -23,6 +24,25 @@ func coreRemoteRoot(t *testing.T) string {
 	return home
 }
 
+// coreIndexRoot is coreRemoteRoot with a populated index, for the paths an
+// agent reaches through add_recipe: a name, never a URL.
+func coreIndexRoot(t *testing.T, sources map[string]string) string {
+	t.Helper()
+	home := root(t)
+	body := "schema = 1\n\n[recipes]\n"
+	names := make([]string, 0, len(sources))
+	for name := range sources {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	for _, name := range names {
+		body += fmt.Sprintf("\n[recipes.%s]\nsource = %q\ndescription = %q\n", name, sources[name], name+" test recipe")
+	}
+	index := testutil.GitRepo(t, map[string]string{"index.toml": body})
+	t.Setenv("STOAT_INDEX", index)
+	return home
+}
+
 func coreRecipeRepo(t *testing.T, repoName, manifestName string) string {
 	t.Helper()
 	src := testutil.GitRepo(t, map[string]string{
@@ -108,6 +128,156 @@ func TestRemoveCheckedWithRecipeUsersDoesNotDeadlock(t *testing.T) {
 	}
 }
 
+// add_recipe refuses a URL, so an index name is the only source an agent can
+// name. The declaration it writes carries the ref alone: the index owns the
+// source, and a later index edit moves every project that names the recipe.
+func TestAddRecipeResolvesAnIndexName(t *testing.T) {
+	src := coreRecipeRepo(t, "demo", "demo")
+	coreIndexRoot(t, map[string]string{"demo": src})
+	project := t.TempDir()
+	t.Chdir(project)
+	if err := os.WriteFile(filepath.Join(project, "stoat.toml"), []byte("[recipes]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := AddRecipe("demo", AddOpts{Yes: true}); err != nil {
+		t.Fatalf("AddRecipe(demo) = %v", err)
+	}
+	scope, err := recipes.ScopeFor(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lock, err := scope.Lock()
+	if err != nil {
+		t.Fatal(err)
+	}
+	entry, ok := lock.Recipes["demo"]
+	if !ok {
+		t.Fatalf("lock = %+v, want a demo entry", lock.Recipes)
+	}
+	if entry.Source != src {
+		t.Fatalf("lock source = %q, want the index source %q", entry.Source, src)
+	}
+	if len(entry.Commit) != 40 {
+		t.Fatalf("lock commit = %q, want a full commit", entry.Commit)
+	}
+	decls, err := scope.Decls()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if decl, ok := decls["demo"]; !ok || decl.Source != "" {
+		t.Fatalf("declaration = %+v, want an index name with no source", decl)
+	}
+}
+
+// An index name that repeats a bundled name is unreachable from add_recipe:
+// the tool passes no force, and CheckCollision refuses without it. Index
+// entries have to avoid the bundled names.
+func TestAddRecipeRefusesAnIndexNameThatShadowsABundledRecipe(t *testing.T) {
+	src := coreRecipeRepo(t, "xfce", "xfce")
+	coreIndexRoot(t, map[string]string{"xfce": src})
+	if err := recipes.Install(); err != nil {
+		t.Fatal(err)
+	}
+	project := t.TempDir()
+	t.Chdir(project)
+	if err := os.WriteFile(filepath.Join(project, "stoat.toml"), []byte("[recipes]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	err := AddRecipe("xfce", AddOpts{Yes: true})
+	if err == nil {
+		t.Fatal("AddRecipe(xfce) = nil, want a collision refusal")
+	}
+	if !strings.Contains(err.Error(), "bundled") {
+		t.Fatalf("AddRecipe(xfce) = %v, want the bundled collision named", err)
+	}
+}
+
+func TestUpdateRecipeRepinsTheDeclaredRef(t *testing.T) {
+	src := coreRecipeRepo(t, "demo", "demo")
+	coreIndexRoot(t, map[string]string{"demo": src})
+	project := t.TempDir()
+	t.Chdir(project)
+	if err := os.WriteFile(filepath.Join(project, "stoat.toml"), []byte("[recipes]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := AddRecipe("demo", AddOpts{Yes: true}); err != nil {
+		t.Fatal(err)
+	}
+	moved := testutil.GitCommit(t, src, map[string]string{"install.sh": "#!/bin/sh\necho v2\n"}, "")
+
+	if err := UpdateRecipe("demo"); err != nil {
+		t.Fatalf("UpdateRecipe(demo) = %v", err)
+	}
+	scope, err := recipes.ScopeFor(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lock, err := scope.Lock()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := lock.Recipes["demo"].Commit; got != moved {
+		t.Fatalf("lock commit = %q, want the new branch head %q", got, moved)
+	}
+	body, err := os.ReadFile(filepath.Join(scope.CachePath, "demo", "install.sh"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(string(body), "v2") {
+		t.Fatalf("cache install.sh = %q, want the updated script", body)
+	}
+}
+
+// remove_recipe has no force, so this refusal is the whole guard on that tool.
+func TestRemoveRecipeRefusesWhileAVMListsIt(t *testing.T) {
+	src := coreRecipeRepo(t, "demo", "demo")
+	coreIndexRoot(t, map[string]string{"demo": src})
+	project := t.TempDir()
+	t.Chdir(project)
+	if err := os.WriteFile(filepath.Join(project, "stoat.toml"), []byte("[recipes]\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	if err := AddRecipe("demo", AddOpts{Yes: true}); err != nil {
+		t.Fatal(err)
+	}
+	v := &config.VM{Name: "work", Mode: "live", OS: "alpine", RAM: 1024, CPUs: 1, SSHPort: 2200, Recipes: []string{"demo"}}
+	if err := v.Save(); err != nil {
+		t.Fatal(err)
+	}
+
+	err := RemoveRecipe("demo", false)
+	if !errors.Is(err, ErrInUse) || !strings.Contains(err.Error(), "work") {
+		t.Fatalf("RemoveRecipe(demo) = %v, want ErrInUse naming work", err)
+	}
+	scope, err := recipes.ScopeFor(false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	lock, err := scope.Lock()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := lock.Recipes["demo"]; !ok {
+		t.Fatal("the refused removal dropped the lock entry")
+	}
+
+	if err := RemoveRecipe("demo", true); err != nil {
+		t.Fatalf("RemoveRecipe(demo, force) = %v", err)
+	}
+	lock, err = scope.Lock()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, ok := lock.Recipes["demo"]; ok {
+		t.Fatal("the forced removal left the lock entry")
+	}
+	if _, err := os.Stat(filepath.Join(scope.CachePath, "demo")); !os.IsNotExist(err) {
+		t.Fatalf("cache stat = %v, want the checkout gone", err)
+	}
+}
+
 func TestPlanApplyAndNeedsProvisionUseAFreshProjectCacheWithoutSync(t *testing.T) {
 	home := coreRemoteRoot(t)
 	project := t.TempDir()

internal/core/snapshot.go

@@ -66,12 +66,38 @@ func Restore(name, tag string) error {
 	if err != nil {
 		return err
 	}
+	if err := requireSnapshot(name, tag); err != nil {
+		return err
+	}
 	if qemu.Running(v) {
 		return qemu.SnapshotLoad(v, tag)
 	}
 	return qemuImgSnapshot(v, "-a", tag)
 }
 
+// requireSnapshot rejects a tag the VM does not have, before qemu does. Both
+// backends answer an unknown tag with their own prose, which reached a caller
+// as an unmapped internal failure rather than the caller's own mistake. The
+// known tags come back in the message because a wrong tag is usually a typo
+// of a right one.
+func requireSnapshot(name, tag string) error {
+	snaps, err := Snapshots(name)
+	if err != nil {
+		return err
+	}
+	tags := make([]string, 0, len(snaps))
+	for _, s := range snaps {
+		if s.Tag == tag {
+			return nil
+		}
+		tags = append(tags, s.Tag)
+	}
+	if len(tags) == 0 {
+		return fmt.Errorf("%w: %s has no snapshot %q, and none at all", ErrNotFound, name, tag)
+	}
+	return fmt.Errorf("%w: %s has no snapshot %q; it has %s", ErrNotFound, name, tag, strings.Join(tags, ", "))
+}
+
 // DeleteSnapshot removes one snapshot and frees its space in the qcow2. It
 // is not part of Destroy's naming family: deleting a VM and deleting a
 // snapshot are very different sizes of mistake.
@@ -80,6 +106,9 @@ func DeleteSnapshot(name, tag string) error {
 	if err != nil {
 		return err
 	}
+	if err := requireSnapshot(name, tag); err != nil {
+		return err
+	}
 	if qemu.Running(v) {
 		return qemu.SnapshotDelete(v, tag)
 	}

internal/core/snapshot_test.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"errors"
+	"strings"
 	"testing"
 
 	"github.com/novusedge/stoat/internal/config"
@@ -125,6 +126,37 @@ func TestSnapshotRejectsBadTags(t *testing.T) {
 	}
 }
 
+// An unknown tag used to reach qemu, which answered in its own prose and
+// mapped to the internal code. It is the caller's mistake, so it is not_found.
+func TestRestoreAndDeleteRejectAnUnknownTag(t *testing.T) {
+	// The stopped-VM path reads the tag list with qemu-img. Without it the
+	// lookup fails before the tag is ever compared.
+	haveQemuImg(t)
+	root(t)
+	// A cloud VM that never started has no qcow2, so Snapshots reports none
+	// rather than failing. That is the empty case the message distinguishes.
+	if err := (&config.VM{Name: "d", Mode: "cloud", RAM: 512, CPUs: 1, SSHPort: 2200, Disk: "8G"}).Save(); err != nil {
+		t.Fatal(err)
+	}
+	for _, tc := range []struct {
+		name string
+		run  func() error
+	}{
+		{"Restore", func() error { return Restore("d", "nope") }},
+		{"DeleteSnapshot", func() error { return DeleteSnapshot("d", "nope") }},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.run()
+			if !errors.Is(err, ErrNotFound) {
+				t.Fatalf("%s = %v, want ErrNotFound", tc.name, err)
+			}
+			if !strings.Contains(err.Error(), "nope") {
+				t.Fatalf("%s = %v, want the tag named", tc.name, err)
+			}
+		})
+	}
+}
+
 func TestSnapshotUnknownVM(t *testing.T) {
 	root(t)
 	if err := TakeSnapshot("nope", "x"); !errors.Is(err, ErrNotFound) {

internal/core/update.go

@@ -198,6 +198,11 @@ func Update(name string, p Patch) (VM, error) {
 	}
 	if p.AgentAccess != nil {
 		work.AgentAccess = *p.AgentAccess
+		// Create derives allow_exec from the level and the two never disagree
+		// on disk. Setting the level alone here left the legacy field behind,
+		// so a VM raised to exec reported allow_exec false to every wire
+		// reader while exec itself worked.
+		work.AllowExec = work.AgentAccess == "exec"
 	}
 
 	if p.SSHPort != nil && *p.SSHPort != work.SSHPort {