test(e2e): replace the layout specs' unconditional sleeps with causal waits (#2648)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

apps/web/e2e/helpers/dockview-resize.ts

@@ -49,6 +49,71 @@ export async function getDockviewGroupWidth(page: Page, panelId: string): Promis
   }, panelId);
 }
 
+/** Read the dockview container's live pixel width. */
+export async function getDockviewContainerWidth(page: Page): Promise<number> {
+  return page.evaluate(() =>
+    Math.round(document.querySelector(".dv-dockview")?.getBoundingClientRect().width ?? -1),
+  );
+}
+
+/**
+ * Wait for a `setViewportSize` to have reached dockview's layout.
+ *
+ * Two conditions, both required, evaluated together in one `page.evaluate` so
+ * they describe a single instant:
+ *
+ *  1. the container's CSS box has moved off the width it had before, and
+ *  2. dockview's own `api.width` agrees with that box.
+ *
+ * (1) alone is not enough, and the reason is subtle. The app syncs a container
+ * resize into `api.layout` from `setupContainerResizeSync`'s ResizeObserver;
+ * measured under load (12 saturated cores, 6x CDP CPU throttling, five
+ * scenarios: auto shrink and grow, manual-width shrink and grow, over-cap
+ * re-clamp), that sync is already complete at the first ResizeObserver
+ * delivery reporting the new box, right column included. But a poll does not
+ * observe via ResizeObserver: `getBoundingClientRect()` forces a synchronous
+ * reflow, so it can read the post-resize box in a task that runs *before* that
+ * delivery, and therefore before `api.layout`. Condition (2) closes that
+ * window by reading the value the sync actually produces.
+ *
+ * (2) alone would be the already-true trap: `api.width` matches the box before
+ * the viewport changes as well, so it is satisfied instantly. Only the pair is
+ * causal, which is why this is not simply a poll on the expected group width.
+ * That matters most for the callers asserting a width must **not** change
+ * across the resize: they have no event of their own, and polling the value
+ * they expect would pass against the width that was already there.
+ *
+ * A 1px tolerance absorbs fractional-vs-rounded differences between the two
+ * readings; it is far below any real column-width assertion's slack.
+ */
+export async function waitForDockviewViewportResize(
+  page: Page,
+  previousContainerWidth: number,
+  timeout = 5_000,
+): Promise<void> {
+  await expect
+    .poll(
+      () =>
+        page.evaluate((previous) => {
+          const element = document.querySelector(".dv-dockview");
+          const api = (window as unknown as { __dockviewApi__?: { width: number } })
+            .__dockviewApi__;
+          if (!element) return "no dockview container";
+          if (!api) return "dockview api not exposed";
+          const css = Math.round(element.getBoundingClientRect().width);
+          if (css === previous) return `container still ${css}px`;
+          const layout = Math.round(api.width);
+          if (Math.abs(layout - css) > 1) return `container ${css}px but api.width ${layout}px`;
+          return "settled";
+        }, previousContainerWidth),
+      {
+        timeout,
+        message: `dockview never took the new viewport (was ${previousContainerWidth}px)`,
+      },
+    )
+    .toBe("settled");
+}
+
 /** Read the live pixel width of a dockview group by group ID. */
 export async function getDockviewGroupWidthById(page: Page, groupId: string): Promise<number> {
   return page.evaluate((id) => {

apps/web/e2e/tests/layout/changes-panel-focus.spec.ts

@@ -6,6 +6,7 @@ import fs from "node:fs";
 import path from "node:path";
 import { execSync } from "node:child_process";
 import { waitForSessionDone } from "../../helpers/session";
+import { dwell } from "../../helpers/causal-waits";
 
 /** Minimal git helper for E2E tests - runs git commands in the test repository. */
 class GitHelper {
@@ -284,8 +285,12 @@ test.describe("Changes panel focus behavior", () => {
       timeout: 15_000,
     });
 
-    // Wait a bit for any async auto-activate to fire
-    await testPage.waitForTimeout(2_000);
+    await dwell(
+      testPage,
+      2_000,
+      "negative-assertion",
+      "the assertion below is that the changes tab does not steal activation; an auto-activate that must never happen publishes nothing, so the only way to give a regression room to fire is to let its window elapse",
+    );
 
     await expect(changesTab(testPage)).not.toHaveClass(/dv-active-tab/, { timeout: 5_000 });
 

apps/web/e2e/tests/layout/pane-resize-edge-cases.spec.ts

@@ -33,7 +33,18 @@ test.describe("Pane resize edge cases", () => {
       if (!matching) throw new Error("files group not found");
       matching.api.maximize();
     });
-    await testPage.waitForTimeout(150);
+    // Wait for the maximized layout to be applied rather than for a number.
+    // `hasMaximizedGroup()` flips synchronously inside `maximize()`, so polling
+    // it would guard nothing; the group growing is the consequence that has to
+    // land before exiting is a real round trip. Measured, maximize takes this
+    // group from the 600px set above to ~1000px -- it does not fill the
+    // container, so this asserts growth rather than a proportion.
+    await expect
+      .poll(() => getDockviewGroupWidth(testPage, "files"), {
+        timeout: 5_000,
+        message: `files group never grew past its pre-maximize ${before}px`,
+      })
+      .toBeGreaterThan(before + 50);
     await testPage.evaluate(() => {
       type GroupApi = { exitMaximized: () => void };
       type Group = { id: string; panels: { id: string }[]; api: GroupApi };
@@ -67,10 +78,31 @@ test.describe("Pane resize edge cases", () => {
     const errors: string[] = [];
     testPage.on("pageerror", (err) => errors.push(err.message));
 
-    await testPage.locator("body").click({ position: { x: 5, y: 5 } });
-    const mod = process.platform === "darwin" ? "Meta" : "Control";
-    await testPage.keyboard.press(`${mod}+b`);
-    await testPage.waitForTimeout(250);
+    // This used to press Ctrl+B and sleep 250ms. Ctrl+B does nothing:
+    // `CONFIGURABLE_SHORTCUTS.TOGGLE_SIDEBAR` defaults to `UNBOUND_SHORTCUT`,
+    // so the binding exists only for users who set one. Nothing downstream
+    // asserted on the sidebar, so the test has been resizing with the sidebar
+    // still open -- a duplicate of the case above rather than the edge case it
+    // is named for. Collapse it through the store instead; the subject here is
+    // the resize, not how the collapse was triggered.
+    await testPage.evaluate(() => {
+      type StoreWindow = Window & {
+        __KANDEV_E2E_STORE__?: {
+          getState: () => { setAppSidebarCollapsed: (collapsed: boolean) => void };
+        };
+      };
+      const store = (window as StoreWindow).__KANDEV_E2E_STORE__;
+      if (!store) throw new Error("E2E store bridge missing");
+      store.getState().setAppSidebarCollapsed(true);
+    });
+    // Assert the collapse instead of budgeting for it: this fails if the flag
+    // stops reaching the DOM, where the old sleep could not. The 300ms width
+    // animation that follows is deliberately not waited out -- the assertions
+    // below are "no page error" and "layout still healthy", neither of which
+    // reads geometry.
+    await expect(testPage.getByTestId("app-sidebar")).toHaveAttribute("data-collapsed", "true", {
+      timeout: 5_000,
+    });
 
     await resizeColumnViaSplitview(testPage, "right", 600);
     await session.expectLayoutHealthy();

apps/web/e2e/tests/layout/pane-resize-right.spec.ts

@@ -6,7 +6,9 @@ import {
   openWideTask,
   expectApproxWidth,
   getDockviewGroupWidth,
+  getDockviewContainerWidth,
   resizeColumnViaSplitview,
+  waitForDockviewViewportResize,
 } from "../../helpers/dockview-resize";
 import { KanbanPage } from "../../pages/kanban-page";
 import { SessionPage } from "../../pages/session-page";
@@ -42,13 +44,15 @@ test.describe("Right pane resize — container-proportional cap", () => {
     const largeWidth = await getDockviewGroupWidth(testPage, "files");
     expectApproxWidth(largeWidth, 450, 12);
 
+    const wideContainer = await getDockviewContainerWidth(testPage);
     await testPage.setViewportSize({ width: 1280, height: 900 });
-    await testPage.waitForTimeout(500);
+    await waitForDockviewViewportResize(testPage, wideContainer);
     const laptopWidth = await getDockviewGroupWidth(testPage, "files");
     expectApproxWidth(laptopWidth, 288, 12);
 
+    const laptopContainer = await getDockviewContainerWidth(testPage);
     await testPage.setViewportSize({ width: 2200, height: 900 });
-    await testPage.waitForTimeout(500);
+    await waitForDockviewViewportResize(testPage, laptopContainer);
     expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), largeWidth, 12);
   });
 
@@ -64,12 +68,19 @@ test.describe("Right pane resize — container-proportional cap", () => {
     const manualWidth = await resizeColumnViaSplitview(testPage, "right", 360);
     expectApproxWidth(manualWidth, 360, 12);
 
+    // The assertion is that this width does *not* move, so there is nothing to
+    // poll for: the expected value is already there. Waiting for the container
+    // to take the new viewport is what gives a regression somewhere to happen,
+    // since the recompute that could clobber the manual width runs in that same
+    // layout pass.
+    const wideContainer = await getDockviewContainerWidth(testPage);
     await testPage.setViewportSize({ width: 1280, height: 900 });
-    await testPage.waitForTimeout(500);
+    await waitForDockviewViewportResize(testPage, wideContainer);
     expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), manualWidth, 12);
 
+    const laptopContainer = await getDockviewContainerWidth(testPage);
     await testPage.setViewportSize({ width: 2200, height: 900 });
-    await testPage.waitForTimeout(500);
+    await waitForDockviewViewportResize(testPage, laptopContainer);
     expectApproxWidth(await getDockviewGroupWidth(testPage, "files"), manualWidth, 12);
   });
 
@@ -186,11 +197,12 @@ test.describe("Right pane resize — container-proportional cap", () => {
     const wideWidth = await resizeColumnViaSplitview(testPage, "right", 900);
     expect(wideWidth).toBeGreaterThan(700);
 
+    // Do not call the resize helper here: it reapplies constraints and would
+    // mask a failure to update them automatically on viewport changes. Wait for
+    // the container instead, which is where the pinned-target enforcement runs.
+    const wideContainer = await getDockviewContainerWidth(testPage);
     await testPage.setViewportSize({ width: 1100, height: 800 });
-    // Allow the container ResizeObserver and pinned-target enforcement to
-    // settle. Do not call the resize helper here: it reapplies constraints and
-    // would mask a failure to update them automatically on viewport changes.
-    await testPage.waitForTimeout(500);
+    await waitForDockviewViewportResize(testPage, wideContainer);
 
     const dockviewBox = await testPage.locator(".dv-dockview").boundingBox();
     expect(dockviewBox).not.toBeNull();
@@ -259,13 +271,18 @@ test.describe("Right pane width — per-task isolation", () => {
       timeout: 15_000,
     });
     await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 });
-    // Allow fromJSON + fixups-capture + enforcePinnedTargets to settle.
-    await testPage.waitForTimeout(500);
 
-    const widthB = await getDockviewGroupWidth(testPage, "files");
-    expect(
-      widthB,
-      `Task B right width ${widthB} should be the default (>350), not Task A's narrow ${narrowedA}`,
-    ).toBeGreaterThan(350);
+    // Poll rather than sleep-then-read: the width is Task A's narrow value
+    // until fromJSON + fixups-capture + enforcePinnedTargets have run, so this
+    // still fails if the leak comes back. Sampled every frame through a real
+    // switch, the width makes exactly one move (240 -> 384 at ~180ms) and then
+    // holds for the next 5.8s, so there is no transient correct value for the
+    // poll to latch onto early.
+    await expect
+      .poll(() => getDockviewGroupWidth(testPage, "files"), {
+        timeout: 10_000,
+        message: `Task B right width should be the default (>350), not Task A's narrow ${narrowedA}`,
+      })
+      .toBeGreaterThan(350);
   });
 });

