Return client errors and restore failed display changes

server/cmd/api/api/chromium_configure.go

@@ -840,6 +840,9 @@ func chromiumDisplayApplyWhileStopped(ctx context.Context, s *ApiService, plan *
 	}
 	_, _, err := s.applyResolutionAndConverge(ctx, w, h, rr, s.isNekoEnabled())
 	if err != nil {
+		if isNekoScreenRejection(err) {
+			return cfg400(err.Error())
+		}
 		return cfg500ConfigureStep(chromiumConfigureStepDisplay, err.Error())
 	}
 	return nil

server/cmd/api/api/display.go

@@ -3,8 +3,10 @@ package api
 import (
 	"context"
 	"encoding/base64"
+	"errors"
 	"fmt"
 	"log/slog"
+	"math"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -14,6 +16,7 @@ import (
 
 	"github.com/kernel/kernel-images/server/lib/cdpclient"
 	"github.com/kernel/kernel-images/server/lib/logger"
+	"github.com/kernel/kernel-images/server/lib/nekoclient"
 	oapi "github.com/kernel/kernel-images/server/lib/oapi"
 	"github.com/kernel/kernel-images/server/lib/recorder"
 	nekooapi "github.com/m1k1o/neko/server/lib/oapi"
@@ -130,6 +133,7 @@ func (s *ApiService) patchDisplayLocked(ctx context.Context, req oapi.PatchDispl
 					"realized", fmt.Sprintf("%dx%d", realizedW, realizedH))
 			}
 			width, height = realizedW, realizedH
+			_, _, refreshRate, _, err = s.getCurrentResolutionFromXrandr(ctx)
 		}
 		// Re-assert the maximized window state via CDP so mutter reflows the
 		// window onto the new root; applyResolutionAndConverge already verified
