fix(vm): retry reseed only on dial failure, not a live agent's rejection

Review follow-ups on the reseed signal:
- reseedVM retried on ANY error, so a version-skew old agent (dial OK,
  reseed frame rejected) burned the full retry budget (~4s) on every clone
  during the rollout window. Retry only failed dials (the re-listen-after-
  resume case); once a live agent answers, its reply is final. Old-agent
  reseed now returns in ~2ms instead of ~4s, verified on bare metal.
- fold refresh+signal into Handler.reseedAfterResume so a caller can't
  signal on a stale (pre-ToVM) VM; move refreshVM beside it and log the
  swallowed inspect error at Debug (was mislabeled 'vsock not configured').
- signalReseed reports whether a reseed was attempted, making the
  Windows/no-vsock skip guards assertable.

Author: CMGS

cmd/vm/reseed.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto/rand"
 	"fmt"
-	"io"
 	"time"
 
 	"github.com/projecteru2/core/log"
@@ -36,8 +35,10 @@ func (h Handler) Reseed(cmd *cobra.Command, args []string) error {
 	return reseedVM(ctx, vm, regenMachineID)
 }
 
-// reseedVM pushes fresh entropy and a CRNG reseed order over vsock, retrying because the
-// guest agent re-listens shortly after a clone/restore resume.
+// reseedVM pushes fresh entropy and a CRNG reseed order over vsock. Only a failed
+// dial is retried — the guest agent re-listens shortly after a clone/restore resume;
+// once a live agent answers, its reply (success, version-skew rejection, or failure)
+// is final, so an old agent isn't billed the whole retry budget.
 func reseedVM(ctx context.Context, vm *types.VM, regenMachineID bool) error {
 	if vm.VsockSocket == "" {
 		return fmt.Errorf("reseed: %w (recreate the VM to enable agent reseed)", ErrVsockNotConfigured)
@@ -47,44 +48,66 @@ func reseedVM(ctx context.Context, vm *types.VM, regenMachineID bool) error {
 		return fmt.Errorf("reseed: generate entropy: %w", err)
 	}
 
-	var err error
+	var dialErr error
 	for attempt := 1; attempt <= reseedMaxAttempts; attempt++ {
-		if err = ctx.Err(); err != nil {
+		if err := ctx.Err(); err != nil {
 			return err
 		}
-		var conn io.ReadWriteCloser
-		if conn, err = dialHybridVsock(ctx, vm.VsockSocket, hypervisor.VsockAgentPort); err == nil {
-			err = client.Reseed(ctx, conn, entropy, regenMachineID)
-			conn.Close() //nolint:errcheck,gosec
-			if err == nil {
-				return nil
+		conn, err := dialHybridVsock(ctx, vm.VsockSocket, hypervisor.VsockAgentPort)
+		if err != nil {
+			dialErr = err
+			if attempt < reseedMaxAttempts {
+				select {
+				case <-ctx.Done():
+					return ctx.Err()
+				case <-time.After(reseedRetryDelay):
+				}
 			}
+			continue
 		}
-		if attempt < reseedMaxAttempts {
-			select {
-			case <-ctx.Done():
-				return ctx.Err()
-			case <-time.After(reseedRetryDelay):
-			}
+		err = client.Reseed(ctx, conn, entropy, regenMachineID)
+		conn.Close() //nolint:errcheck,gosec
+		if err != nil {
+			return fmt.Errorf("reseed: %w", err)
 		}
+		return nil
 	}
-	return fmt.Errorf("reseed: %w", err)
+	return fmt.Errorf("reseed: dial agent: %w", dialErr)
+}
+
+// reseedAfterResume re-inspects then fires the best-effort reseed. Clone/Restore return a
+// *types.VM built in-process that never passed through ToVM, so its VsockSocket is zero;
+// pairing refresh with signal keeps a caller from silently no-op-ing on a stale value.
+func (h Handler) reseedAfterResume(ctx context.Context, hyper hypervisor.Hypervisor, vm *types.VM, regenMachineID bool) {
+	signalReseed(ctx, refreshVM(ctx, hyper, vm), regenMachineID)
 }
 
-// signalReseed is the non-fatal wrapper called from clone/restore paths: skips silently on
-// Windows guests (agent verb is Linux-only) or legacy VMs without vsock, and never fails the
-// calling command on agent version skew.
-func signalReseed(ctx context.Context, vm *types.VM, regenMachineID bool) {
+// signalReseed is the non-fatal wrapper for clone/restore paths: it reports whether a reseed
+// was attempted (false = skipped for a Windows guest or a legacy VM without vsock) and never
+// fails the calling command on agent version skew.
+func signalReseed(ctx context.Context, vm *types.VM, regenMachineID bool) bool {
 	logger := log.WithFunc("cmd.vm.reseed")
 	if vm.Config.Windows {
 		logger.Debug(ctx, "skip reseed signal: Windows guest")
-		return
+		return false
 	}
 	if vm.VsockSocket == "" {
 		logger.Debug(ctx, "skip reseed signal: vsock not configured")
-		return
+		return false
 	}
 	if err := reseedVM(ctx, vm, regenMachineID); err != nil {
 		logger.Warnf(ctx, "reseed signal failed (agent >= v0.1.6 required): %v; run 'cocoon vm reseed %s' after fixing", err, vm.ID)
 	}
+	return true
+}
+
+// refreshVM re-inspects to recover runtime-only fields (VsockSocket, PID, SocketPath) that
+// ToVM sets but Clone/Restore's in-process return value lacks; keeps the original on error.
+func refreshVM(ctx context.Context, hyper hypervisor.Hypervisor, vm *types.VM) *types.VM {
+	info, err := hyper.Inspect(ctx, vm.ID)
+	if err != nil {
+		log.WithFunc("cmd.vm.reseed").Debugf(ctx, "refresh VM %s before reseed: %v", vm.ID, err)
+		return vm
+	}
+	return info
 }

cmd/vm/reseed_test.go

@@ -9,13 +9,18 @@ import (
 )
 
 func TestSignalReseed_SkipsWindows(t *testing.T) {
+	// VsockSocket is set: a regressed Windows guard would fall through to a dial instead of skipping.
 	vm := &types.VM{ID: "vm1", Config: types.VMConfig{Config: types.Config{Windows: true}}, VsockSocket: "/tmp/whatever.sock"}
-	signalReseed(t.Context(), vm, true)
+	if signalReseed(t.Context(), vm, true) {
+		t.Error("attempted reseed on a Windows guest, want skip")
+	}
 }
 
 func TestSignalReseed_SkipsNoVsock(t *testing.T) {
 	vm := &types.VM{ID: "vm1"}
-	signalReseed(t.Context(), vm, true)
+	if signalReseed(t.Context(), vm, true) {
+		t.Error("attempted reseed with no vsock, want skip")
+	}
 }
 
 func TestReseedVM_NoVsock(t *testing.T) {

cmd/vm/run.go

@@ -127,7 +127,7 @@ func (h Handler) Clone(cmd *cobra.Command, args []string) error {
 		rollbackNetwork(ctx, netProvider, vmID)
 		return fmt.Errorf("clone VM: %w", cloneErr)
 	}
-	signalReseed(ctx, refreshVM(ctx, hyper, vm), true)
+	h.reseedAfterResume(ctx, hyper, vm, true)
 
 	if done, jsonErr := cliutil.MaybeOutputJSON(cmd, vm); done {
 		return jsonErr
@@ -197,7 +197,7 @@ func (h Handler) Restore(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return fmt.Errorf("restore: %w", err)
 	}
-	signalReseed(ctx, refreshVM(ctx, hyper, result), false)
+	h.reseedAfterResume(ctx, hyper, result, false)
 
 	if done, jsonErr := cliutil.MaybeOutputJSON(cmd, result); done {
 		return jsonErr
@@ -290,7 +290,7 @@ func (h Handler) cloneFromSrcDir(ctx context.Context, cmd *cobra.Command, conf *
 		rollbackNetwork(ctx, netProvider, vmID)
 		return fmt.Errorf("clone VM: %w", cloneErr)
 	}
-	signalReseed(ctx, refreshVM(ctx, hyper, vm), true)
+	h.reseedAfterResume(ctx, hyper, vm, true)
 
 	if wantJSON {
 		return cliutil.OutputJSON(vm)
@@ -364,7 +364,7 @@ func (h Handler) runDirectRestore(ctx context.Context, cmd *cobra.Command, hyper
 	if err != nil {
 		return fmt.Errorf("restore: %w", err)
 	}
-	signalReseed(ctx, refreshVM(ctx, hyper, result), false)
+	h.reseedAfterResume(ctx, hyper, result, false)
 	if wantJSON {
 		return cliutil.OutputJSON(result)
 	}
@@ -501,16 +501,6 @@ func rollbackNetwork(ctx context.Context, netProvider network.Network, vmID stri
 	}
 }
 
-// refreshVM re-inspects after clone/restore: the value Clone/Restore return is built in-process and
-// never passes through ToVM, so runtime-only fields (VsockSocket, PID, SocketPath) are still zero.
-// Falls back to the original value if inspect fails.
-func refreshVM(ctx context.Context, hyper hypervisor.Hypervisor, vm *types.VM) *types.VM {
-	if info, err := hyper.Inspect(ctx, vm.ID); err == nil {
-		return info
-	}
-	return vm
-}
-
 func printPostCloneHints(vm *types.VM) {
 	if vm.Config.Windows {
 		fmt.Println()