macOS guests 3/6: Darwin guest service, readiness and exec

cmd/api/api/cp.go

@@ -104,6 +104,11 @@ func (s *ApiService) CpHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpFileCopy); reason != "" {
+		writeGuestUnsupported(w, reason)
+		return
+	}
+
 	if inst.State != instances.StateRunning {
 		http.Error(w, fmt.Sprintf(`{"code":"invalid_state","message":"instance must be running (current state: %s)"}`, inst.State), http.StatusConflict)
 		return

cmd/api/api/exec.go

@@ -71,8 +71,8 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if inst.MacOS != nil {
-		http.Error(w, `{"code":"unsupported","message":"exec is not implemented for experimental macOS instances"}`, http.StatusNotImplemented)
+	if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpExec); reason != "" {
+		writeGuestUnsupported(w, reason)
 		return
 	}
 
@@ -151,6 +151,14 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
 
 	// Create WebSocket read/writer wrapper that handles resize messages
 	wsConn := &wsReadWriter{ws: ws, ctx: ctx, resizeChan: resizeChan}
+	if inst.MacOS != nil {
+		// The macOS guest agent cancels a command when its stream ends, so a lost
+		// websocket ends the session instead of leaving the command running.
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithCancel(ctx)
+		defer cancel()
+		wsConn.ctx, wsConn.cancel = ctx, cancel
+	}
 
 	dialer, err := s.InstanceManager.GetVsockDialer(ctx, inst.Id)
 	if err != nil {
@@ -221,6 +229,7 @@ type wsReadWriter struct {
 	reader     io.Reader
 	mu         sync.Mutex
 	resizeChan chan<- *guest.WindowSize // Channel to send resize events (nil if not TTY)
+	cancel     context.CancelFunc       // ends a macOS exec session when the websocket fails
 }
 
 func (w *wsReadWriter) Read(p []byte) (n int, err error) {
@@ -241,6 +250,7 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) {
 		// Read next WebSocket message
 		messageType, data, err := w.ws.ReadMessage()
 		if err != nil {
+			w.cancelSession()
 			return 0, err
 		}
 
@@ -273,7 +283,14 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) {
 
 func (w *wsReadWriter) Write(p []byte) (n int, err error) {
 	if err := w.ws.WriteMessage(websocket.BinaryMessage, p); err != nil {
+		w.cancelSession()
 		return 0, err
 	}
 	return len(p), nil
 }
+
+func (w *wsReadWriter) cancelSession() {
+	if w.cancel != nil {
+		w.cancel()
+	}
+}

cmd/api/api/exec_disconnect_test.go

@@ -0,0 +1,45 @@
+package api
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/gorilla/websocket"
+	"github.com/stretchr/testify/require"
+)
+
+func TestExecWebsocketDisconnectCancelsSession(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+	readDone := make(chan error, 1)
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		conn, err := upgrader.Upgrade(w, r, nil)
+		if err != nil {
+			readDone <- err
+			return
+		}
+		defer conn.Close()
+		wrapped := &wsReadWriter{ws: conn, ctx: ctx, cancel: cancel}
+		_, err = wrapped.Read(make([]byte, 1))
+		readDone <- err
+	}))
+	defer server.Close()
+	conn, _, err := websocket.DefaultDialer.Dial("ws"+strings.TrimPrefix(server.URL, "http"), nil)
+	require.NoError(t, err)
+	require.NoError(t, conn.Close())
+	select {
+	case <-ctx.Done():
+	case <-time.After(5 * time.Second):
+		t.Fatal("WebSocket disconnect did not cancel exec session")
+	}
+	select {
+	case err := <-readDone:
+		require.Error(t, err)
+	case <-time.After(5 * time.Second):
+		t.Fatal("WebSocket reader did not exit")
+	}
+}

cmd/api/api/guest_unsupported.go

@@ -0,0 +1,13 @@
+package api
+
+import (
+	"encoding/json"
+	"net/http"
+)
+
+// writeGuestUnsupported writes the shared 501 "unsupported" body for raw HTTP handlers.
+func writeGuestUnsupported(w http.ResponseWriter, reason string) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusNotImplemented)
+	_ = json.NewEncoder(w).Encode(map[string]string{"code": "unsupported", "message": reason})
+}

cmd/api/api/instances.go

@@ -988,11 +988,8 @@ func (s *ApiService) StatInstancePath(ctx context.Context, request oapi.StatInst
 		}, nil
 	}
 
