Review round: restart the Windows service on an error exit, deps, cuts, docs

README.md

@@ -7,7 +7,7 @@ In-VM exec agent for [Cocoon](https://github.com/cocoonstack/cocoon)-managed VMs
 ## Highlights
 
 - Linux and Windows guest exec with stdin, stdout, stderr, and exit-code forwarding.
-- Host access through `cocoon vm exec`; direct AF_VSOCK client for smoke tests.
+- Host access through `cocoon vm exec`.
 - Linux clone/restore reseed with optional machine-ID regeneration.
 
 PTY mode is planned; see
@@ -16,7 +16,7 @@ PTY mode is planned; see
 ## Quick start
 
 ```bash
-cocoon-agent client --cid 3 --port 1024 -- echo "hello from guest"
+cocoon vm exec <vm> -- echo "hello from guest"
 ```
 
 Full steps in [Usage](docs/usage.md).

agent/exec.go

@@ -101,10 +101,9 @@ func runExec(parentCtx context.Context, argv []string, env map[string]string, st
 	}
 
 	exitCode := 0
-	var exitErr *exec.ExitError
-	switch {
+	switch exitErr, isExit := errors.AsType[*exec.ExitError](waitErr); {
 	case waitErr == nil:
-	case errors.As(waitErr, &exitErr):
+	case isExit:
 		exitCode = exitErr.ExitCode()
 	case errors.Is(waitErr, exec.ErrWaitDelay):
 		// Pipes were abandoned to a background child; the command itself exited.

agent/sysproc_windows.go

@@ -14,31 +14,6 @@ import (
 
 var ntResumeProcess = windows.NewLazySystemDLL("ntdll.dll").NewProc("NtResumeProcess")
 
-// windowsProcessController owns the Job Object of one runExec session; cmd.Cancel and runExec's deferred close both release it.
-type windowsProcessController struct {
-	job   windows.Handle
-	close func()
-}
-
-func (c *windowsProcessController) assign(cmd *exec.Cmd) error {
-	pid := uint32(cmd.Process.Pid) //nolint:gosec // the OS hands out PIDs as DWORDs; the int round-trip can't overflow
-	proc, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE|windows.PROCESS_SUSPEND_RESUME, false, pid)
-	if err != nil {
-		return fmt.Errorf("open process: %w", err)
-	}
-	defer windows.CloseHandle(proc) //nolint:errcheck
-
-	if err := windows.AssignProcessToJobObject(c.job, proc); err != nil {
-		return fmt.Errorf("assign process to job object: %w", err)
-	}
-	return resumeProcess(proc)
-}
-
-func (c *windowsProcessController) cancel() error {
-	c.close()
-	return nil
-}
-
 // setupProcess creates a kill-on-close Job Object so the child's whole process tree dies with the session.
 func setupProcess(cmd *exec.Cmd) (processController, error) {
 	job, err := windows.CreateJobObject(nil, nil)
@@ -58,16 +33,30 @@ func setupProcess(cmd *exec.Cmd) (processController, error) {
 		return processController{}, fmt.Errorf("set job object limits: %w", err)
 	}
 
-	ctl := &windowsProcessController{job: job, close: sync.OnceFunc(func() { _ = windows.CloseHandle(job) })}
+	closeJob := sync.OnceFunc(func() { _ = windows.CloseHandle(job) })
 	cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_SUSPENDED}
-	cmd.Cancel = ctl.cancel
+	cmd.Cancel = func() error { closeJob(); return nil }
 
 	return processController{
-		afterStart: ctl.assign,
-		close:      ctl.close,
+		afterStart: func(cmd *exec.Cmd) error { return assignToJob(job, cmd) },
+		close:      closeJob,
 	}, nil
 }
 
+func assignToJob(job windows.Handle, cmd *exec.Cmd) error {
+	pid := uint32(cmd.Process.Pid) //nolint:gosec // the OS hands out PIDs as DWORDs; the int round-trip can't overflow
+	proc, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE|windows.PROCESS_SUSPEND_RESUME, false, pid)
+	if err != nil {
+		return fmt.Errorf("open process: %w", err)
+	}
+	defer windows.CloseHandle(proc) //nolint:errcheck
+
+	if err := windows.AssignProcessToJobObject(job, proc); err != nil {
+		return fmt.Errorf("assign process to job object: %w", err)
+	}
+	return resumeProcess(proc)
+}
+
 func resumeProcess(proc windows.Handle) error {
 	if err := ntResumeProcess.Find(); err != nil {
 		return err

cmd/root.go

@@ -3,7 +3,6 @@ package cmd
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"os"
 	"os/signal"
@@ -21,12 +20,11 @@ func NewRootCmd() *cobra.Command {
 		Use:     "cocoon-agent",
 		Short:   "vsock-based command exec agent for Cocoon-managed VMs",
 		Version: fmt.Sprintf("%s (rev=%s built=%s)", version.VERSION, version.REVISION, version.BUILTAT),
-		// run logs and maps exit codes itself, so cobra's Error/Usage dump would double up
+		// run logs the error itself, so cobra's Error/Usage dump would double up
 		SilenceErrors: true,
 		SilenceUsage:  true,
 	}
 	rootCmd.AddCommand(newServeCmd())
-	rootCmd.AddCommand(newClientCmd())
 	return rootCmd
 }
 
@@ -53,9 +51,6 @@ func run() int {
 	}
 
 	if err := NewRootCmd().ExecuteContext(ctx); err != nil {
-		if ec, ok := errors.AsType[*exitCodeError](err); ok {
-			return ec.code
-		}
 		log.WithFunc("cmd.run").Error(ctx, err, "command failed")
 		return 1
 	}

cmd/serve.go

@@ -30,7 +30,7 @@ func newServeCmd() *cobra.Command {
 				return fmt.Errorf("listen vsock port %d: %w", port, err)
 			}
 			srv := agent.NewServer(listener)
-			log.WithFunc("cmd.newServeCmd").Infof(ctx, "cocoon-agent serving vsock port %d", port)
+			log.WithFunc("cmd.serve").Infof(ctx, "cocoon-agent serving vsock port %d", port)
 			return srv.Serve(ctx)
 		},
 	}

