style: full-repo SKILL.md audit closed-loop

Multi-round senior-review + /simplify pass on all 190 .go files:

- hypervisor.BalloonSize(memoryBytes, windows) consolidates 5 sites
  (CH/FC args, CH config patch, FC start, debug printer); fixed bugs:
  FC start.go missed Windows gate, debug.go default was 50% not 25%,
  printCommonCHArgs emitted --balloon unconditionally
- backend.SnapshotSequence: build the http.Client once and pass it via
  SnapshotSpec so HTTP keep-alive reuses the API socket connection
  across pause/capture/resume
- firecracker.DevPath exported; cmd/vm/debug.go printFCDebug uses it
  to fix 'a'+nLayers overflow at >=26 layers
- snapshot/localfile clone-MAC + delete-multiple bugs fixed; per-id
  atomic Delete preserves invariants under partial failure
- cmd/core/helpers.go RouteRefs returns resolved full IDs so
  network-recovery / lifecycle batchers never re-resolve prefixes;
  resolveVMOwner guards (nil, nil) Inspect contract
- log tags repo-wide: every log.WithFunc("pkg.funcName") now matches
  the enclosing func (cmd.images.Import, cmd.images.RM, oci.import*,
  oci.handleCachedLayer, cloudimg.import*, storage.json.withLocked)
- Comment hygiene: godoc starts with the func/type name; drop
  walkthrough narratives, decorative // --- foo --- dividers (5 in
  impl + 62 in tests), history narration ("no longer", "previously"),
  WHAT-restate comments
- Layout: utility funcs below struct methods (BalloonSize), private
  methods after public (resolveRecord), CH/FC functionality grouped
  contiguously, types at top of file (chDebugSpec, tarImportJob)
- Quality: drop chDebugArgs (parameter sprawl with chDebugSpec),
  collapse selfHealBootFiles to take layerJob instead of 8 positional
  fields, drop redundant Seek-to-0 after os.Open, drop SA4010 dead
  append in test, drop dead IsTarFile / IsQcow2File exports
- Error wrap: %w: %w multi-wrap at extend.go:231; bdfFromSysfsPath
  uses CutPrefix's bool