apps/web/e2e/tests/layout/preview-tab-session-switch.spec.ts

@@ -6,6 +6,7 @@ import type { ApiClient } from "../../helpers/api-client";
 import { SessionPage } from "../../pages/session-page";
 import { GitHelper, makeGitEnv } from "../../helpers/git-helper";
 import { KanbanPage } from "../../pages/kanban-page";
+import { dwell } from "../../helpers/causal-waits";
 
 const FILE_A = "alpha.ts";
 const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"];
@@ -180,9 +181,13 @@ test.describe("Preview tab survives session switch", () => {
     await expect(testPage).toHaveURL((url) => url.pathname.includes(taskA.id), {
       timeout: 15_000,
     });
-    // Wait for layout to stabilize after fromJSON restore
     await expect(testPage.locator(".dv-dockview")).toBeVisible({ timeout: 15_000 });
-    await testPage.waitForTimeout(1_000);
+    await dwell(
+      testPage,
+      1_000,
+      "negative-assertion",
+      "the count assertion below auto-retries, so it already covers a slow restore; what it cannot cover is a duplicate tab appearing after it first sees one, and a tab that must never be added publishes nothing to wait on",
+    );
 
     // File tab should be restored (preview or pinned — the file was open)
     const fileTab = testPage.locator(".dv-default-tab").filter({ hasText: FILE_A });