cmd/transport_linux.go

@@ -5,33 +5,12 @@ package cmd
 import (
 	"context"
 	"fmt"
-	"io"
 	"net"
 
 	"github.com/mdlayher/vsock"
 	"github.com/projecteru2/core/log"
 )
 
-func listenVsock(ctx context.Context, port uint32) (net.Listener, error) {
-	l, err := vsock.Listen(port, nil)
-	if err != nil {
-		return nil, fmt.Errorf("vsock listen: %w", err)
-	}
-	return &hostOnlyListener{
-		Listener: l,
-		ctx:      ctx,
-		logger:   log.WithFunc("cmd.hostOnlyListener.Accept"),
-	}, nil
-}
-
-func dialVsock(cid, port uint32) (io.ReadWriteCloser, error) {
-	conn, err := vsock.Dial(cid, port, nil)
-	if err != nil {
-		return nil, fmt.Errorf("vsock dial: %w", err)
-	}
-	return conn, nil
-}
-
 var _ net.Listener = (*hostOnlyListener)(nil)
 
 // Peers other than VMADDR_CID_HOST are rejected: VMADDR_CID_LOCAL loopback would let an unprivileged guest process run commands as root.
@@ -55,6 +34,18 @@ func (l *hostOnlyListener) Accept() (net.Conn, error) {
 	}
 }
 