@@ -183,6 +187,9 @@ func (s *ApiService) patchDisplayLocked(ctx context.Context, req oapi.PatchDispl
 
 	if err != nil {
 		log.Error("failed to change resolution", "error", err)
+		if isNekoScreenRejection(err) {
+			return oapi.PatchDisplay400JSONResponse{BadRequestErrorJSONResponse: oapi.BadRequestErrorJSONResponse{Message: err.Error()}}, nil
+		}
 		return oapi.PatchDisplay500JSONResponse{
 			InternalErrorJSONResponse: oapi.InternalErrorJSONResponse{
 				Message: fmt.Sprintf("failed to change resolution: %s", err.Error()),
@@ -553,6 +560,27 @@ func abs(x int) int {
 // relaunching Chromium in --kiosk — must go through here rather than call
 // setResolutionViaNeko directly.
 func (s *ApiService) applyResolutionAndConverge(ctx context.Context, width, height, refreshRate int, useNeko bool) (int, int, error) {
+	previousW, previousH, previousRate, _, err := s.getCurrentResolutionFromXrandr(ctx)
+	if err != nil {
+		return 0, 0, err
+	}
+	realizedW, realizedH, err := s.setResolutionAndConverge(ctx, width, height, refreshRate, useNeko)
+	if err == nil {
+		return realizedW, realizedH, nil
+	}
+
+	// Mode creation can move the X root even when the screen change fails.
+	// Restoration needs the same retries as resizing: Neko can drop a reconfig.
+	restoreCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 35*time.Second)
+	defer cancel()
+	if _, _, restoreErr := s.setResolutionAndConverge(restoreCtx, previousW, previousH, previousRate, useNeko); restoreErr != nil {
+		logger.FromContext(ctx).Error("failed to restore display after resize failure", "error", restoreErr)
+		err = fmt.Errorf("%w; failed to restore display: %s", err, restoreErr)
+	}
+	return realizedW, realizedH, err
+}
+
+func (s *ApiService) setResolutionAndConverge(ctx context.Context, width, height, refreshRate int, useNeko bool) (int, int, error) {
 	log := logger.FromContext(ctx)
 	var err error
 	if useNeko {
@@ -593,6 +621,11 @@ func (s *ApiService) applyResolutionAndConverge(ctx context.Context, width, heig
 	return realizedW, realizedH, nil
 }
 
+func isNekoScreenRejection(err error) bool {
+	var screenErr *nekoclient.ScreenConfigurationError
+	return errors.As(err, &screenErr) && screenErr.StatusCode >= 400 && screenErr.StatusCode < 500
+}
+
 // setViewportViaCDP resizes the browser viewport using the CDP
 // Emulation.setDeviceMetricsOverride command. This is near-instant and does
 // not require restarting Chromium or Xvfb.
@@ -719,16 +752,15 @@ func (s *ApiService) getCurrentResolution(ctx context.Context) (int, int, int, e
 
 // getCurrentResolutionFromXrandr queries xrandr for the current display
 // resolution. The fourth return value reports whether the refresh rate was
-// parsed from xrandr output (true) or is the synthesized fallback (false);
+// parsed from xrandr output (true) or is unavailable (false);
 // callers can use it to prefer a previously-recorded rate when xrandr is
 // silent, as Xvfb is.
 func (s *ApiService) getCurrentResolutionFromXrandr(ctx context.Context) (int, int, int, bool, error) {
 	log := logger.FromContext(ctx)
 	display := s.resolveDisplayFromEnv()
 
 	// Use xrandr to get current resolution
-	// Note: Using bash -c (not -lc) to avoid login shell overriding DISPLAY env var
-	cmd := exec.CommandContext(ctx, "bash", "-c", "xrandr | grep -E '\\*' | awk '{print $1}'")
+	cmd := exec.CommandContext(ctx, "xrandr", "--current")
 	cmd.Env = append(os.Environ(), fmt.Sprintf("DISPLAY=%s", display))
 
 	out, err := cmd.Output()
@@ -737,40 +769,35 @@ func (s *ApiService) getCurrentResolutionFromXrandr(ctx context.Context) (int, i
 		return 0, 0, 0, false, fmt.Errorf("failed to execute xrandr command: %w", err)
 	}
 
-	resStr := strings.TrimSpace(string(out))
-	parts := strings.Split(resStr, "x")
-	if len(parts) != 2 {
-		log.Error("unexpected xrandr output format", "output", resStr)
-		return 0, 0, 0, false, fmt.Errorf("unexpected xrandr output format: %s", resStr)
-	}
+	return parseCurrentResolutionFromXrandr(string(out))
+}
 
-	width, err := strconv.Atoi(parts[0])
-	if err != nil {
-		log.Error("failed to parse width", "error", err, "value", parts[0])
-		return 0, 0, 0, false, fmt.Errorf("failed to parse width '%s': %w", parts[0], err)
-	}
-
-	// Parse height and refresh rate (e.g., "1080_60.00" -> height=1080, rate=60)
-	heightStr := parts[1]
-	refreshRate := 60 // default when xrandr omits the _rate suffix (e.g. Xvfb)
-	rateFromXrandr := false
-	if idx := strings.Index(heightStr, "_"); idx != -1 {
-		rateStr := heightStr[idx+1:]
-		heightStr = heightStr[:idx]
-		// Parse the refresh rate (e.g., "60.00" -> 60)
-		if rateFloat, err := strconv.ParseFloat(rateStr, 64); err == nil {
-			refreshRate = int(rateFloat)
-			rateFromXrandr = true
+func parseCurrentResolutionFromXrandr(out string) (int, int, int, bool, error) {
+	for _, line := range strings.Split(out, "\n") {
+		fields := strings.Fields(line)
+		if len(fields) < 2 {
+			continue
+		}
+		for _, field := range fields[1:] {
+			if !strings.Contains(field, "*") {
+				continue
+			}
+			rate, err := strconv.ParseFloat(strings.TrimRight(field, "*+"), 64)
+			if err != nil {
+				return 0, 0, 0, false, fmt.Errorf("invalid xrandr refresh rate %q: %w", field, err)
+			}
+			modeName := strings.SplitN(fields[0], "_", 2)[0]
+			var width, height int
+			if _, err := fmt.Sscanf(modeName, "%dx%d", &width, &height); err != nil {
+				return 0, 0, 0, false, fmt.Errorf("invalid xrandr mode %q: %w", fields[0], err)
+			}
+			if rate == 0 {
+				return width, height, 60, false, nil
+			}
+			return width, height, int(math.Round(rate)), true, nil
 		}
 	}
-
-	height, err := strconv.Atoi(heightStr)
-	if err != nil {
-		log.Error("failed to parse height", "error", err, "value", heightStr)
-		return 0, 0, 0, false, fmt.Errorf("failed to parse height '%s': %w", heightStr, err)
-	}
-
-	return width, height, refreshRate, rateFromXrandr, nil
+	return 0, 0, 0, false, fmt.Errorf("no current mode in xrandr output")
 }
 
 // stoppedRecordingInfo holds state captured from a recording that was stopped

server/cmd/api/api/display_resize_test.go

@@ -0,0 +1,132 @@
+package api
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/kernel/kernel-images/server/lib/nekoclient"
+	oapi "github.com/kernel/kernel-images/server/lib/oapi"
+	"github.com/kernel/kernel-images/server/lib/recorder"
+	nekooapi "github.com/m1k1o/neko/server/lib/oapi"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestParseCurrentResolutionFromXrandr(t *testing.T) {
+	for _, tc := range []struct {
+		name                string
+		out                 string
+		width, height, rate int
+		hasRate             bool
+	}{
+		{"rounded CVT rate", "   376x480_60  59.21*+\n", 376, 480, 59, true},
+		{"nearest integer", "   1280x800_60.00 59.81*\n", 1280, 800, 60, true},
+		{"non-default rate", "   1920x1080_25.00 24.98*+ 60.00\n", 1920, 1080, 25, true},
+		{"Xvfb", "   1280x720 0.00*\n", 1280, 720, 60, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			w, h, rate, hasRate, err := parseCurrentResolutionFromXrandr(tc.out)
+			require.NoError(t, err)
+			assert.Equal(t, tc.width, w)
+			assert.Equal(t, tc.height, h)
+			assert.Equal(t, tc.rate, rate)
+			assert.Equal(t, tc.hasRate, hasRate)
+		})
+	}
+	_, _, _, _, err := parseCurrentResolutionFromXrandr("DUMMY0 connected\n")
+	require.Error(t, err)
+}
+
+func TestRejectedResizeRestoresDisplay(t *testing.T) {
+	for _, status := range []int{http.StatusUnprocessableEntity, http.StatusBadRequest, http.StatusInternalServerError} {
+		for _, path := range []string{"display", "configure-live", "configure-stopped"} {
+			t.Run(fmt.Sprintf("%s/%d", path, status), func(t *testing.T) {
+				dir := t.TempDir()
+				state := filepath.Join(dir, "screen")
+				previous := "   1280x800_25.00 25.00*+\n"
+				require.NoError(t, os.WriteFile(state, []byte(previous), 0600))
+				require.NoError(t, os.WriteFile(filepath.Join(dir, "xrandr"), []byte("#!/bin/sh\ncat \"$TEST_XRANDR_STATE\"\n"), 0700))
+				t.Setenv("TEST_XRANDR_STATE", state)
+				t.Setenv("PATH", dir+":"+os.Getenv("PATH"))
+				t.Setenv("ENABLE_WEBRTC", "true")
+
+				var requests []nekooapi.ScreenConfiguration
+				server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+					w.Header().Set("Content-Type", "application/json")
+					if r.URL.Path == "/api/login" {
+						_, _ = w.Write([]byte(`{"token":"test-token"}`))
+						return
+					}
+					var config nekooapi.ScreenConfiguration
+					if err := json.NewDecoder(r.Body).Decode(&config); err != nil {
+						w.WriteHeader(http.StatusBadRequest)
+						return
+					}
+					requests = append(requests, config)
+					if len(requests) == 1 {
+						_ = os.WriteFile(state, []byte("   3840x2160_25.00 25.00*+\n"), 0600)
+						w.WriteHeader(status)
+						_, _ = w.Write([]byte(`{"message":"cannot set screen size"}`))
+						return
+					}
+					if path == "configure-live" && len(requests) == 2 {
+						// Neko can accept a reconfig without applying it.
+						w.WriteHeader(http.StatusNoContent)
+						return
+					}
+					_ = os.WriteFile(state, []byte(previous), 0600)
+					w.WriteHeader(http.StatusNoContent)
+				}))
+				defer server.Close()
+				client, err := nekoclient.NewAuthClient(server.URL, "admin", "test")
+				require.NoError(t, err)
+				svc := &ApiService{nekoAuthClient: client, recordManager: recorder.NewFFmpegManager()}
+				svc.displayModeOnce.Do(func() { svc.displayModeVal = "xorg" })
+				width, height, rate, idle := 379, 480, 60, false
+				requestRate := oapi.PatchDisplayRequestRefreshRate(rate)
+				body := &oapi.PatchDisplayJSONRequestBody{Width: &width, Height: &height, RefreshRate: &requestRate, RequireIdle: &idle}
+				ctx := context.Background()
+				if path == "display" {
+					resp, err := svc.PatchDisplay(ctx, oapi.PatchDisplayRequestObject{Body: body})
+					require.NoError(t, err)
+					if status < 500 {
+						require.IsType(t, oapi.PatchDisplay400JSONResponse{}, resp)
+					} else {
+						require.IsType(t, oapi.PatchDisplay500JSONResponse{}, resp)
+					}
+				} else {
+					var resp oapi.ChromiumConfigureResponseObject
+					if path == "configure-live" {
+						resp = chromiumRunPatchDisplay(ctx, svc, body)
+					} else {
+						resp = chromiumDisplayApplyWhileStopped(ctx, svc, &chromiumDisplayPlan{width: width, height: height, refreshRate: rate})
+					}
+					if status < 500 {
+						require.IsType(t, oapi.ChromiumConfigure400JSONResponse{}, resp)
+					} else {
+						require.IsType(t, oapi.ChromiumConfigure500JSONResponse{}, resp)
+					}
+				}
+				if path == "configure-live" {
+					require.Len(t, requests, 3)
+				} else {
+					require.Len(t, requests, 2)
+				}
+				for _, restore := range requests[1:] {
+					assert.Equal(t, 1280, *restore.Width)
+					assert.Equal(t, 800, *restore.Height)
+					assert.Equal(t, 25, *restore.Rate)
+				}
+				got, err := os.ReadFile(state)
+				require.NoError(t, err)
+				assert.Equal(t, previous, string(got))
+			})
+		}
+	}
+}

server/lib/nekoclient/client.go

@@ -117,6 +117,16 @@ func (c *AuthClient) SessionsGet(ctx context.Context) ([]nekooapi.SessionData, e
 	return *resp.JSON200, nil
 }
 
+// ScreenConfigurationError preserves the HTTP status of a rejected screen change.
+type ScreenConfigurationError struct {
+	StatusCode int
+	Body       string
+}
+
+func (e *ScreenConfigurationError) Error() string {
+	return fmt.Sprintf("screen configuration API returned status %d: %s", e.StatusCode, e.Body)
+}
+
 // ScreenConfigurationChange changes the screen resolution via Neko API.
 // The HTTP response body echoes the request, not the realized
 // configuration (neko's screenConfigurationChange handler returns `data`,
@@ -158,7 +168,7 @@ func (c *AuthClient) ScreenConfigurationChange(ctx context.Context, config nekoo
 	}
 
 	if resp.StatusCode() != http.StatusOK && resp.StatusCode() != http.StatusNoContent {
-		return fmt.Errorf("screen configuration API returned status %d: %s", resp.StatusCode(), string(resp.Body))
+		return &ScreenConfigurationError{StatusCode: resp.StatusCode(), Body: string(resp.Body)}
 	}
 
 	return nil