From c86f5141cf99822698311f283202e73d6f3763ec Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Thu, 4 Jun 2026 14:50:18 +0000 Subject: [PATCH 1/5] display: return realized X root size instead of failing on libxcvt rounding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PATCH /display previously enforced strict equality between the request and the X root size after a resize. libxcvt's CVT 8-pixel grid round (e.g. 1365 → 1360) and the hard-coded FWXGA bump for 1360×768 → 1366×768 make that equality impossible to satisfy for some requests, returning 500. Callers that treat /display 500s as fatal end up tainting the browser instance on every odd-width resize. Replace the strict post-condition with a single xrandr read of the X root after neko's synchronous resize returns. Use the realized dimensions for the response body so callers' coordinate math lines up with what the X server actually rendered. Log the request-vs-realized gap for diagnosability. The e2e reproducer is flipped from asserting the 500 to asserting the 200 + realized 1366 in the response body. Co-Authored-By: Claude Opus 4.7 --- server/cmd/api/api/display.go | 96 +++++++------------- server/e2e/e2e_display_resize_window_test.go | 83 +++++++++++++++++ server/lib/nekoclient/client.go | 4 + 3 files changed, 122 insertions(+), 61 deletions(-) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index 79b59829..d470da85 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -143,15 +143,31 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ // accepted for API compatibility but no longer triggers a // restart on this path. if err == nil { - if cdpErr := s.setWindowMaximizedViaCDP(ctx); cdpErr != nil { - log.Error("CDP maximize re-assert failed after Xorg resolution change", "error", cdpErr) - err = fmt.Errorf("CDP maximize re-assert failed: %w", cdpErr) + // Read the X root immediately after neko's synchronous + // resize, before any window-state CDP calls. This captures + // libxcvt's realized size — which can differ from the + // request (CVT 8-pixel grid round + FWXGA bump for + // 1360×768 → 1366×768) — without racing the chromium + // maximize that follows. Returning the realized size in + // the response body lets callers' coordinate math line up + // with what the X server actually rendered. + realizedW, realizedH, _, _, xrErr := s.getCurrentResolutionFromXrandr(ctx) + if xrErr != nil { + log.Error("failed to read X root after resize", "error", xrErr) + err = fmt.Errorf("X root read: %w", xrErr) + } else { + if realizedW != width || realizedH != height { + log.Info("X root differs from request after resize", + "requested", fmt.Sprintf("%dx%d", width, height), + "realized", fmt.Sprintf("%dx%d", realizedW, realizedH)) + } + width, height = realizedW, realizedH } } if err == nil { - if waitErr := s.waitForXRootSize(ctx, width, height, 3*time.Second); waitErr != nil { - log.Error("X root did not reach requested size after resize", "error", waitErr) - err = fmt.Errorf("X root verification: %w", waitErr) + if cdpErr := s.setWindowMaximizedViaCDP(ctx); cdpErr != nil { + log.Error("CDP maximize re-assert failed after Xorg resolution change", "error", cdpErr) + err = fmt.Errorf("CDP maximize re-assert failed: %w", cdpErr) } } } else if len(stopped) > 0 { @@ -431,12 +447,13 @@ func (s *ApiService) withCDPClient(ctx context.Context, fn func(context.Context, // in the state that triggers the reflow. The CDP call is idempotent: it // no-ops on a window already in maximized or fullscreen state. // -// The PATCH /display handler waits for the X root to reach the requested -// size separately (waitForXRootSize) rather than reading the chromium -// window's own bounds, so the contract stays panel-robust: if mutter -// ever gained a taskbar/dock, a maximized window would be smaller than -// the root by the panel's reserved space, and a window-bounds poll -// would 3s-timeout forever. +// The PATCH /display handler reads the X root separately (via +// getCurrentResolutionFromXrandr) to capture the realized size for the +// response, rather than reading the chromium window's own bounds. That +// keeps the contract panel-robust: if mutter ever gained a taskbar/dock, +// a maximized window would be smaller than the root by the panel's +// reserved space, and any window-bounds-based check would diverge from +// the X root. func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { log := logger.FromContext(ctx) if err := s.withCDPClient(ctx, func(cdpCtx context.Context, client *cdpclient.Client) error { @@ -448,54 +465,6 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { return nil } -// waitForXRootSize polls the X root resolution via xrandr until it matches -// the requested size, or the deadline expires. This is the authoritative -// post-condition for PATCH /display: the X root is what the server set -// (via Neko or xrandr), and the rest of the stack — mutter, chromium — -// follows from there. Polling the X root rather than chromium's window -// bounds keeps the contract panel-robust: a future mutter config with a -// taskbar/dock would shrink the maximized window below the root size, -// but the root itself would still match what we asked for. -// -// Calls getCurrentResolutionFromXrandr directly rather than the -// higher-level getCurrentResolution: the latter prefers a cached -// viewportOverride when one is set, which would silently validate this -// post-condition against the CDP viewport instead of the X root. The -// override is only set on the headless Xvfb fast path today, so the -// Xorg branch never reaches that case — but the invariant is non-local -// and would silently regress this check if anyone ever sets the -// override on the Xorg path. -// -// Typical convergence is sub-millisecond (Neko's ScreenConfigurationChange -// returns after the X server has applied the mode), but a small loop -// covers any future asynchrony in the resize chain. -func (s *ApiService) waitForXRootSize(ctx context.Context, width, height int, timeout time.Duration) error { - deadline := time.Now().Add(timeout) - var lastW, lastH int - var lastErr error - for { - w, h, _, _, err := s.getCurrentResolutionFromXrandr(ctx) - lastErr = err - if err == nil { - lastW, lastH = w, h - if w == width && h == height { - return nil - } - } - if time.Now().After(deadline) { - if lastErr != nil { - return fmt.Errorf("last getCurrentResolutionFromXrandr error: %w", lastErr) - } - return fmt.Errorf("x root is %dx%d, want %dx%d", lastW, lastH, width, height) - } - select { - case <-ctx.Done(): - return ctx.Err() - case <-time.After(50 * time.Millisecond): - } - } -} - // setViewportViaCDP resizes the browser viewport using the CDP // Emulation.setDeviceMetricsOverride command. This is near-instant and does // not require restarting Chromium or Xvfb. @@ -804,7 +773,12 @@ func (s *ApiService) isNekoEnabled() bool { return os.Getenv("ENABLE_WEBRTC") == "true" } -// setResolutionViaNeko delegates resolution change to Neko API +// setResolutionViaNeko delegates resolution change to Neko API. The +// realized X root dimensions can differ from the request — libxcvt rounds +// widths to the CVT 8-pixel grid and applies a FWXGA bump for +// 1360×768 → 1366×768. Neko's HTTP response echoes the request, not the +// realized size, so the PatchDisplay handler reads the X root via xrandr +// after this call returns. func (s *ApiService) setResolutionViaNeko(ctx context.Context, width, height, refreshRate int) error { log := logger.FromContext(ctx) diff --git a/server/e2e/e2e_display_resize_window_test.go b/server/e2e/e2e_display_resize_window_test.go index 6e47c207..527e1202 100644 --- a/server/e2e/e2e_display_resize_window_test.go +++ b/server/e2e/e2e_display_resize_window_test.go @@ -501,6 +501,89 @@ func patchDisplayExpectingOK(t *testing.T, ctx context.Context, c *TestContainer require.Equal(t, height, *rsp.JSON200.Height) } +// TestDisplayResizeOddWidthHonoursLibxcvtRounding covers the path where a +// caller requests dimensions libxcvt cannot honour exactly. libxcvt rounds +// widths to the CVT 8-pixel grid and then applies a hard-coded FWXGA bump +// for 1360×768 → 1366×768 (xf86EdidModes.c convention so virtual displays +// match real "HD ready" laptop panels). For request 1365×768 the realized +// X root is 1366×768. +// +// Before the fix, waitForXRootSize compared the X root against the +// requested 1365 and returned 500 with "X root verification: x root is +// 1366x768, want 1365x768". The browser pool treats that 500 as fatal — +// the production incident that prompted this test recycled the VM on +// every odd-width /configure call. +// +// After the fix (PatchDisplay reads the realized dimensions back from +// neko's ScreenConfigurationChange response and uses them for both the +// post-condition and the 200 body), the resize succeeds and the response +// truthfully reports 1366×768. +func TestDisplayResizeOddWidthHonoursLibxcvtRounding(t *testing.T) { + if _, err := exec.LookPath("docker"); err != nil { + t.Skipf("docker not available: %v", err) + } + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + // Production-equivalent: Neko + --start-maximized + CHROMIUM_FLAGS + // mirror of run-docker.sh. Same config the real failing instance used. + env := map[string]string{ + "WIDTH": "1024", + "HEIGHT": "768", + "ENABLE_WEBRTC": "true", + "NEKO_ADMIN_PASSWORD": "admin", + "CHROMIUM_FLAGS": "--user-data-dir=/home/kernel/user-data --disable-dev-shm-usage --disable-gpu --start-maximized --disable-software-rasterizer --remote-allow-origins=*", + } + + c := NewTestContainer(t, headfulImage) + require.NoError(t, c.Start(ctx, ContainerConfig{Env: env}), "failed to start container") + defer c.Stop(ctx) + require.NoError(t, c.WaitReady(ctx), "api not ready") + require.NoError(t, c.WaitDevTools(ctx), "devtools not ready") + + navigateBlank(t, ctx, c) + + // Request an odd width — the exact pattern from the production taint. + // libxcvt rounds 1365 to 1360 (CVT 8-pixel grid) then bumps to 1366 + // (FWXGA hack for 1360×768 specifically). The X root lands at 1366×768. + const requestedWidth, height, refreshRate = 1365, 768, 60 + const realizedWidth = 1366 + + client, err := c.APIClient() + require.NoError(t, err) + rate := instanceoapi.PatchDisplayRequestRefreshRate(refreshRate) + w, h := requestedWidth, height + rsp, err := client.PatchDisplayWithResponse(ctx, instanceoapi.PatchDisplayJSONRequestBody{ + Width: &w, + Height: &h, + RefreshRate: &rate, + }) + require.NoError(t, err) + + rootW, rootH, xrErr := getXRootResolution(ctx, c) + require.NoError(t, xrErr, "read x root after resize") + t.Logf("PATCH /display(width=%d, height=%d) -> status=%d body=%s; x_root=%dx%d", + requestedWidth, height, rsp.StatusCode(), strings.TrimSpace(string(rsp.Body)), rootW, rootH) + + // X root reflects libxcvt's realized mode, not the request. + require.Equal(t, realizedWidth, rootW, + "expected libxcvt FWXGA hack to round %dx%d to %dx%d in X root; got %dx%d. If libxcvt's rounding rule changed, update the expected realized width.", + requestedWidth, height, realizedWidth, height, rootW, rootH) + require.Equal(t, height, rootH, "height should pass through unchanged") + + // API now succeeds and reports the realized dimensions — the caller + // learns the screen is 1366×768 even though they asked for 1365×768. + require.Equal(t, http.StatusOK, rsp.StatusCode(), + "PATCH /display: expected 200 after fix, got %d body=%s", rsp.StatusCode(), string(rsp.Body)) + require.NotNil(t, rsp.JSON200, "JSON200 response must be present") + require.NotNil(t, rsp.JSON200.Width, "response must include realized width") + require.NotNil(t, rsp.JSON200.Height, "response must include realized height") + require.Equal(t, realizedWidth, *rsp.JSON200.Width, + "response width should be the realized %d, not the requested %d", realizedWidth, requestedWidth) + require.Equal(t, height, *rsp.JSON200.Height) +} + // waitForXRootResolution polls xrandr until the X root reaches the requested // size, mirroring waitForXvfbResolution but operating on the live X root // instead of the Xvfb process command line. diff --git a/server/lib/nekoclient/client.go b/server/lib/nekoclient/client.go index 68fc1533..514b2b4b 100644 --- a/server/lib/nekoclient/client.go +++ b/server/lib/nekoclient/client.go @@ -118,6 +118,10 @@ func (c *AuthClient) SessionsGet(ctx context.Context) ([]nekooapi.SessionData, e } // 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`, +// not `size`), so callers that need the realized dimensions must read +// the X root directly via xrandr. func (c *AuthClient) ScreenConfigurationChange(ctx context.Context, config nekooapi.ScreenConfiguration) error { c.tokenMu.Lock() defer c.tokenMu.Unlock() From d969e028018f001362551e533ddfb520e86c5f8c Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Thu, 4 Jun 2026 15:33:49 +0000 Subject: [PATCH 2/5] display: poll X root for stability instead of single-read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single xrandr read immediately after neko's resize could catch a transient — chromium running in --kiosk briefly pushes the X root to the dummy DDX's max mode (3840×2160) while mutter settles on the new screen, producing a misleading "realized" size in the response. Replace the single read with a short poll loop that returns early when either (a) xrandr reports the requested size, or (b) consecutive readings are stable for ~150ms. The deadline (10s) only triggers if the X root never converges, in which case the last observation is still returned — the path stays non-fatal. Co-Authored-By: Claude Opus 4.7 --- server/cmd/api/api/display.go | 82 +++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 13 deletions(-) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index d470da85..3ebae008 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -143,25 +143,29 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ // accepted for API compatibility but no longer triggers a // restart on this path. if err == nil { - // Read the X root immediately after neko's synchronous - // resize, before any window-state CDP calls. This captures - // libxcvt's realized size — which can differ from the - // request (CVT 8-pixel grid round + FWXGA bump for - // 1360×768 → 1366×768) — without racing the chromium - // maximize that follows. Returning the realized size in - // the response body lets callers' coordinate math line up - // with what the X server actually rendered. - realizedW, realizedH, _, _, xrErr := s.getCurrentResolutionFromXrandr(ctx) - if xrErr != nil { - log.Error("failed to read X root after resize", "error", xrErr) - err = fmt.Errorf("X root read: %w", xrErr) - } else { + // Wait for the X root to settle, then use it as the + // realized size in the response. The wait returns early + // when either (a) xrandr reports the requested size — the + // common case — or (b) consecutive reads are stable for a + // short window, capturing libxcvt's rounded size on + // requests it can't honour exactly (CVT 8-pixel grid + + // FWXGA bump for 1360×768 → 1366×768). + // + // The poll absorbs transient X root states — chromium + // running in --kiosk briefly pushes the root to the dummy + // DDX's max mode (3840×2160) while mutter settles on the + // new screen, and a single immediate read would catch that + // transient instead of the steady-state size. + realizedW, realizedH := s.waitForXRootRealized(ctx, width, height, 10*time.Second) + if realizedW > 0 && realizedH > 0 { if realizedW != width || realizedH != height { log.Info("X root differs from request after resize", "requested", fmt.Sprintf("%dx%d", width, height), "realized", fmt.Sprintf("%dx%d", realizedW, realizedH)) } width, height = realizedW, realizedH + } else { + log.Warn("X root never read successfully after resize, returning requested dimensions") } } if err == nil { @@ -465,6 +469,58 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { return nil } +// waitForXRootRealized polls the X root via xrandr after a resize and +// returns the realized dimensions. It returns early on either of two +// success conditions: +// +// 1. The reading matches the requested width/height — the common case, +// when libxcvt honoured the request exactly. +// 2. The reading has been stable across stableReads consecutive samples — +// captures libxcvt's rounded size on requests it can't honour +// (CVT 8-pixel grid round + FWXGA bump for 1360×768 → 1366×768). +// +// If neither happens before timeout, returns the last observation. Always +// non-fatal — the response always echoes some size, never 500s. +// +// Calls getCurrentResolutionFromXrandr directly rather than the higher- +// level getCurrentResolution: the latter prefers a cached viewportOverride +// when one is set, which would silently report the CDP viewport instead +// of the X root. The override is only set on the headless Xvfb fast path +// today, so the Xorg branch never reaches that case — but the invariant +// is non-local and would silently regress if anyone ever sets the +// override on the Xorg path. +func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, timeout time.Duration) (int, int) { + const stableReads = 3 + deadline := time.Now().Add(timeout) + var lastW, lastH int + var stableCount int + for { + w, h, _, _, err := s.getCurrentResolutionFromXrandr(ctx) + if err == nil { + if w == wantW && h == wantH { + return w, h + } + if w == lastW && h == lastH { + stableCount++ + if stableCount >= stableReads { + return w, h + } + } else { + stableCount = 1 + lastW, lastH = w, h + } + } + if time.Now().After(deadline) { + return lastW, lastH + } + select { + case <-ctx.Done(): + return lastW, lastH + case <-time.After(50 * time.Millisecond): + } + } +} + // setViewportViaCDP resizes the browser viewport using the CDP // Emulation.setDeviceMetricsOverride command. This is near-instant and does // not require restarting Chromium or Xvfb. From 1e848b775620943da300f3ee5db888869a0f2e57 Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Thu, 4 Jun 2026 15:42:43 +0000 Subject: [PATCH 3/5] display: don't accept pre-resize baseline as realized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit waitForXRootRealized would treat any three consecutive stable readings as the realized size, which silently accepts the pre-resize baseline if the X server hasn't committed the new mode yet. The response would echo back the old dimensions as 'realized' — a success-looking lie. Capture the pre-resize X root, pass it through, and exclude it from the stable-match condition. The match-the-request fast path is unchanged. Co-Authored-By: Claude Opus 4.7 --- server/cmd/api/api/display.go | 44 +++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index 3ebae008..71e36bf8 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -146,17 +146,18 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ // Wait for the X root to settle, then use it as the // realized size in the response. The wait returns early // when either (a) xrandr reports the requested size — the - // common case — or (b) consecutive reads are stable for a - // short window, capturing libxcvt's rounded size on - // requests it can't honour exactly (CVT 8-pixel grid + - // FWXGA bump for 1360×768 → 1366×768). + // common case — or (b) consecutive reads are stable at a + // value that is NOT the pre-resize baseline (the resize + // has propagated and libxcvt's rounded size is the steady + // state, e.g. CVT 8-pixel grid + FWXGA bump for + // 1360×768 → 1366×768). // - // The poll absorbs transient X root states — chromium - // running in --kiosk briefly pushes the root to the dummy - // DDX's max mode (3840×2160) while mutter settles on the - // new screen, and a single immediate read would catch that - // transient instead of the steady-state size. - realizedW, realizedH := s.waitForXRootRealized(ctx, width, height, 10*time.Second) + // Excluding the baseline from the stable-match case is the + // guard against silently returning the pre-resize size as + // "realized" if the X server hasn't committed yet. The + // match-the-request path is unaffected and still returns + // immediately when the screen reaches the requested size. + realizedW, realizedH := s.waitForXRootRealized(ctx, width, height, currentWidth, currentHeight, 10*time.Second) if realizedW > 0 && realizedH > 0 { if realizedW != width || realizedH != height { log.Info("X root differs from request after resize", @@ -475,12 +476,20 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // // 1. The reading matches the requested width/height — the common case, // when libxcvt honoured the request exactly. -// 2. The reading has been stable across stableReads consecutive samples — -// captures libxcvt's rounded size on requests it can't honour -// (CVT 8-pixel grid round + FWXGA bump for 1360×768 → 1366×768). +// 2. The reading has been stable across stableReads consecutive samples +// AT A VALUE OTHER THAN the pre-resize baseline — captures libxcvt's +// rounded size on requests it can't honour exactly (CVT 8-pixel grid +// round + FWXGA bump for 1360×768 → 1366×768). // -// If neither happens before timeout, returns the last observation. Always -// non-fatal — the response always echoes some size, never 500s. +// Excluding the baseline from stable matches prevents the pathological +// case where the X server hasn't committed the new mode yet and every +// reading still shows the pre-resize size. Without this guard, the +// caller would get 200 with the pre-resize dimensions echoed back as +// "realized" — a silent failure that looks like success. +// +// If neither condition fires before the timeout, returns the last +// observation. Always non-fatal — the response always echoes some size, +// never 500s. // // Calls getCurrentResolutionFromXrandr directly rather than the higher- // level getCurrentResolution: the latter prefers a cached viewportOverride @@ -489,7 +498,7 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // today, so the Xorg branch never reaches that case — but the invariant // is non-local and would silently regress if anyone ever sets the // override on the Xorg path. -func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, timeout time.Duration) (int, int) { +func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH, baselineW, baselineH int, timeout time.Duration) (int, int) { const stableReads = 3 deadline := time.Now().Add(timeout) var lastW, lastH int @@ -500,7 +509,8 @@ func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, if w == wantW && h == wantH { return w, h } - if w == lastW && h == lastH { + isBaseline := w == baselineW && h == baselineH + if w == lastW && h == lastH && !isBaseline { stableCount++ if stableCount >= stableReads { return w, h From 07268efa363f8902b8e820095c599cd4d3dfae0a Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Thu, 4 Jun 2026 15:54:11 +0000 Subject: [PATCH 4/5] display: revert baseline-exclusion guard from waitForXRootRealized MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit added a guard that refused to accept the pre-resize baseline as a stable-N match, defending against a hypothetical race where the X server hadn't committed the new mode yet. In practice XSetScreenConfiguration is a synchronous X protocol round-trip, so by the time neko's call returns the server has committed — that race doesn't exist. The guard had a real cost: when a request rounds back to the current X root (idempotent re-PATCH of an odd width, or any resize whose libxcvt realization equals the current screen), the poll loop would never declare baseline-stable and would wait the full 10s before returning. Drop the guard. Stable-N now accepts any value, including the baseline. Co-Authored-By: Claude Opus 4.7 --- server/cmd/api/api/display.go | 50 ++++++++++++++++------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index 71e36bf8..eceb8cad 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -146,18 +146,17 @@ func (s *ApiService) PatchDisplay(ctx context.Context, req oapi.PatchDisplayRequ // Wait for the X root to settle, then use it as the // realized size in the response. The wait returns early // when either (a) xrandr reports the requested size — the - // common case — or (b) consecutive reads are stable at a - // value that is NOT the pre-resize baseline (the resize - // has propagated and libxcvt's rounded size is the steady - // state, e.g. CVT 8-pixel grid + FWXGA bump for - // 1360×768 → 1366×768). + // common case — or (b) consecutive reads are stable for a + // short window, capturing libxcvt's rounded size on + // requests it can't honour exactly (CVT 8-pixel grid + + // FWXGA bump for 1360×768 → 1366×768). // - // Excluding the baseline from the stable-match case is the - // guard against silently returning the pre-resize size as - // "realized" if the X server hasn't committed yet. The - // match-the-request path is unaffected and still returns - // immediately when the screen reaches the requested size. - realizedW, realizedH := s.waitForXRootRealized(ctx, width, height, currentWidth, currentHeight, 10*time.Second) + // The poll absorbs transient X root states — chromium + // running in --kiosk briefly pushes the root to the dummy + // DDX's max mode (3840×2160) while mutter settles on the + // new screen, and a single immediate read would catch that + // transient instead of the steady-state size. + realizedW, realizedH := s.waitForXRootRealized(ctx, width, height, 10*time.Second) if realizedW > 0 && realizedH > 0 { if realizedW != width || realizedH != height { log.Info("X root differs from request after resize", @@ -476,20 +475,18 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // // 1. The reading matches the requested width/height — the common case, // when libxcvt honoured the request exactly. -// 2. The reading has been stable across stableReads consecutive samples -// AT A VALUE OTHER THAN the pre-resize baseline — captures libxcvt's -// rounded size on requests it can't honour exactly (CVT 8-pixel grid -// round + FWXGA bump for 1360×768 → 1366×768). +// 2. The reading has been stable across stableReads consecutive samples — +// captures libxcvt's rounded size on requests it can't honour +// (CVT 8-pixel grid round + FWXGA bump for 1360×768 → 1366×768) and +// covers the idempotent re-PATCH case where the request rounds back +// to the current X root. // -// Excluding the baseline from stable matches prevents the pathological -// case where the X server hasn't committed the new mode yet and every -// reading still shows the pre-resize size. Without this guard, the -// caller would get 200 with the pre-resize dimensions echoed back as -// "realized" — a silent failure that looks like success. -// -// If neither condition fires before the timeout, returns the last -// observation. Always non-fatal — the response always echoes some size, -// never 500s. +// If neither happens before timeout, returns the last observation. Always +// non-fatal — the response always echoes some size, never 500s. Since +// XSetScreenConfiguration is a synchronous X protocol round-trip, the X +// server has committed by the time neko's call returns, so the first +// reading reliably reflects the realized state — no separate guard for +// "pre-resize baseline still showing" is needed. // // Calls getCurrentResolutionFromXrandr directly rather than the higher- // level getCurrentResolution: the latter prefers a cached viewportOverride @@ -498,7 +495,7 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // today, so the Xorg branch never reaches that case — but the invariant // is non-local and would silently regress if anyone ever sets the // override on the Xorg path. -func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH, baselineW, baselineH int, timeout time.Duration) (int, int) { +func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, timeout time.Duration) (int, int) { const stableReads = 3 deadline := time.Now().Add(timeout) var lastW, lastH int @@ -509,8 +506,7 @@ func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH, bas if w == wantW && h == wantH { return w, h } - isBaseline := w == baselineW && h == baselineH - if w == lastW && h == lastH && !isBaseline { + if w == lastW && h == lastH { stableCount++ if stableCount >= stableReads { return w, h From 2050e6adb937d2a09a7ebbce3b0bba433f287d3b Mon Sep 17 00:00:00 2001 From: hiroTamada <88675973+hiroTamada@users.noreply.github.com> Date: Thu, 4 Jun 2026 17:03:04 +0000 Subject: [PATCH 5/5] display: reject transient far-from-request xrandr readings on stable-N path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Why: chromium --start-maximized briefly drives xrandr to report the dummy DDX's max mode (e.g. 3840x2160) while mode-switch propagates. A naive stable-N would echo that transient into the response body. Real libxcvt rounding is <16 px; the dummy max is >1000 px off — acceptableDelta=32 sits between them. --- server/cmd/api/api/display.go | 37 +++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/server/cmd/api/api/display.go b/server/cmd/api/api/display.go index eceb8cad..28781ea7 100644 --- a/server/cmd/api/api/display.go +++ b/server/cmd/api/api/display.go @@ -475,18 +475,23 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // // 1. The reading matches the requested width/height — the common case, // when libxcvt honoured the request exactly. -// 2. The reading has been stable across stableReads consecutive samples — -// captures libxcvt's rounded size on requests it can't honour -// (CVT 8-pixel grid round + FWXGA bump for 1360×768 → 1366×768) and -// covers the idempotent re-PATCH case where the request rounds back -// to the current X root. +// 2. The reading has been stable across stableReads consecutive samples +// AT A VALUE CLOSE TO THE REQUEST — captures libxcvt's rounded size +// on requests it can't honour exactly (CVT 8-pixel grid round + +// FWXGA bump for 1360×768 → 1366×768) and covers the idempotent +// re-PATCH case. // -// If neither happens before timeout, returns the last observation. Always -// non-fatal — the response always echoes some size, never 500s. Since -// XSetScreenConfiguration is a synchronous X protocol round-trip, the X -// server has committed by the time neko's call returns, so the first -// reading reliably reflects the realized state — no separate guard for -// "pre-resize baseline still showing" is needed. +// The "close to request" guard on the stable-N path rejects transient +// xrandr readings far from the request — chromium running in +// --start-maximized or --kiosk briefly drives xrandr to report the dummy +// DDX's max mode (e.g. 3840×2160) while mode-switch propagates, and a +// naive stable-N would echo that transient into the response body. Real +// libxcvt rounding is <16 px; the dummy max is >1000 px off any normal +// request — acceptableDelta=32 sits comfortably between them. +// +// If neither condition fires before the timeout, returns the last +// observation. Always non-fatal — the response always echoes some size, +// never 500s. // // Calls getCurrentResolutionFromXrandr directly rather than the higher- // level getCurrentResolution: the latter prefers a cached viewportOverride @@ -497,6 +502,7 @@ func (s *ApiService) setWindowMaximizedViaCDP(ctx context.Context) error { // override on the Xorg path. func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, timeout time.Duration) (int, int) { const stableReads = 3 + const acceptableDelta = 32 deadline := time.Now().Add(timeout) var lastW, lastH int var stableCount int @@ -508,7 +514,7 @@ func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, } if w == lastW && h == lastH { stableCount++ - if stableCount >= stableReads { + if stableCount >= stableReads && abs(w-wantW) <= acceptableDelta && abs(h-wantH) <= acceptableDelta { return w, h } } else { @@ -527,6 +533,13 @@ func (s *ApiService) waitForXRootRealized(ctx context.Context, wantW, wantH int, } } +func abs(x int) int { + if x < 0 { + return -x + } + return x +} + // setViewportViaCDP resizes the browser viewport using the CDP // Emulation.setDeviceMetricsOverride command. This is near-instant and does // not require restarting Chromium or Xvfb.