display: converge X root on the configure stop/start resize path

[Sayan-]

Problem

On the batched chromium-configure stop/start path (chromiumDisplayApplyWhileStopped), the Neko screen resize was fire-and-forget: it called setResolutionViaNeko and returned as soon as Neko ACKed, without waiting for the X root to actually reach the requested size. Neko applies screen config asynchronously and can silently drop a reconfig that lands while a previous one is in flight, leaving the root at the dummy DDX default (3840x2160). Chromium then relaunches in --kiosk and maximizes onto the stale root, so the live view shows the browser in a small corner of the frame. Intermittent, and the call returns 200 despite the wrong size.

This branch runs whenever a profile/extension/policy/flags change is bundled with a viewport change in the same configure call. The live-resize path (PatchDisplay) already guards against this (poll + retry, #280); the stop/start branch never got that guard.

Fix

Extract the apply-poll-retry loop into applyResolutionAndConverge and route both paths through it, so the convergence guarantee can't drift between them again. PatchDisplay behavior is unchanged; the stop/start path now waits for the resize to take effect before Chromium relaunches.

Testing

Static

go build, go vet, go test ./cmd/api/api/, gofmt all clean. No unit test (helper needs live X/Neko; matches the untested #280 loop).

Manual live (headful docker container, ENABLE_WEBRTC=true so the Neko path is exercised)

Approach: drive the real HTTP API against a running chromium-headful container. To hit the fixed stop/start branch, every POST /configure bundled a restart-triggering part (chromium_flags) with a display viewport change, so the resize runs through applyResolutionAndConverge while Chromium is stopped. After each call: read the X root via xrandr and assert it reached the request (within the ±32 libxcvt-rounding delta), and capture a full-screen screenshot to confirm the browser fills the root rather than sitting in a corner of a stale 3840×2160 frame.

Suite	Coverage	Result
Stop/start convergence	40 POST /configure (flags + display), cycling 1280×720 / 1600×900 / 1024×768	40/40 X root == request, ~4.6s each
Live PATCH /display regression	1440×900, 1920×1080	2/2 converged, CDP maximize re-assert fired
No-op skip	re-send current size	200, resize skipped
Non-standard sizes	1360×768, 1366×768, 1365×769, 1111×777, 999×666, 800×600, 1920×1200, 1234×567, 1023×767, 1728×1117	10/10 within ±32

Log evidence (52 successful resizes across all suites): using Neko API for Xorg resolution change and successfully changed resolution via Neko API fired on every resize (Neko path, not the xrandr fallback); retrying neko reconfig = 0 and did not converge to requested mode = 0.

Non-standard sizes confirmed libxcvt behaves as the code comments document: widths snap down to the CVT 8-pixel grid (≤7px observed) with heights untouched, and the 1360→1366 FWXGA bump fired exactly. Real rounding topped out at 7px — comfortably inside the ±32 acceptableDelta and far from the ~1000px+ dummy-DDX transient that delta is meant to reject.

Not covered: the intermittent dropped-reconfig race never reproduced live (timing-sensitive, low-probability — consistent with the original repro difficulty), so the neko-retry and fail-loud 500 branches were not observed firing. The tests prove the convergence guarantee holds on the happy path; the mechanism for the failure path remains confirmed only in code and prod logs (frequent realized=3840x2160 drops, recovered on the guarded path).

---

Note

Medium Risk
Touches headful display resize timing on two API paths; live-path behavior is largely preserved but the stop/start path now fails configure when X never converges instead of silently relaunching Chromium on a wrong root.

Overview
Xorg resize now shares one convergence path for live PatchDisplay and batched chromium configure while Chromium is stopped. The apply → poll X root → Neko retry loop moves into applyResolutionAndConverge in display.go.

The configure stop/start path (chromiumDisplayApplyWhileStopped) no longer calls Neko/xrandr and returns on ACK alone. It waits until the X root reaches the requested size (with the same Neko reconfig retries as the live path) before Chromium relaunches in --kiosk, avoiding intermittent wrong live view when Neko drops an in-flight reconfig and X stays at 3840×2160.

PatchDisplay on Xorg is refactored to call the helper; realized dimensions and CDP maximize re-assert stay on the caller. Non-convergence still fails the request instead of returning 200 with a stale root.

^{Reviewed by Cursor Bugbot for commit 189d12d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[hiroTamada]

reviewed — approving. clean, well-scoped change. the extraction is behavior-preserving on the live-resize side (traced each branch: set-fail, converge-fail, realized≠requested, cdp re-assert all match pre-PR), and routing the configure stop/start path through the shared routine is the right fix — this regression only existed because #280 patched one of the two mirror paths and this closes the other while removing the duplication that let them diverge.

a couple of non-blocking questions worth answering on the record:

Questions (non-blocking)

• server/cmd/api/api/chromium_configure.go:692-696 — the xrandr sub-branch here still calls setResolutionXorgViaXrandr directly, bypassing applyResolutionAndConverge, so the configure stop/start path relaunches --kiosk with no X-root convergence check when neko is disabled. intended because xrandr applies synchronously (no async drop, so no stale-root window)? if so, a one-line comment saying that would help — and note the helper's doc comment ("the configure stop/start path … must go through here") slightly overclaims, since this sub-path deliberately doesn't. (this branch is unchanged by the PR, so not a regression either way.)
• server/cmd/api/api/chromium_configure.go:693 — this path can now return 500 (and block up to ~3×10s ≈ 30s on non-convergence) where it previously returned 200 instantly after the neko ACK. that's the intended fail-loud fix, just confirming the new contract + latency ceiling is expected for callers of configure-with-viewport.

Nits

• server/cmd/api/api/display.go:120-143 — two back-to-back if err == nil { blocks read a little awkwardly now that the first is short; could fold under one guard. pure style, optional.
• server/cmd/api/api/display.go:542-546 — "every caller must go through here" is convention-only (nothing stops a direct setResolutionViaNeko call). fine as-is, just noting.