+func listenVsock(ctx context.Context, port uint32) (net.Listener, error) {
+	l, err := vsock.Listen(port, nil)
+	if err != nil {
+		return nil, fmt.Errorf("vsock listen: %w", err)
+	}
+	return &hostOnlyListener{
+		Listener: l,
+		ctx:      ctx,
+		logger:   log.WithFunc("cmd.hostOnlyListener.Accept"),
+	}, nil
+}
+
 func isHostPeer(conn net.Conn) bool {
 	addr, ok := conn.RemoteAddr().(*vsock.Addr)
 	if !ok {

cmd/transport_linux_test.go

@@ -3,10 +3,8 @@
 package cmd
 
 import (
-	"io"
 	"net"
 	"testing"
-	"time"
 
 	"github.com/mdlayher/vsock"
 )
@@ -45,14 +43,8 @@ func TestIsHostPeer(t *testing.T) {
 }
 
 type staticAddrConn struct {
+	net.Conn
 	addr net.Addr
 }
 
-func (c *staticAddrConn) Read(_ []byte) (int, error)         { return 0, io.EOF }
-func (c *staticAddrConn) Write(p []byte) (int, error)        { return len(p), nil }
-func (c *staticAddrConn) Close() error                       { return nil }
-func (c *staticAddrConn) LocalAddr() net.Addr                { return c.addr }
-func (c *staticAddrConn) RemoteAddr() net.Addr               { return c.addr }
-func (c *staticAddrConn) SetDeadline(_ time.Time) error      { return nil }
-func (c *staticAddrConn) SetReadDeadline(_ time.Time) error  { return nil }
-func (c *staticAddrConn) SetWriteDeadline(_ time.Time) error { return nil }
+func (c *staticAddrConn) RemoteAddr() net.Addr { return c.addr }

cmd/transport_other.go

@@ -5,7 +5,6 @@ package cmd
 import (
 	"context"
 	"errors"
-	"io"
 	"net"
 )
 
@@ -15,7 +14,3 @@ var errVsockUnsupported = errors.New("vsock is only supported on linux and windo
 func listenVsock(_ context.Context, _ uint32) (net.Listener, error) {
 	return nil, errVsockUnsupported
 }
-
-func dialVsock(_, _ uint32) (io.ReadWriteCloser, error) {
-	return nil, errVsockUnsupported
-}

cmd/transport_windows.go

@@ -37,7 +37,6 @@ var (
 	procBind       = modws2_32.NewProc("bind")
 	procListen     = modws2_32.NewProc("listen")
 	procAccept     = modws2_32.NewProc("accept")
-	procConnect    = modws2_32.NewProc("connect")
 	procRecv       = modws2_32.NewProc("recv")
 	procSend       = modws2_32.NewProc("send")
 	procWSAStartup = modws2_32.NewProc("WSAStartup")
@@ -192,21 +191,13 @@ type vsockAddr struct {
 func (a *vsockAddr) Network() string { return "vsock" }
 func (a *vsockAddr) String() string  { return fmt.Sprintf("vsock://%d:%d", a.cid, a.port) }
 
-func newVsockSocket() (windows.Handle, error) {
+func listenVsock(ctx context.Context, port uint32) (net.Listener, error) {
 	if err := wsaInit(); err != nil {
-		return 0, err
+		return nil, err
 	}
 	h, err := windows.Socket(afVsock, windows.SOCK_STREAM, 0)
 	if err != nil {
-		return 0, fmt.Errorf("vsock socket: %w", err)
-	}
-	return h, nil
-}
-
-func listenVsock(ctx context.Context, port uint32) (net.Listener, error) {
-	h, err := newVsockSocket()
-	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("vsock socket: %w", err)
 	}
 	sa := sockaddrVM{Family: afVsock, Port: port, CID: vmAddrCidAny}
 	r, _, callErr := procBind.Call(uintptr(h), uintptr(unsafe.Pointer(&sa)), uintptr(sockaddrVMSize)) //nolint:gosec // winsock bind requires raw pointer
@@ -226,21 +217,3 @@ func listenVsock(ctx context.Context, port uint32) (net.Listener, error) {
 		logger: log.WithFunc("cmd.vsockListener.Accept"),
 	}, nil
 }
-
-func dialVsock(cid, port uint32) (io.ReadWriteCloser, error) {
-	h, err := newVsockSocket()
-	if err != nil {
-		return nil, err
-	}
-	sa := sockaddrVM{Family: afVsock, Port: port, CID: cid}
-	r, _, callErr := procConnect.Call(uintptr(h), uintptr(unsafe.Pointer(&sa)), uintptr(sockaddrVMSize)) //nolint:gosec // winsock connect requires raw pointer
-	if r == socketError {
-		_ = windows.Closesocket(h)
-		return nil, fmt.Errorf("vsock connect %d:%d: %w", cid, port, callErr)
-	}
-	return &vsockConn{
-		h:        h,
-		peerCID:  cid,
-		peerPort: port,
-	}, nil
-}

docs/architecture.md

@@ -9,7 +9,7 @@ Cocoon's host components (vk-cocoon, cocoon CLI) and the VMs they manage always
 - requires per-VM credentials to be provisioned and rotated
 - comes with the full SSH protocol surface (handshake, key exchange, host key trust)
 
-vsock is host↔guest only, has no IP layer, and the kernel `vhost_vsock` module is the auth boundary — anything that can connect from the host is already privileged on that host. cocoon-agent leverages this for a small focused daemon that gives kubectl-exec semantics (stdin / stdout / stderr / exit code) with none of those dependencies.
+vsock is host↔guest only, has no IP layer, and access to the hypervisor's host-side vsock socket (for cocoon, the per-VM hybrid-vsock Unix socket) is the auth boundary — anything that can connect from the host is already privileged on that host. cocoon-agent leverages this for a small focused daemon that gives kubectl-exec semantics (stdin / stdout / stderr / exit code) with none of those dependencies.
 
 ## Status
 

docs/configuration.md

@@ -9,5 +9,3 @@ CLI flags:
 | Flag | Default | Notes |
 |---|---|---|
 | `serve --port` | `1024` | vsock port to bind on |
-| `client --cid` | required | target VM CID (host-side) |
-| `client --port` | `1024` | matches `serve --port` |

docs/index.md

@@ -26,8 +26,8 @@ host (cocoon node)                                  guest VM
   status, the wire protocol, and the roadmap
 - [Installation](install.md) — baking the Linux binary into an image,
   the Windows service installer, and pipe-mode limitations
-- [Usage](usage.md) — smoke-testing an agent from the host with the
-  `client` subcommand
+- [Usage](usage.md) — smoke-testing an agent from the host with
+  `cocoon vm exec`
 - [Configuration](configuration.md) — environment variables and CLI flags
 - [Build](build.md) — cross-compiling for Linux and Windows guests
 

docs/install.md

@@ -37,7 +37,7 @@ Windows support requires the `viosock` driver shipped with **virtio-win >= 0.1.2
 .\install-cocoon-agent.ps1
 ```
 
-The script copies `cocoon-agent.exe` to `C:\Program Files\Cocoon\`, registers a Windows service (`cocoon-agent`, `LocalSystem`, auto-start, restart-on-crash), and starts it. See [`packaging/install-cocoon-agent.ps1`](../packaging/install-cocoon-agent.ps1).
+The script copies `cocoon-agent.exe` to `C:\Program Files\Cocoon\`, registers a Windows service (`cocoon-agent`, `LocalSystem`, auto-start, restart on failure), and starts it. See [`packaging/install-cocoon-agent.ps1`](../packaging/install-cocoon-agent.ps1).
 
 Verify it's running:
 

docs/usage.md

@@ -2,11 +2,11 @@
 
 ## Smoke test from the host
 
-cocoon-agent ships a `client` subcommand for vsock smoke tests without needing to plumb through cocoon CLI / vk-cocoon. It dials AF_VSOCK by CID, so take the guest CID from the hypervisor's vsock config (cocoon itself reaches the agent over the hybrid-vsock Unix socket that `cocoon vm inspect` reports as `vsock_socket`, not by CID) and run:
+The host reaches the agent through cocoon: `cocoon vm exec` dials the VM's hybrid-vsock Unix socket and speaks the agent protocol.
 
 ```bash
-cocoon-agent client --cid 3 --port 1024 -- echo "hello from guest"
-echo "world" | cocoon-agent client --cid 3 --port 1024 -- cat
+cocoon vm exec <vm> -- echo "hello from guest"
+echo "world" | cocoon vm exec -i <vm> -- cat
 ```
 
-Exit codes pass through.
+Stdin is attached only with `-i`; exit codes pass through.