-	if inst.MacOS != nil {
-		return oapi.StatInstancePath501JSONResponse{
-			Code:    "unsupported",
-			Message: "stat is not implemented for experimental macOS instances",
-		}, nil
+	if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpStat); reason != "" {
+		return oapi.StatInstancePath501JSONResponse{Code: "unsupported", Message: reason}, nil
 	}
 
 	if inst.State != instances.StateRunning {

cmd/api/api/macos_test.go

@@ -36,20 +36,50 @@ func TestMacOSExecRejectedBeforeWebsocketUpgrade(t *testing.T) {
 	w := httptest.NewRecorder()
 	s.ExecHandler(w, r)
 	require.Equal(t, http.StatusNotImplemented, w.Code)
-	require.Contains(t, w.Body.String(), "not implemented")
+	require.Contains(t, w.Body.String(), "requires the shared guest agent")
 }
 
-func TestMacOSStatRejectedBeforeGuestDial(t *testing.T) {
+func TestMacOSStatRequiresGuestAgentBeforeDial(t *testing.T) {
 	s := &ApiService{}
 	inst := &instances.Instance{StoredMetadata: instances.StoredMetadata{MacOS: &images.MacOSImage{}}, State: instances.StateRunning}
 	ctx := mw.WithResolvedInstance(context.Background(), "test", inst)
 	resp, err := s.StatInstancePath(ctx, oapi.StatInstancePathRequestObject{Id: "test"})
 	require.NoError(t, err)
 	unsupported, ok := resp.(oapi.StatInstancePath501JSONResponse)
-	require.True(t, ok, "macOS stat must be rejected with 501, got %T", resp)
+	require.True(t, ok, "macOS stat without the agent must be rejected with 501, got %T", resp)
 	require.Equal(t, "unsupported", unsupported.Code)
 }
 
+func TestMacOSGuestAgentAdmissionBeforeUpgrade(t *testing.T) {
+	for _, tc := range []struct {
+		name              string
+		declared, skipped bool
+		status            int
+	}{
+		{"unmanaged", false, false, http.StatusNotImplemented},
+		{"disabled", true, true, http.StatusNotImplemented},
+		{"enabled but stopped", true, false, http.StatusConflict},
+	} {
+		for _, operation := range []string{"exec", "cp"} {
+			t.Run(tc.name+"/"+operation, func(t *testing.T) {
+				s := &ApiService{}
+				inst := &instances.Instance{StoredMetadata: instances.StoredMetadata{
+					MacOS: &images.MacOSImage{GuestAgent: tc.declared}, SkipGuestAgent: tc.skipped,
+				}, State: instances.StateStopped}
+				ctx := mw.WithResolvedInstance(context.Background(), "test", inst)
+				r := httptest.NewRequest(http.MethodGet, "/instances/test/"+operation, nil).WithContext(ctx)
+				w := httptest.NewRecorder()
+				if operation == "exec" {
+					s.ExecHandler(w, r)
+				} else {
+					s.CpHandler(w, r)
+				}
+				require.Equal(t, tc.status, w.Code)
+			})
+		}
+	}
+}
+
 func TestLinuxCreateDefaultsMatchSchema(t *testing.T) {
 	spec, err := oapi.GetSwagger()
 	require.NoError(t, err)

cmd/api/main.go

@@ -684,6 +684,14 @@ func run() error {
 			return restartController.StartRestartPolicyController(gctx)
 		})
 	}
+	if startupController, ok := app.InstanceManager.(interface {
+		StartMacOSAgentStartupController(context.Context) error
+	}); ok && runtime.GOOS == "darwin" {
+		grp.Go(func() error {
+			logger.Info("starting macOS agent startup controller")
+			return startupController.StartMacOSAgentStartupController(gctx)
+		})
+	}
 	if ttlReaper, ok := app.InstanceManager.(interface {
 		StartTTLReaper(context.Context) error
 	}); ok {

cmd/vz-shim/macos_arm64.go

@@ -49,7 +49,8 @@ func configureMacPlatform(vc *vz.VirtualMachineConfiguration, c *shimconfig.Shim
 	if e != nil {
 		return e
 	}
-	display, e := vz.NewMacGraphicsDisplayConfiguration(1280, 800, 80)
+	// 3024x1964 pixels at 2x: 1512x982 points, the logical size of a 14-inch MacBook Pro.
+	display, e := vz.NewMacGraphicsDisplayConfiguration(3024, 1964, 160)
 	if e != nil {
 		return e
 	}

docs/macos-experimental.md

@@ -73,13 +73,19 @@ Use the usual JWT-authenticated API with a VZ-capable, signed build:
 ```
 
 CPU and memory default to the imported template (minimum 2 CPUs and 4GB).
-Each instance gets its own writable complete boot disk and auxiliary storage;
-no Linux kernel, initrd, configuration disk, overlay filesystem or balloon is
+Each instance gets its own writable complete boot disk (`boot.raw`) and auxiliary
+storage; instances created before that naming must be deleted and recreated. No
+Linux kernel, initrd, configuration disk, overlay filesystem or balloon is
 attached. NAT uses the preserved MAC. The IP is observed from the host's VZ
 DHCP leases, which may retain an old lease before the guest is ready.
 
-**`Running` means the VMM is running, not that SSH, the desktop or Chrome is
-ready.** Readiness is currently tested over SSH. Startup does not launch Chrome.
+**Without the guest agent, `Running` means the VMM is running, not that SSH,
+the desktop or Chrome is ready.** Unmanaged templates still need an external
+readiness check. Templates provisioned with the shared Darwin GuestService can opt
+in with `guest_agent: true` in their platform configuration; those instances stay
+`Initializing` until the agent answers, as Linux guests do, so waiting for
+`Running` waits for the agent. See [Darwin GuestService](macos-guest-agent.md).
+Startup does not launch Chrome.
 A shim killed without cleanup (for example, SIGKILL) leaves its control socket
 behind; when nothing accepts on it and the recorded shim is gone, the instance
 reads as Stopped and its resources are released for admission. If the API dies
@@ -91,8 +97,9 @@ instance with a given Mac identifier is allowed. Do not run the source bundle
 or an external clone concurrently. Production provisioning needs identity and
 credential rekeying; this spike is not a multi-tenant image format.
 
-`POST /instances/{id}/stop` shuts down the VMM; without a Darwin guest agent,
-it does **not** guarantee an orderly guest OS/application shutdown. Use a human
+`POST /instances/{id}/stop` attempts orderly GuestService shutdown for opted-in
+images, then falls back to VMM shutdown if necessary. Without an enabled Darwin
+guest agent it does **not** guarantee an orderly guest OS/application shutdown. Use a human
 or an authorized guest shutdown workflow before destructive operations when
 application consistency matters. Start cold-boots the instance's existing disk.
 Delete removes instance storage, not the imported image. Deleting the imported