review: 8 fixes, cuts, docs against the code

.gitignore

@@ -1,9 +1,4 @@
 
-# Internal review documents (contain sensitive information)
-docs/P0-RESOLVED.md
-docs/P0-VERIFICATION-SUMMARY.md
-docs/P1-TODO.md
-
 # Build artifacts
 /cocoon
 cocoon-linux
@@ -40,7 +35,7 @@ coverage/
 .testcache/
 
 # Local dev environment (macOS)
-dev/
+/dev/
 
 # Implementation plan (internal)
 TODO*
@@ -50,8 +45,8 @@ tmp/*
 .tmp/*
 
 .claude/*
-cocoon-test
+/cocoon-test
 
 # Local scratch / test scripts (not for upstream)
 /scratch/
-storebench
+/storebench

.goreleaser.yml

@@ -38,7 +38,7 @@ builds:
 
 archives:
   - id: release
-    builds:
+    ids:
       - cocoon-linux
     files:
       - LICENSE*
@@ -53,7 +53,7 @@ archives:
       {{- if .Arm }}v{{ .Arm }}{{ end -}}
 
   - id: debug
-    builds:
+    ids:
       - cocoon-debug
     files:
       - LICENSE*
@@ -69,7 +69,7 @@ archives:
 
   # Unversioned alias so releases/latest/download/cocoon_Linux_x86_64.tar.gz resolves.
   - id: release-latest
-    builds:
+    ids:
       - cocoon-linux
     files:
       - LICENSE*

Makefile

@@ -66,7 +66,7 @@ build: ## Build cocoon binary
 # --- Testing ---
 
 test: vet ## Run tests with race detection and coverage
-	go test -race -timeout 120s -count=1 -cover -coverprofile=coverage.out ./...
+	go test -race -timeout 20m -count=1 -cover -coverprofile=coverage.out ./...
 
 coverage: test ## Generate and display coverage report
 	go tool cover -func=coverage.out
@@ -93,7 +93,7 @@ fmt: gofumpt goimports ## Format code with gofumpt and goimports
 
 fmt-check: gofumpt goimports ## Check formatting (fails if files need formatting)
 	@test -z "$$($(GOFMT) -extra -l .)" || { echo "Files need formatting (gofumpt):"; $(GOFMT) -extra -l .; exit 1; }
-	@test -z "$$($(GOIMPORTS) -l .)" || { echo "Files need formatting (goimports):"; $(GOIMPORTS) -l .; exit 1; }
+	@test -z "$$($(GOIMPORTS) -l --local 'github.com/cocoonstack/cocoon' .)" || { echo "Files need formatting (goimports):"; $(GOIMPORTS) -l --local 'github.com/cocoonstack/cocoon' .; exit 1; }
 
 # --- Maintenance ---
 

README.md

@@ -20,12 +20,12 @@ Lightweight MicroVM engine with dual hypervisor backends: [Cloud Hypervisor](htt
 ## Quick Start
 
 ```bash
-# Install the binary (Linux amd64; arm64: cocoon_0.7.5_Linux_arm64.tar.gz)
-curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.5/cocoon_0.7.5_Linux_x86_64.tar.gz
+# Install the binary (Linux amd64; arm64: cocoon_0.7.6_Linux_arm64.tar.gz)
+curl -fsSL -o cocoon.tar.gz https://github.com/cocoonstack/cocoon/releases/download/v0.7.6/cocoon_0.7.6_Linux_x86_64.tar.gz
 tar -xzf cocoon.tar.gz && install -m 0755 cocoon /usr/local/bin/
 
 # One-time environment setup (installs CH, firmware, CNI plugins)
-curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.5/doctor/check.sh
+curl -fsSL -o cocoon-check https://raw.githubusercontent.com/cocoonstack/cocoon/refs/tags/v0.7.6/doctor/check.sh
 install -m 0755 cocoon-check /usr/local/bin/ && sudo cocoon-check --upgrade
 
 # Pull an image and run a VM

cgroup/cgroup.go

@@ -23,8 +23,7 @@ const (
 	// Root is the cgroup v2 unified hierarchy mount point.
 	Root = "/sys/fs/cgroup"
 	// DefaultParent holds every per-VM scope unless cgroup_parent overrides it.
-	DefaultParent = "cocoon.slice"
-	// defaultPeriodUs is the kernel's default cpu.max period.
+	DefaultParent   = "cocoon.slice"
 	defaultPeriodUs = 100000
 	// AutoCPUSet asks launch to place the VM on the least-loaded last-level-cache domain.
 	AutoCPUSet = "auto"
@@ -153,7 +152,7 @@ func ScopeDir(parentDir, vmID string) string {
 	return filepath.Join(parentDir, scopePrefix+vmID+scopeSuffix)
 }
 
-// Prepare creates or reconfigures vmID's scope and returns its opened directory for CLONE_INTO_CGROUP; idempotent, so a relaunch reuses a scope its dying predecessor still occupies. deferQuota leaves the ceiling at max for the provisioning window — weight, fence, and placement still apply — until Arm sets the finite quota.
+// Prepare creates or reconfigures vmID's scope and returns its directory for CLONE_INTO_CGROUP; idempotent.
 func Prepare(parentDir, fence, vmID string, k Knobs, deferQuota bool) (*os.File, error) {
 	if err := ensureParent(parentDir, fence, k.CPUSet != ""); err != nil {
 		return nil, err
@@ -189,7 +188,7 @@ func Prepare(parentDir, fence, vmID string, k Knobs, deferQuota bool) (*os.File,
 	return scope, nil
 }
 
-// Arm sets the finite quota on a scope prepared with deferQuota; called after memory load, before resume, so the guest never runs uncapped. Idempotent — a retry after a partial failure converges.
+// Arm sets the finite quota on a scope prepared with deferQuota, which keeps the ceiling at max until now; call it after memory load, before resume.
 func Arm(parentDir, vmID string, k Knobs) error {
 	return armQuota(ScopeDir(parentDir, vmID), k)
 }

cgroup/topology.go

@@ -57,7 +57,7 @@ func ReadTopology(root string, fence []int) (*Topology, error) {
 	return t, nil
 }
 
-// Place picks the least-loaded domain and, for two or more queues, one hardware thread per queue inside it: distinct cores before SMT siblings, fewer pins than queues once the domain runs out.
+// Place picks the least-loaded domain and, for 2+ queues, one thread per queue, preferring distinct cores.
 func (t *Topology) Place(queues int, load map[int]int) (domain, pins []int) {
 	pick := 0
 	best := sumLoad(t.domains[0], load)
@@ -76,8 +76,8 @@ func (t *Topology) Place(queues int, load map[int]int) (domain, pins []int) {
 	for _, c := range t.cores[pick] {
 		core := slices.Clone(c)
 		slices.SortFunc(core, byLoad)
-		for i, c := range core {
-			layer[c] = i
+		for i, cpu := range core {
+			layer[cpu] = i
 		}
 		ranked = append(ranked, core...)
 	}

cmd/cliutil/cliutil.go

@@ -92,7 +92,7 @@ func FormatSize(bytes int64) string {
 	return units.BytesSize(float64(bytes))
 }
 
-// The discarded error on each Flag* accessor means "no such flag", which the command tree makes impossible.
+// A flag absent on this command reads as its zero value.
 func FlagBool(cmd *cobra.Command, name string) bool { v, _ := cmd.Flags().GetBool(name); return v }
 
 func FlagInt(cmd *cobra.Command, name string) int { v, _ := cmd.Flags().GetInt(name); return v }

cmd/core/init.go

@@ -130,7 +130,7 @@ func InitSnapshot(ctx context.Context, conf *config.Config, opts ...localfile.Op
 	return s, nil
 }
 
-// PinEnvelopeBlobs locks envelope-sourced pins (clone/restore --from-dir, snapshot import) on the owning backend; digests with no local blob are skipped.
+// PinEnvelopeBlobs locks envelope-sourced blob pins on the owning backend, skipping digests with no local blob.
 func PinEnvelopeBlobs(ctx context.Context, conf *config.Config, blobIDs map[string]struct{}) (func(), error) {
 	if len(blobIDs) == 0 {
 		return func() {}, nil
@@ -175,7 +175,7 @@ func wireHypervisor[H networkedHypervisor](newFn func(*config.Config, metering.R
 	}
 }
 
-// pinnedElsewhere unions VM and snapshot blob pins for image GC's under-lock recheck; the backends build once, on the first candidate blob.
+// pinnedElsewhere unions VM and snapshot blob pins for image GC's under-lock recheck.
 func pinnedElsewhere(conf *config.Config) imagebackend.PinRecheck {
 	var (
 		once    sync.Once

cmd/core/metastore.go

@@ -2,6 +2,7 @@ package core
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 	"slices"
 	"sync"
@@ -37,7 +38,7 @@ var (
 	metaStore meta.Store
 	metaErr   error
 
-	// vmTables maps the legacy vms.json fields onto the vms-namespace tables; the json field names are engine knowledge and live only here.
+	// vmTables maps legacy vms.json fields onto the vms-namespace tables.
 	vmTables = metajson.TableCodec{Specs: []metajson.TableSpec{
 		{Key: "vms", Table: hypervisor.TableRecords},
 		{Key: "names", Table: hypervisor.TableNames},
@@ -68,7 +69,7 @@ func MetaNamespaces() []metasqlite.Namespace {
 	}
 }
 
-// MetaJSONNamespaces declares the json engine's namespace set: legacy file locations and field mappings, consumed by open and by conversion.
+// MetaJSONNamespaces declares the json engine's namespaces.
 func MetaJSONNamespaces(conf *config.Config) []metajson.Namespace {
 	chCfg := cloudhypervisor.NewConfig(conf)
 	fcCfg := firecracker.NewConfig(conf)
@@ -95,7 +96,7 @@ func MetaDBPath(conf *config.Config) string {
 	return filepath.Join(conf.RootDir, "meta", metasqlite.DBFileName)
 }
 
-// ResolveMetaBackend returns the effective engine: an explicit setting wins, then an existing store binds (meta.db → sqlite, legacy json files → json), and a fresh root gets sqlite.
+// ResolveMetaBackend returns the effective engine: explicit setting, then the existing store, else sqlite.
 func ResolveMetaBackend(conf *config.Config) string {
 	if conf.MetaBackend != "" {
 		return conf.MetaBackend
@@ -116,7 +117,7 @@ func LegacyJSONPresent(conf *config.Config) bool {
 	})
 }
 
-// MetaStore builds the process-wide meta store once, one store for every namespace and backend; the engine follows ResolveMetaBackend, and a fresh sqlite root bootstraps itself.
+// MetaStore builds the process-wide meta store once.
 func MetaStore(conf *config.Config) (meta.Store, error) {
 	metaOnce.Do(func() {
 		// Bootstrap owns its context: the store outlives any single caller, and a canceled first caller must not poison the Once for everyone.
@@ -132,6 +133,10 @@ func MetaStore(conf *config.Config) (meta.Store, error) {
 			metaStore, metaErr = openSQLiteStore(ctx, conf, dbPath)
 			return
 		}
+		if utils.FileExists(dbPath) {
+			metaErr = fmt.Errorf("meta_backend is json but %s holds sqlite metadata; run `cocoon meta convert`", dbPath)
+			return
+		}
 		if conf.MetaBackend == "" {
 			log.WithFunc("core.MetaStore").Info(ctx, "legacy json meta store in use; `cocoon meta convert` upgrades it to sqlite")
 		}
@@ -175,7 +180,7 @@ func ImageJSONNamespace(c *images.BaseConfig) metajson.Namespace {
 	}
 }
 
-// openSQLiteStore opens the sqlite engine, bootstrapping a fresh root or repairing a crashed bootstrap; a legacy json root never bootstraps — that would shadow its data.
+// openSQLiteStore never bootstraps over legacy json data, which a fresh store would shadow.
 func openSQLiteStore(ctx context.Context, conf *config.Config, dbPath string) (meta.Store, error) {
 	if !LegacyJSONPresent(conf) {
 		if err := metasqlite.InitIfMissing(ctx, dbPath, MetaNamespaces()...); err != nil {

cmd/core/metastore_test.go

@@ -96,6 +96,22 @@ func TestMetaStoreRepairsCrashedBootstrap(t *testing.T) {
 	resetMetaStore()
 }
 
+func TestMetaStoreRefusesJSONOverSQLiteData(t *testing.T) {
+	resetMetaStore()
+	conf := testConf(t)
+	if _, err := MetaStore(conf); err != nil {
+		t.Fatalf("bootstrap sqlite root: %v", err)
+	}
+	CloseMetaStore(t.Context())
+	resetMetaStore()
+	conf.MetaBackend = config.MetaBackendJSON
+	if _, err := MetaStore(conf); err == nil {
+		t.Fatal("json backend opened over a live meta.db and would hide its records")
+	}
+	CloseMetaStore(t.Context())
+	resetMetaStore()
+}
+
 func TestResolveMetaBackend(t *testing.T) {
 	conf := testConf(t)
 	if b := ResolveMetaBackend(conf); b != "sqlite" {

cmd/core/metering_test.go

@@ -21,7 +21,6 @@ func TestBuildRecorderBackends(t *testing.T) {
 		{"explicit file", config.MeteringFile, (*meteringfile.Recorder)(nil)},
 		{"nop", config.MeteringNop, metering.NopRecorder{}},
 		{"stderr", config.MeteringStderr, (*meteringstderr.Recorder)(nil)},
-		{"unknown falls back to nop", "kafka", metering.NopRecorder{}},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/core/utils.go

@@ -152,31 +152,31 @@ func CloseOnCancel(ctx context.Context, c io.Closer) func() bool {
 }
 
 // SnapshotNameFlags reads the --name/--description pair and rejects a taken name; an empty name passes.
-func SnapshotNameFlags(ctx context.Context, cmd *cobra.Command, snapBackend snapshot.Snapshot) (name, description string, err error) {
-	name, _ = cmd.Flags().GetString("name")
+func SnapshotNameFlags(ctx context.Context, cmd *cobra.Command, snapBackend snapshot.Snapshot) (snapName, description string, err error) {
+	snapName, _ = cmd.Flags().GetString("name")
 	description, _ = cmd.Flags().GetString("description")
-	if err = (&types.SnapshotConfig{Name: name}).Validate(); err != nil {
+	if err = (&types.SnapshotConfig{Name: snapName}).Validate(); err != nil {
 		return "", "", err
 	}
-	if name == "" {
-		return name, description, nil
+	if snapName == "" {
+		return snapName, description, nil
 	}
-	// The name index, not Inspect: a killed save leaves a pending record still holding the name that Inspect reports as not-found, and passing that preflight wastes the whole capture before the insert rejects it.
+	// Use the name index: Inspect misses a pending record from a killed save that still holds the name.
 	nh, ok := snapBackend.(snapshot.NameHolder)
 	if !ok {
 		return "", "", fmt.Errorf("backend %s has no snapshot name index", snapBackend.Type())
 	}
-	id, held, err := nh.NameOwner(ctx, name)
+	id, held, err := nh.NameOwner(ctx, snapName)
 	if err != nil {
 		return "", "", fmt.Errorf("check snapshot name: %w", err)
 	}
 	if held {
-		return "", "", fmt.Errorf("snapshot name %q already exists (held by %s)", name, id)
+		return "", "", fmt.Errorf("snapshot name %q already exists (held by %s)", snapName, id)
 	}
-	return name, description, nil
+	return snapName, description, nil
 }
 
-// PersistSnapshotDir stores a finalized capture dir, moving it in place when srcDir shares a filesystem with the backend and streaming a tar otherwise; srcDir is consumed on every path.
+// PersistSnapshotDir stores a finalized capture dir; srcDir is consumed on every path.
 func PersistSnapshotDir(ctx context.Context, snapBackend snapshot.Snapshot, cfg *types.SnapshotConfig, srcDir, name, description string) (string, error) {
 	logger := log.WithFunc("core.PersistSnapshotDir")
 	cfg.Name = name
@@ -234,7 +234,7 @@ func resolveOwner[T typed](backends []T, ref string, found func(T) (bool, error)
 	}
 }
 
-// resolveVMOwner returns the owning hypervisor and resolved *types.VM so callers use vm.ID instead of re-resolving the raw ref.
+// resolveVMOwner returns the owning hypervisor and the resolved VM.
 func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref string) (hypervisor.Hypervisor, *types.VM, error) {
 	var resolved *types.VM
 	owner, err := resolveOwner(
@@ -255,7 +255,7 @@ func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref str
 	return owner, resolved, err
 }
 
-// validateRefShape rejects URL/OCI ref mismatches early so backends don't surface misleading downstream errors.
+// validateRefShape rejects URL/OCI ref mismatches before any backend runs.
 func validateRefShape(ref, imageType string) error {
 	switch imageType {
 	case types.ImageTypeCloudImg:

cmd/core/utils_test.go

@@ -149,7 +149,7 @@ func TestReconcileStateClearsRuntimePathsOnStaleRunning(t *testing.T) {
 	}
 }
 
-// directErrSnap is a DirectCreator whose CreateFromDir always fails; the embedded interface panics on any other call, so the test proves the failure path alone.
+// directErrSnap fails CreateFromDir; any other call panics on the nil embedded Snapshot.
 type directErrSnap struct {
 	snapshot.Snapshot
 }

cmd/core/vmconfig.go

@@ -67,7 +67,7 @@ func VMConfigFromFlags(cmd *cobra.Command, image string) (*types.VMConfig, error
 	return cfg, nil
 }
 
-// CloneVMConfigFromFlags builds VMConfig for a clone. The snapshot's cgroup knobs record its source VM's policy and are never applied; the clone's policy comes from flags alone.
+// CloneVMConfigFromFlags builds a clone's VMConfig; cgroup knobs come from flags alone, never the snapshot.
 func CloneVMConfigFromFlags(cmd *cobra.Command, snapCfg types.SnapshotConfig) (*types.VMConfig, error) {
 	restoreMode, err := restoreModeFromFlags(cmd)
 	if err != nil {

cmd/daemon/handler.go

@@ -22,6 +22,10 @@ type Handler struct {
 
 func (h Handler) Run(cmd *cobra.Command, _ []string) error {
 	ctx, conf := h.Init(cmd)
+	dconf, err := daemonConfig(cmd, conf)
+	if err != nil {
+		return err
+	}
 	store, err := cmdcore.MetaStore(conf)
 	if err != nil {
 		return err
@@ -34,10 +38,6 @@ func (h Handler) Run(cmd *cobra.Command, _ []string) error {
 	for _, hyper := range hypers {
 		supervisors = append(supervisors, hyper)
 	}
-	dconf, err := daemonConfig(cmd, conf)
-	if err != nil {
-		return err
-	}
 	return cocoond.New(dconf, store, supervisors).Run(ctx)
 }
 

cmd/images/handler_test.go

@@ -139,7 +139,7 @@ func TestDetectReader_GzipPreservesFullContent(t *testing.T) {
 
 func TestDetectLocalImportSource(t *testing.T) {
 	dir := t.TempDir()
-	qcow2Path := writeTempFile(t, dir, "image.qcow2", append(utils.Qcow2Magic, make([]byte, 32)...))
+	qcow2Path := writeTempFile(t, dir, "image.qcow2", slices.Concat(utils.Qcow2Magic, make([]byte, 32)))
 	tarPath := writeTempFile(t, dir, "image.tar", tarWrap(t, "payload.txt", []byte("payload")))
 	gzipTarPath := writeTempFile(t, dir, "image.tar.gz", gzipWrap(t, tarWrap(t, "payload.txt", []byte("payload"))))
 	invalidPath := writeTempFile(t, dir, "invalid.bin", []byte("not an archive"))
@@ -177,7 +177,7 @@ func TestDetectLocalImportSource(t *testing.T) {
 
 func TestPlanLocalImportDetectsKindFromFirstFile(t *testing.T) {
 	dir := t.TempDir()
-	part1 := writeTempFile(t, dir, "part-1.qcow2", append(utils.Qcow2Magic, make([]byte, 8)...))
+	part1 := writeTempFile(t, dir, "part-1.qcow2", slices.Concat(utils.Qcow2Magic, make([]byte, 8)))
 	part2 := writeTempFile(t, dir, "part-2.qcow2", []byte("second-part"))
 
 	kind, err := planLocalImport([]string{part1, part2})

cmd/images/import.go

@@ -113,7 +113,7 @@ func importCloudimg(ctx context.Context, conf *config.Config, name, downloadMsg
 	return nil
 }
 
-func importOCI(ctx context.Context, conf *config.Config, name string, pullMsg func(int) string, do func(*oci.OCI, progress.Tracker) error) error {
+func importOCI(ctx context.Context, conf *config.Config, name string, pullMsg pullMsgFunc, do func(*oci.OCI, progress.Tracker) error) error {
 	store, _, err := cmdcore.InitImageBackendsForPull(ctx, conf)
 	if err != nil {
 		return err