make lint dual-platform 0 issues, go vet clean, staticcheck clean
(except 1 intentional tar.TypeRegA //nolint), gofumpt/goimports
silent, all tests pass.

Author: CMGS

cmd/core/helpers.go

@@ -32,17 +32,17 @@ import (
 	"github.com/cocoonstack/cocoon/utils"
 )
 
+var hypervisorFactories = []hypervisorFactory{
+	{config.HypervisorCH, func(c *config.Config) (hypervisor.Hypervisor, error) { return cloudhypervisor.New(c) }},
+	{config.HypervisorFirecracker, func(c *config.Config) (hypervisor.Hypervisor, error) { return firecracker.New(c) }},
+}
+
 // hypervisorFactory keeps backend lookup and iteration order together.
 type hypervisorFactory struct {
 	typ  config.HypervisorType
 	ctor func(*config.Config) (hypervisor.Hypervisor, error)
 }
 
-var hypervisorFactories = []hypervisorFactory{
-	{config.HypervisorCH, func(c *config.Config) (hypervisor.Hypervisor, error) { return cloudhypervisor.New(c) }},
-	{config.HypervisorFirecracker, func(c *config.Config) (hypervisor.Hypervisor, error) { return firecracker.New(c) }},
-}
-
 // BaseHandler provides shared config access for all command handlers.
 type BaseHandler struct {
 	ConfProvider func() *config.Config
@@ -52,7 +52,6 @@ func NewBaseHandler(conf *config.Config) BaseHandler {
 	return BaseHandler{ConfProvider: func() *config.Config { return conf }}
 }
 
-// Init returns command context and validated config.
 func (h BaseHandler) Init(cmd *cobra.Command) (context.Context, *config.Config, error) {
 	conf, err := h.Conf()
 	if err != nil {
@@ -61,7 +60,6 @@ func (h BaseHandler) Init(cmd *cobra.Command) (context.Context, *config.Config,
 	return CommandContext(cmd), conf, nil
 }
 
-// Conf validates and returns the config.
 func (h BaseHandler) Conf() (*config.Config, error) {
 	if h.ConfProvider == nil {
 		return nil, fmt.Errorf("config provider is nil")
@@ -73,7 +71,7 @@ func (h BaseHandler) Conf() (*config.Config, error) {
 	return conf, nil
 }
 
-// CommandContext returns command context, fallback to Background.
+// CommandContext returns ctx from cmd or Background as fallback.
 func CommandContext(cmd *cobra.Command) context.Context {
 	if cmd != nil && cmd.Context() != nil {
 		return cmd.Context()
@@ -142,7 +140,8 @@ func FindHypervisor(ctx context.Context, conf *config.Config, ref string) (hyper
 	if err != nil {
 		return nil, err
 	}
-	return resolveVMOwner(ctx, hypers, ref)
+	owner, _, err := resolveVMOwner(ctx, hypers, ref)
+	return owner, err
 }
 
 func ListAllVMs(ctx context.Context, hypers []hypervisor.Hypervisor) ([]*types.VM, error) {
@@ -157,14 +156,18 @@ func ListAllVMs(ctx context.Context, hypers []hypervisor.Hypervisor) ([]*types.V
 	return all, nil
 }
 
+// RouteRefs resolves each user ref (full ID, name, or ID prefix) to its
+// owning hypervisor and returns a map of owner → resolved full VM IDs.
+// Returning full IDs (not raw refs) ensures downstream code — including
+// network recovery and lifecycle batchers — never has to re-resolve prefixes.
 func RouteRefs(ctx context.Context, hypers []hypervisor.Hypervisor, refs []string) (map[hypervisor.Hypervisor][]string, error) {
 	result := map[hypervisor.Hypervisor][]string{}
 	for _, ref := range refs {
-		owner, err := resolveVMOwner(ctx, hypers, ref)
+		owner, vm, err := resolveVMOwner(ctx, hypers, ref)
 		if err != nil {
 			return nil, err
 		}
-		result[owner] = append(result[owner], ref)
+		result[owner] = append(result[owner], vm.ID)
 	}
 	return result, nil
 }
@@ -219,13 +222,9 @@ func ResolveImage(ctx context.Context, backends []imagebackend.Images, vmCfg *ty
 	return storageConfigs, bootCfg, nil
 }
 
-// EnsureImage checks whether the exact base image (by digest) required by
-// vmCfg exists locally and pulls it if missing. Inspect uses ImageDigest
-// to match the exact version recorded at snapshot time; Pull uses Image
-// (tag/URL) as the registry reference, then verifies the pulled digest.
-//
-// For imported images (not pullable from a registry), a warning is logged
-// and the caller proceeds — VerifyBaseFiles will catch the actual error.
+// EnsureImage pulls the digest-pinned base image if missing locally; warns
+// (rather than fails) for imported images so VerifyBaseFiles surfaces the
+// real error.
 func EnsureImage(ctx context.Context, backends []imagebackend.Images, vmCfg *types.VMConfig) {
 	if vmCfg.Image == "" || vmCfg.ImageType == "" {
 		return
@@ -270,27 +269,41 @@ func EnsureImage(ctx context.Context, backends []imagebackend.Images, vmCfg *typ
 }
 
 func ResolveImageOwner(ctx context.Context, backends []imagebackend.Images, ref string) (imagebackend.Images, error) {
-	var matches []imagebackend.Images
-	for _, b := range backends {
+	return resolveOwner(backends, ref, func(b imagebackend.Images) (bool, error) {
 		img, err := b.Inspect(ctx, ref)
+		return img != nil, err
+	},
+		fmt.Errorf("image %q: not found in any backend", ref),
+		fmt.Errorf("image %s: %w", ref, imagebackend.ErrAmbiguous),
+	)
+}
+
+// resolveOwner returns the unique backend among backends for which found
+// reports true. notFound is returned when zero match; ambiguous wraps the
+// caller-supplied error and lists the matched backend types.
+func resolveOwner[T interface{ Type() string }](backends []T, ref string, found func(T) (bool, error), notFound, ambiguous error) (T, error) {
+	var matches []T
+	var zero T
+	for _, b := range backends {
+		ok, err := found(b)
 		if err != nil {
-			return nil, fmt.Errorf("inspect %s in %s: %w", ref, b.Type(), err)
+			return zero, fmt.Errorf("inspect %s in %s: %w", ref, b.Type(), err)
 		}
-		if img != nil {
+		if ok {
 			matches = append(matches, b)
 		}
 	}
 	switch len(matches) {
 	case 0:
-		return nil, fmt.Errorf("image %q: not found in any backend", ref)
+		return zero, notFound
 	case 1:
 		return matches[0], nil
 	default:
 		names := make([]string, len(matches))
 		for i, b := range matches {
 			names[i] = b.Type()
 		}
-		return nil, fmt.Errorf("image %s: %w (backends: %s)", ref, imagebackend.ErrAmbiguous, strings.Join(names, ", "))
+		return zero, fmt.Errorf("%w (backends: %s)", ambiguous, strings.Join(names, ", "))
 	}
 }
 
@@ -502,31 +515,27 @@ func findHypervisorFactory(typ config.HypervisorType) func(*config.Config) (hype
 	return nil
 }
 
-func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref string) (hypervisor.Hypervisor, error) {
-	var matches []hypervisor.Hypervisor
-	for _, h := range hypers {
-		_, resolveErr := h.Inspect(ctx, ref)
-		if resolveErr == nil {
-			matches = append(matches, h)
-			continue
+// resolveVMOwner returns the owning hypervisor and the resolved *types.VM.
+// Reusing the Inspect we already pay for fixes prefix-resolution downstream:
+// callers can use vm.ID instead of the user's raw ref (which may be a prefix
+// or name).
+func resolveVMOwner(ctx context.Context, hypers []hypervisor.Hypervisor, ref string) (hypervisor.Hypervisor, *types.VM, error) {
+	var resolved *types.VM
+	owner, err := resolveOwner(hypers, ref, func(h hypervisor.Hypervisor) (bool, error) {
+		vm, err := h.Inspect(ctx, ref)
+		if err == nil && vm != nil {
+			resolved = vm
+			return true, nil
 		}
-		if errors.Is(resolveErr, hypervisor.ErrNotFound) {
-			continue
+		if err != nil && !errors.Is(err, hypervisor.ErrNotFound) {
+			return false, err
 		}
-		return nil, fmt.Errorf("inspect %s in %s: %w", ref, h.Type(), resolveErr)
-	}
-	switch len(matches) {
-	case 0:
-		return nil, fmt.Errorf("vm %s: %w", ref, hypervisor.ErrNotFound)
-	case 1:
-		return matches[0], nil
-	default:
-		names := make([]string, len(matches))
-		for i, h := range matches {
-			names[i] = h.Type()
-		}
-		return nil, fmt.Errorf("vm %s: %w (backends: %s)", ref, hypervisor.ErrAmbiguous, strings.Join(names, ", "))
-	}
+		return false, nil
+	},
+		fmt.Errorf("vm %s: %w", ref, hypervisor.ErrNotFound),
+		fmt.Errorf("vm %s: %w", ref, hypervisor.ErrAmbiguous),
+	)
+	return owner, resolved, err
 }
 
 // sanitizeVMName derives a safe VM name from an image reference.

cmd/images/import.go

@@ -6,6 +6,7 @@ import (
 	"bytes"
 	"compress/gzip"
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -26,7 +27,7 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	logger := log.WithFunc("cmd.image.import")
+	logger := log.WithFunc("cmd.images.Import")
 
 	name := args[0]
 	files := args[1:]
@@ -40,7 +41,7 @@ func (h Handler) Import(cmd *cobra.Command, args []string) error {
 }
 
 func (h Handler) importLocalFiles(ctx context.Context, conf *config.Config, name string, files ...string) error {
-	logger := log.WithFunc("cmd.image.import")
+	logger := log.WithFunc("cmd.images.importLocalFiles")
 
 	plan, err := planLocalImport(files)
 	if err != nil {
@@ -76,9 +77,6 @@ func (h Handler) importLocalStream(ctx context.Context, conf *config.Config, nam
 	}
 	defer f.Close() //nolint:errcheck
 
-	if _, err := f.Seek(0, io.SeekStart); err != nil {
-		return fmt.Errorf("seek %s: %w", filePath, err)
-	}
 	logger := log.WithFunc("cmd.images.importLocalStream")
 	logger.Infof(ctx, "importing from %s ...", filePath)
 	return h.importFromReader(ctx, conf, name, f)
@@ -224,7 +222,7 @@ func detectLocalImportSource(filePath string) (importSourceKind, error) {
 
 	var magic [4]byte
 	n, readErr := io.ReadFull(f, magic[:])
-	if readErr != nil && readErr != io.EOF && readErr != io.ErrUnexpectedEOF {
+	if readErr != nil && !errors.Is(readErr, io.EOF) && !errors.Is(readErr, io.ErrUnexpectedEOF) {
 		return 0, fmt.Errorf("peek %s: %w", filePath, readErr)
 	}
 

cmd/images/manage.go

@@ -56,7 +56,7 @@ func (h Handler) RM(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	logger := log.WithFunc("cmd.image.rm")
+	logger := log.WithFunc("cmd.images.RM")
 	backends, err := cmdcore.InitImageBackends(ctx, conf)
 	if err != nil {
 		return err

cmd/root.go

@@ -82,7 +82,6 @@ var (
 	}()
 )
 
-// Execute is the main entry point called from main.go.
 func Execute(ctx context.Context) error {
 	ctx, cancel := signal.NotifyContext(ctx, syscall.SIGINT, syscall.SIGTERM)
 	defer cancel()

cmd/vm/attach.go

@@ -1,6 +1,7 @@
 package vm
 
 import (
+	"context"
 	"errors"
 	"fmt"
 
@@ -14,18 +15,10 @@ import (
 )
 
 func (h Handler) FsAttach(cmd *cobra.Command, args []string) error {
-	ctx, conf, err := h.Init(cmd)
+	ctx, a, err := resolveAttacher[fs.Attacher](h, cmd, args, "fs attach", fs.ErrUnsupportedBackend)
 	if err != nil {
 		return err
 	}
-	hyper, err := cmdcore.FindHypervisor(ctx, conf, args[0])
-	if err != nil {
-		return fmt.Errorf("fs attach: %w", err)
-	}
-	a, ok := hyper.(fs.Attacher)
-	if !ok {
-		return fmt.Errorf("fs attach: backend %s: %w", hyper.Type(), fs.ErrUnsupportedBackend)
-	}
 	socket, _ := cmd.Flags().GetString("socket")
 	tag, _ := cmd.Flags().GetString("tag")
 	numQ, _ := cmd.Flags().GetInt("num-queues")
@@ -42,18 +35,10 @@ func (h Handler) FsAttach(cmd *cobra.Command, args []string) error {
 }
 
 func (h Handler) FsDetach(cmd *cobra.Command, args []string) error {
-	ctx, conf, err := h.Init(cmd)
+	ctx, a, err := resolveAttacher[fs.Attacher](h, cmd, args, "fs detach", fs.ErrUnsupportedBackend)
 	if err != nil {
 		return err
 	}
-	hyper, err := cmdcore.FindHypervisor(ctx, conf, args[0])
-	if err != nil {
-		return fmt.Errorf("fs detach: %w", err)
-	}
-	a, ok := hyper.(fs.Attacher)
-	if !ok {
-		return fmt.Errorf("fs detach: backend %s: %w", hyper.Type(), fs.ErrUnsupportedBackend)
-	}
 	tag, _ := cmd.Flags().GetString("tag")
 	if err := a.FsDetach(ctx, args[0], tag); err != nil {
 		return classifyAttachErr(err)
@@ -66,18 +51,10 @@ func (h Handler) FsDetach(cmd *cobra.Command, args []string) error {
 }
 
 func (h Handler) DeviceAttach(cmd *cobra.Command, args []string) error {
-	ctx, conf, err := h.Init(cmd)
+	ctx, a, err := resolveAttacher[vfio.Attacher](h, cmd, args, "device attach", vfio.ErrUnsupportedBackend)
 	if err != nil {
 		return err
 	}
-	hyper, err := cmdcore.FindHypervisor(ctx, conf, args[0])
-	if err != nil {
-		return fmt.Errorf("device attach: %w", err)
-	}
-	a, ok := hyper.(vfio.Attacher)
-	if !ok {
-		return fmt.Errorf("device attach: backend %s: %w", hyper.Type(), vfio.ErrUnsupportedBackend)
-	}
 	pci, _ := cmd.Flags().GetString("pci")
 	id, _ := cmd.Flags().GetString("id")
 	deviceID, err := a.DeviceAttach(ctx, args[0], vfio.Spec{PCI: pci, ID: id})
@@ -92,18 +69,10 @@ func (h Handler) DeviceAttach(cmd *cobra.Command, args []string) error {
 }
 
 func (h Handler) DeviceDetach(cmd *cobra.Command, args []string) error {
-	ctx, conf, err := h.Init(cmd)
+	ctx, a, err := resolveAttacher[vfio.Attacher](h, cmd, args, "device detach", vfio.ErrUnsupportedBackend)
 	if err != nil {
 		return err
 	}
-	hyper, err := cmdcore.FindHypervisor(ctx, conf, args[0])
-	if err != nil {
-		return fmt.Errorf("device detach: %w", err)
-	}
-	a, ok := hyper.(vfio.Attacher)
-	if !ok {
-		return fmt.Errorf("device detach: backend %s: %w", hyper.Type(), vfio.ErrUnsupportedBackend)
-	}
 	id, _ := cmd.Flags().GetString("id")
 	if err := a.DeviceDetach(ctx, args[0], id); err != nil {
 		return classifyAttachErr(err)
@@ -115,6 +84,26 @@ func (h Handler) DeviceDetach(cmd *cobra.Command, args []string) error {
 	return nil
 }
 
+// resolveAttacher resolves args[0] to a hypervisor implementing A
+// (fs.Attacher / vfio.Attacher). op ("fs attach", "device detach") prefixes
+// both error wraps so callers see the operation, not just the type-assert.
+func resolveAttacher[A any](h Handler, cmd *cobra.Command, args []string, op string, errUnsupported error) (context.Context, A, error) {
+	var zero A
+	ctx, conf, err := h.Init(cmd)
+	if err != nil {
+		return ctx, zero, err
+	}
+	hyper, err := cmdcore.FindHypervisor(ctx, conf, args[0])
+	if err != nil {
+		return ctx, zero, fmt.Errorf("%s: %w", op, err)
+	}
+	a, ok := hyper.(A)
+	if !ok {
+		return ctx, zero, fmt.Errorf("%s: backend %s: %w", op, hyper.Type(), errUnsupported)
+	}
+	return ctx, a, nil
+}
+
 // classifyAttachErr surfaces ErrNotRunning more clearly than the generic wrap.
 func classifyAttachErr(err error) error {
 	if errors.Is(err, hypervisor.ErrNotRunning) {