diff --git a/apps/web/e2e/fixtures/backend.ts b/apps/web/e2e/fixtures/backend.ts index 02cf695445..5d28e2db03 100644 --- a/apps/web/e2e/fixtures/backend.ts +++ b/apps/web/e2e/fixtures/backend.ts @@ -5,6 +5,7 @@ import os from "node:os"; import path from "node:path"; import { BackendFixtureEnvOverrides, createScopedEnvUse } from "./backend-env"; import { E2E_DOCKER_SCOPE } from "./docker-probe"; +import { dwell } from "../helpers/causal-waits"; const BACKEND_DIR = path.resolve(__dirname, "../../../../apps/backend"); const WEB_DIR = path.resolve(__dirname, "../.."); @@ -123,7 +124,11 @@ export async function waitForHealth( } catch { // not ready yet } - await new Promise((r) => setTimeout(r, HEALTH_POLL_MS)); + await dwell( + HEALTH_POLL_MS, + "poll-interval", + "sampling interval for the backend health probe; the process is still starting, so there is nothing to subscribe to and the only signal is the port answering", + ); } throw new Error(`Service did not become healthy at ${url} within ${timeoutMs}ms`); } finally { @@ -150,7 +155,11 @@ async function waitForPortFree(port: number, timeoutMs = 10_000): Promise sock.once("error", () => resolve(true)); // ECONNREFUSED → port is free }); if (free) return; - await new Promise((r) => setTimeout(r, 100)); + await dwell( + 100, + "poll-interval", + "sampling interval while waiting for the previous backend to release its port; the OS publishes nothing when a socket is finally freed", + ); } // Timeout expired — proceed anyway; the new process will fail-fast if the // port is still held and waitForHealth will surface the error. diff --git a/apps/web/e2e/fixtures/docker-probe.ts b/apps/web/e2e/fixtures/docker-probe.ts index fe7e540d39..82a0f5357c 100644 --- a/apps/web/e2e/fixtures/docker-probe.ts +++ b/apps/web/e2e/fixtures/docker-probe.ts @@ -3,6 +3,7 @@ import { randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { dwell } from "../helpers/causal-waits"; /** * Image used by the Docker E2E project. Built once per machine and reused. @@ -111,7 +112,11 @@ export async function waitForScopedKandevContainersRemoved( while (Date.now() < deadline) { removeScopedKandevContainers(scope); if (scopedKandevContainerIDs(scope).length === 0) return; - await new Promise((resolve) => setTimeout(resolve, 100)); + await dwell( + 100, + "poll-interval", + "sampling interval for the container-teardown loop above; docker rm is asynchronous and the CLI reports completion only by the container list emptying", + ); } removeScopedKandevContainers(scope); const remaining = scopedKandevContainerIDs(scope); diff --git a/apps/web/e2e/fixtures/test-base.ts b/apps/web/e2e/fixtures/test-base.ts index 52fd85f3f9..aedffa5f70 100644 --- a/apps/web/e2e/fixtures/test-base.ts +++ b/apps/web/e2e/fixtures/test-base.ts @@ -7,6 +7,7 @@ import { ApiClient } from "../helpers/api-client"; import { PrAssetCapture } from "../helpers/pr-asset-capture"; import { makeGitEnv } from "../helpers/git-helper"; import type { WorkflowStep } from "../../lib/types/http"; +import { dwell } from "../helpers/causal-waits"; const DEFAULT_SIDEBAR_VIEW = { id: "view-all-tasks", @@ -73,7 +74,11 @@ export const test = backendFixture.extend< lastText = await probe.text(); break; } - await new Promise((r) => setTimeout(r, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the mock-harness health probe above; the backend is still booting and there is no page, let alone a socket, to receive a ready signal on", + ); } if (lastStatus === 404) { throw new Error( @@ -139,7 +144,11 @@ export const test = backendFixture.extend< lastAgentCount = agents.length; agentProfileId = agents[0]?.profiles[0]?.id; if (agentProfileId) break; - await new Promise((r) => setTimeout(r, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the seeded-agent-profile poll above; seeding is asynchronous backend work read back over HTTP, with no page in this fixture", + ); } if (!agentProfileId) { throw new Error( diff --git a/apps/web/e2e/helpers/api-client.ts b/apps/web/e2e/helpers/api-client.ts index 20b0a2f1a7..33e4eb3e22 100644 --- a/apps/web/e2e/helpers/api-client.ts +++ b/apps/web/e2e/helpers/api-client.ts @@ -39,6 +39,7 @@ import type { SSHTestResult, } from "../../lib/types/http-ssh"; import { loadInterimSettingsInterlockToken } from "./interim-settings-interlock"; +import { dwell } from "./causal-waits"; // --- GitHub Mock Types --- @@ -2398,7 +2399,11 @@ export class ApiClient { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (await this.integrationReportsHealthy(integration, workspaceId)) return; - await new Promise((resolve) => setTimeout(resolve, 100)); + await dwell( + 100, + "poll-interval", + "sampling interval for the auth-health loop above; this client has no Page, and the 90s health poller updates the record without publishing anything it can subscribe to", + ); } throw new Error(`${integration} config never reported lastOk: true within ${timeoutMs}ms`); } diff --git a/apps/web/e2e/helpers/dockview-persistence.ts b/apps/web/e2e/helpers/dockview-persistence.ts new file mode 100644 index 0000000000..b441016b08 --- /dev/null +++ b/apps/web/e2e/helpers/dockview-persistence.ts @@ -0,0 +1,94 @@ +// Waiting on the dockview layout having been *saved*. +// +// Dockview persists the layout on a ~300ms debounce, to `sessionStorage` under +// `kandev.dockview.env-layout-v3.`. The debounce publishes no event, so a +// spec that reloads to prove the layout survived has nothing to wait for and +// historically slept past the debounce instead — which fails the wrong way +// round: on a loaded shard the save is late, the reload takes the old JSON, and +// the test fails claiming the layout was not preserved. +// +// The debounce leaves an artifact even though it fires no event, so wait on the +// artifact. `snapshotPersistedLayouts` before the action that changes the +// layout, `waitForPersistedLayoutChange` after it, and the reload then reads a +// save that has demonstrably happened. +// +// Every key is read rather than one session's, so callers do not need an +// environment id: a spec drives one task at a time, and any write is the write +// being waited for. +import { expect, type Page } from "@playwright/test"; + +/** + * Copy of `DOCKVIEW_ENV_LAYOUT_PREFIX` in `apps/web/lib/local-storage.ts`, which + * is the authoritative definition — bump both together. + * + * Importing it instead would be better, and does not work: `local-storage.ts` + * pulls in `lib/api/domains/attachment-api.ts` -> `lib/config.ts`, which uses + * `import.meta`, and Playwright's CJS transform rejects that at load time + * ("Cannot use 'import.meta' outside a module") before any test is collected. + * Only type-only imports reach app source from here for that reason. + * + * A silent drift is what the copy risks, so it does not stay silent: + * {@link waitForPersistedLayoutChange} reports the live `sessionStorage` keys on + * timeout, which distinguishes a stale prefix here from the layout genuinely + * never being persisted. + */ +const LAYOUT_KEY_PREFIX = "kandev.dockview.env-layout-v3."; + +/** Serialise every persisted dockview layout currently in `sessionStorage`. */ +export async function snapshotPersistedLayouts(page: Page): Promise { + return page.evaluate((prefix) => { + const entries: string[] = []; + for (let i = 0; i < window.sessionStorage.length; i++) { + const key = window.sessionStorage.key(i); + if (!key?.startsWith(prefix)) continue; + entries.push(`${key}=${window.sessionStorage.getItem(key) ?? ""}`); + } + return entries.sort().join("\n"); + }, LAYOUT_KEY_PREFIX); +} + +/** + * Wait until the persisted layout differs from `previous`, i.e. the debounced + * save has actually run. + * + * Fails if no save lands, which is the point: a layout change that never + * reaches storage is exactly the bug the reload in these specs is checking for, + * and a sleep would have carried on into a misleading assertion instead. + * + * The two ways that failure can happen are indistinguishable from the snapshot + * alone — the app stopped persisting, or {@link LAYOUT_KEY_PREFIX} drifted from + * the production constant and this polled a key nothing writes — so the timeout + * names the keys that were actually present instead of leaving the reader to + * assume the first. + */ +export async function waitForPersistedLayoutChange( + page: Page, + previous: string, + timeout = 10_000, +): Promise { + try { + await expect + .poll(() => snapshotPersistedLayouts(page), { + timeout, + message: "dockview never persisted the layout change", + }) + .not.toBe(previous); + } catch (error) { + const keys = await page.evaluate(() => { + const found: string[] = []; + for (let i = 0; i < window.sessionStorage.length; i++) { + const key = window.sessionStorage.key(i); + if (key) found.push(key); + } + return found.sort(); + }); + const matched = keys.filter((key) => key.startsWith(LAYOUT_KEY_PREFIX)); + const diagnosis = + matched.length > 0 + ? `${matched.length} key(s) matched "${LAYOUT_KEY_PREFIX}", so the prefix is live and the layout genuinely did not change.` + : `no key matched "${LAYOUT_KEY_PREFIX}" — this helper's copy of the prefix has probably drifted from DOCKVIEW_ENV_LAYOUT_PREFIX in apps/web/lib/local-storage.ts.`; + throw new Error( + `${error instanceof Error ? error.message : String(error)}\n\n${diagnosis}\nsessionStorage keys present: ${keys.join(", ") || "(none)"}`, + ); + } +} diff --git a/apps/web/e2e/helpers/dockview-resize.ts b/apps/web/e2e/helpers/dockview-resize.ts index 9ab8c4731c..66f7f519d3 100644 --- a/apps/web/e2e/helpers/dockview-resize.ts +++ b/apps/web/e2e/helpers/dockview-resize.ts @@ -3,10 +3,7 @@ import { computeRightMaxPx, computeSidebarMaxPx } from "../../lib/state/layout-m import type { SeedData } from "../fixtures/test-base"; import type { ApiClient } from "../helpers/api-client"; import { SessionPage } from "../pages/session-page"; - -/** Bounding-box info Playwright returns. Re-declared to avoid pulling the - * full Locator type just for one shape. */ -type Box = { x: number; y: number; width: number; height: number }; +import { dwell } from "./causal-waits"; export const WIDE_VIEWPORT = { width: 1600, height: 900 }; @@ -65,43 +62,6 @@ export async function getDockviewGroupWidthById(page: Page, groupId: string): Pr }, groupId); } -async function sashBoxAt(page: Page, index: number): Promise { - const sashes = page.locator(".dv-sash"); - const count = await sashes.count(); - if (count === 0) throw new Error("no .dv-sash elements found"); - if (index >= count) { - throw new Error(`sash index ${index} out of range (${count} sashes)`); - } - const box = await sashes.nth(index).boundingBox(); - if (!box) throw new Error(`sash ${index} has no bounding box`); - return box; -} - -/** - * Drag a horizontal-direction sash (between two columns) by deltaX pixels. - * sashIndex is the dockview sash order (0 = left-most). Reserved for tests - * that exercise real pointer motion (double-click smoke tests, etc.); most - * resize tests should prefer {@link resizeColumnViaSplitview} for stability - * in headless CI. - */ -export async function dragHorizontalSash( - page: Page, - sashIndex: number, - deltaX: number, - steps = 20, -): Promise { - const box = await sashBoxAt(page, sashIndex); - const cx = box.x + box.width / 2; - const cy = box.y + box.height / 2; - await page.mouse.move(cx, cy); - await page.mouse.down(); - await page.mouse.move(cx + deltaX, cy, { steps }); - await page.mouse.up(); - // Give the debounced layout-save 350ms to fire so subsequent reload assertions - // see the new width. - await page.waitForTimeout(400); -} - /** * Programmatically resize a column via dockview's internal splitview API. * @@ -210,8 +170,17 @@ export async function resizeColumnViaSplitview( }, { col: column, target: targetWidth }, ); - // Allow the debounced layout persistence to fire. - await page.waitForTimeout(400); + // The evaluate above force-flushes via `__persistDockviewLayout__`, but the + // ordinary debounced write can still be in flight behind it. Not converted to + // an observation of the stored layout: a resize that clamps to the width it + // already had writes nothing, and the ~30 call sites include several that + // deliberately ask for a clamped width. + await dwell( + page, + 400, + "product-timer", + "the dockview layout persists on a ~350ms debounce in our own code and publishes nothing when it lands, so reload assertions downstream have to be spaced past it", + ); return result; } diff --git a/apps/web/e2e/helpers/office-routing.ts b/apps/web/e2e/helpers/office-routing.ts index daec1122c1..ecbbcb21e7 100644 --- a/apps/web/e2e/helpers/office-routing.ts +++ b/apps/web/e2e/helpers/office-routing.ts @@ -1,5 +1,6 @@ import type { ApiClient } from "./api-client"; import type { OfficeApiClient } from "./office-api-client"; +import { dwell } from "./causal-waits"; const PROFILE_WAIT_MS = 20_000; @@ -26,7 +27,11 @@ export async function balancedExecutionProfileRouting( }), ); if (profiles.some((profile) => profile === undefined)) { - await new Promise((resolve) => setTimeout(resolve, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the profile-creation retry above; no Page exists in this helper, and profile availability is read back over HTTP rather than pushed", + ); continue; } const routing = await officeApi.getRouting(workspaceId); @@ -52,7 +57,11 @@ export async function balancedExecutionProfileRouting( ), }; } - await new Promise((resolve) => setTimeout(resolve, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the routing-readiness loop above; no Page exists in this helper, and the routing config is read back over HTTP rather than pushed", + ); } throw new Error(`execution profiles did not become available for ${providerOrder.join(", ")}`); } diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index b4e05321fb..29c0aa4f2a 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -1,5 +1,6 @@ import { type Locator, type Page, expect } from "@playwright/test"; import { FileTreePage } from "./file-tree-page"; +import { dwell } from "../helpers/causal-waits"; function escapeRegExp(value: string): string { return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); @@ -344,7 +345,12 @@ export class SessionPage { if ((await this.readXtermBuffer("passthrough-terminal")).includes(text)) { throw new Error(`Expected passthrough terminal NOT to contain "${text}", but it was found`); } - await this.page.waitForTimeout(200); + await dwell( + this.page, + 200, + "poll-interval", + "sampling interval for the stability window above; the assertion is that the text never appears, so the loop keeps re-reading the buffer across real elapsed time", + ); } } @@ -679,7 +685,12 @@ export class SessionPage { } catch { // Menu was likely detached by a re-render — dismiss and retry await this.page.keyboard.press("Escape"); - await this.page.waitForTimeout(500); + await dwell( + this.page, + 500, + "unverified", + "spacing before the next attempt in this menu-retry loop; the menu was detached mid-render and nothing was identified that signals it is safe to re-open", + ); } } // Final attempt without catch diff --git a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts index 1bd109cfd8..18bcb7724d 100644 --- a/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts +++ b/apps/web/e2e/tests/auth/auth-lifecycle.spec.ts @@ -1,5 +1,6 @@ import { expect } from "@playwright/test"; import { backendFixture as test } from "../../fixtures/backend"; +import { dwell } from "../../helpers/causal-waits"; import { acceptInvite, createInviteToken, @@ -201,7 +202,12 @@ test.describe.serial("opt-in authentication", () => { ); expect(renamed.ok()).toBeTruthy(); - await memberPage.waitForTimeout(1500); + await dwell( + memberPage, + 1500, + "negative-assertion", + "asserts the member's socket never receives the admin's workspace traffic; a frame that must not arrive publishes nothing, so the check needs real time on the wire before sampling", + ); const frames = await memberPage.evaluate( () => (window as unknown as { __wsFrames: string[] }).__wsFrames, ); diff --git a/apps/web/e2e/tests/auth/auth-screenshots.spec.ts b/apps/web/e2e/tests/auth/auth-screenshots.spec.ts index df2a301c1c..c84f47a1b2 100644 --- a/apps/web/e2e/tests/auth/auth-screenshots.spec.ts +++ b/apps/web/e2e/tests/auth/auth-screenshots.spec.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import path from "node:path"; import { backendFixture as test } from "../../fixtures/backend"; import { createInviteToken, login } from "../../helpers/auth"; +import { dwell } from "../../helpers/causal-waits"; /** * Not an assertion suite — captures screenshots of every auth surface into @@ -46,7 +47,12 @@ test.describe.serial("auth screenshots", () => { await page.getByTestId("setup-submit").click(); await page.waitForURL("**/"); await expect(page.getByTestId("setup-email")).toHaveCount(0, { timeout: 15_000 }); - await page.waitForTimeout(1500); + await dwell( + page, + 1500, + "unverified", + "settling the first post-setup render before capturing it; this suite exists to produce reviewable screenshots rather than assertions, and no specific timer was identified behind the number", + ); await shot(page, "02-app-after-setup"); await ctx.close(); }); @@ -187,18 +193,30 @@ test.describe.serial("auth screenshots", () => { await login(ctx, backend.baseUrl, MEMBER); const page = await ctx.newPage(); await page.goto("/"); - await page.waitForTimeout(2000); + await dwell( + page, + 2000, + "unverified", + "settling the member's first app render before capturing it; capture quality, not synchronisation, and no specific timer was identified behind the number", + ); await shot(page, "11-member-app"); // Dismiss the first-run onboarding wizard (it overlays the sidebar), then // hover the current-user chip to verify the border (not accent-fill) hover. const skip = page.getByRole("button", { name: /skip/i }); if (await skip.isVisible().catch(() => false)) { await skip.click(); - await page.waitForTimeout(400); + // The wizard overlays the sidebar, so the capture below is only right + // once it is gone. Assert that rather than budgeting for it. + await expect(skip).toBeHidden({ timeout: 5_000 }); } const chip = page.getByTestId("current-user-chip"); await chip.hover(); - await page.waitForTimeout(300); + await dwell( + page, + 300, + "product-timer", + "the chip's hover treatment is a CSS transition in our own styles; it publishes nothing on completion, and capturing mid-transition is what this shot exists to avoid", + ); await chip.screenshot({ path: path.join(SHOT_DIR, "12-user-chip-hover.png") }); await ctx.close(); }); diff --git a/apps/web/e2e/tests/git/diff-expansion.spec.ts b/apps/web/e2e/tests/git/diff-expansion.spec.ts index 1c410ab07e..342553a432 100644 --- a/apps/web/e2e/tests/git/diff-expansion.spec.ts +++ b/apps/web/e2e/tests/git/diff-expansion.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import type { SeedData } from "../../fixtures/test-base"; @@ -125,7 +126,12 @@ async function hoverUntilGutterSlotAppears(testPage: Page) { }; }); if (geometry) return geometry; - await testPage.waitForTimeout(50); + await dwell( + testPage, + 50, + "poll-interval", + "sampling interval for the hover-geometry loop above; the utility slot is rendered by a hover handler that publishes nothing, so the loop re-reads the DOM rather than awaiting an event", + ); } } diff --git a/apps/web/e2e/tests/git/diff-update.spec.ts b/apps/web/e2e/tests/git/diff-update.spec.ts index 2d2a23a8c7..cc8fecf605 100644 --- a/apps/web/e2e/tests/git/diff-update.spec.ts +++ b/apps/web/e2e/tests/git/diff-update.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; import { fileDiffTab, getDiffsContainer, @@ -425,8 +426,12 @@ test.describe("Untracked file diff update", () => { session.chat.getByText("untracked-file-modify complete", { exact: false }), ).toBeVisible({ timeout: 45_000 }); - // Wait for git polling to detect the file change (polling interval is ~1-2s) - await testPage.waitForTimeout(3_000); + await dwell( + testPage, + 3_000, + "poll-interval", + "git status is discovered by a ~1-2s backend poll rather than pushed, so the file change becomes visible on the poll's schedule and there is no event marking the sweep that will find it", + ); // Switch back to Changes tab and click on the diff file again await openChangesTab(testPage); diff --git a/apps/web/e2e/tests/git/git-changes-panel.spec.ts b/apps/web/e2e/tests/git/git-changes-panel.spec.ts index df95cf87c6..11a04e8ae2 100644 --- a/apps/web/e2e/tests/git/git-changes-panel.spec.ts +++ b/apps/web/e2e/tests/git/git-changes-panel.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; import type { ApiClient } from "../../helpers/api-client"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; @@ -869,8 +870,12 @@ test.describe("Git Changes Panel", () => { await session.clickTab("Changes"); await expect(session.changes).toBeVisible({ timeout: 10_000 }); - // Wait for initial load - await testPage.waitForTimeout(2_000); + await dwell( + testPage, + 2_000, + "unverified", + "pre-existing spacing before the external-git edits below; the panel is already asserted visible above and no timer was identified behind this, so it is labelled as debt rather than given a cause it does not have", + ); // Set up git helper const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); @@ -1273,10 +1278,15 @@ test.describe("Git Changes Panel", () => { const session = await openTaskSession(testPage, "Git Cumulative Test"); - // Wait for the session to fully initialize including base commit capture. - // The captureBaseCommit runs async after agent launch, we need it to complete - // before making commits so the cumulative diff works correctly. - await testPage.waitForTimeout(3_000); + // captureBaseCommit runs asynchronously after agent launch and has to finish + // before the commits below, or the cumulative diff is computed against the + // wrong base. + await dwell( + testPage, + 3_000, + "product-timer", + "captureBaseCommit runs async after agent launch and publishes nothing when it completes, so the commits below have to be spaced past it", + ); // Set up git helper const repoDir = path.join(backend.tmpDir, "repos", "e2e-repo"); @@ -1311,17 +1321,48 @@ test.describe("Git Changes Panel", () => { await expect(session.changes.getByText("Add first line")).toBeVisible({ timeout: 10_000 }); await expect(session.changes.getByText("Add second line")).toBeVisible({ timeout: 10_000 }); + // The poll below proves a "diff-viewer" panel exists after the click, which + // only implicates the click if none existed before it. Nothing in this test + // opens one earlier, so this reads as already-true today — it is here so + // that stops being an unstated assumption if the surrounding test ever + // reuses a session or restores a saved layout. + const diffViewerOpen = () => + testPage.evaluate(() => { + type Api = { getPanel: (id: string) => unknown }; + return Boolean( + (window as unknown as { __dockviewApi__?: Api }).__dockviewApi__?.getPanel("diff-viewer"), + ); + }); + expect(await diffViewerOpen(), "no cumulative diff panel before clicking Diff").toBe(false); + // Click the "Diff" button in the header to open the cumulative diff view await session.changes.getByRole("button", { name: "Diff" }).click(); - // Wait for diff viewer to load - await testPage.waitForTimeout(2_000); + // Assert what the click actually opens. Without this the checks below are + // vacuous: the two commit texts were already visible before the click and + // never go away, and "No changes" is absent from a page where the diff + // never opened at all, so all three passed whether or not the button did + // anything. The button calls `addDiffViewerPanel()`, which mounts a + // dockview panel with id "diff-viewer" -- it is not the Review dialog, as a + // first attempt at this assertion assumed and a run disproved. + await expect + .poll(diffViewerOpen, { + timeout: 15_000, + message: "the Diff button never opened the cumulative diff panel", + }) + .toBe(true); - // Verify commits are visible (this proves git changes are tracked) await expect(session.changes.getByText("Add first line")).toBeVisible({ timeout: 5_000 }); await expect(session.changes.getByText("Add second line")).toBeVisible({ timeout: 5_000 }); - // The cumulative diff should NOT show "No changes" + // The cumulative diff should NOT show "No changes". A negative has no event + // to wait on, so it needs the render window to elapse before sampling. + await dwell( + testPage, + 2_000, + "negative-assertion", + 'asserts the cumulative diff never renders its empty state; "No changes" not appearing publishes nothing, so the check has to outlast the dialog\'s own load', + ); await expect(testPage.locator("text=No changes")).not.toBeVisible({ timeout: 5_000 }); }); diff --git a/apps/web/e2e/tests/gitlab/mobile-mr-automation-options.spec.ts b/apps/web/e2e/tests/gitlab/mobile-mr-automation-options.spec.ts index 76da8f55a8..1a8f3e5b1f 100644 --- a/apps/web/e2e/tests/gitlab/mobile-mr-automation-options.spec.ts +++ b/apps/web/e2e/tests/gitlab/mobile-mr-automation-options.spec.ts @@ -5,6 +5,7 @@ import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-asserti import type { ApiClient } from "../../helpers/api-client"; import type { SeedData } from "../../fixtures/test-base"; import type { Locator } from "@playwright/test"; +import { dwell } from "../../helpers/causal-waits"; const MR_IID = 211; @@ -19,7 +20,12 @@ async function expectTouchTarget(locator: Locator, label: string) { // A boundingBox() measured mid-animation reports the pre-settle scaled-down // size, not the final CSS size — wait past the animation before measuring. async function waitForDropdownSettled(page: import("@playwright/test").Page) { - await page.waitForTimeout(200); + await dwell( + page, + 200, + "library-timer", + "@kandev/ui's DropdownMenuContent runs a 100ms zoom-in entrance animation that publishes nothing on completion, and a boundingBox read mid-animation reports the pre-settle scaled size", + ); } async function seedTaskWithLinkedMR(apiClient: ApiClient, seedData: SeedData, title: string) { diff --git a/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts b/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts index c968d4a9e7..9bf5c330c0 100644 --- a/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts +++ b/apps/web/e2e/tests/integrations/github-workspace-settings.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import { stubGitHubRateLimits } from "./github-rate-limit-fixture"; +import { dwell } from "../../helpers/causal-waits"; type ReviewWatchesResponse = { watches: Array<{ id: string; enabled: boolean }>; @@ -210,7 +211,12 @@ test.describe("GitHub workspace settings", () => { expect(executorBox).not.toBeNull(); expect(executorBox!.y - (managedBox!.y + managedBox!.height)).toBeLessThanOrEqual(9); await dialog.getByRole("radio", { name: "Inherit executor Git credentials" }).click(); - await testPage.waitForTimeout(300); + await dwell( + testPage, + 300, + "unverified", + "settling the radio selection's render before the capture below; no timer was identified behind the number, and the capture itself is a no-op unless CAPTURE_PR_ASSETS is set", + ); await prCapture.screenshot("desktop-task-git-access-dialog", { caption: "Task Git access is configured alongside the workspace connection", }); diff --git a/apps/web/e2e/tests/lsp/mobile-lsp-file-intelligence.spec.ts b/apps/web/e2e/tests/lsp/mobile-lsp-file-intelligence.spec.ts index edae275bab..11f797f997 100644 --- a/apps/web/e2e/tests/lsp/mobile-lsp-file-intelligence.spec.ts +++ b/apps/web/e2e/tests/lsp/mobile-lsp-file-intelligence.spec.ts @@ -12,6 +12,7 @@ import { readFakeLspEvents, releaseFakeLspInitialization, } from "./lsp-e2e-helpers"; +import { dwell } from "../../helpers/causal-waits"; test.describe("Mobile LSP boundaries", () => { test.describe.configure({ timeout: 90_000 }); @@ -56,7 +57,12 @@ test.describe("Mobile LSP boundaries", () => { await expect(viewer.locator(".cm-editor")).toBeVisible(); await expect(viewer.getByTestId("lsp-status-button")).toHaveCount(0); await expect(testPage.getByTestId("app-status-lsp")).toHaveCount(0); - await testPage.waitForTimeout(1_000); + await dwell( + testPage, + 1_000, + "negative-assertion", + "asserts that no LSP socket is ever opened on mobile; a connection that must not happen publishes nothing, so the check needs the window in which it would have opened to elapse", + ); expect(lspSockets).toEqual([]); expect(readFakeLspEvents(backend)).toEqual([]); } finally { diff --git a/apps/web/e2e/tests/office/agent-run-detail.spec.ts b/apps/web/e2e/tests/office/agent-run-detail.spec.ts index cb8f7800ea..774dc0c4cf 100644 --- a/apps/web/e2e/tests/office/agent-run-detail.spec.ts +++ b/apps/web/e2e/tests/office/agent-run-detail.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/office-fixture"; +import { injectLatency } from "../../helpers/causal-waits"; /** * E2E coverage for the per-agent paginated runs list and the run @@ -102,12 +103,10 @@ test.describe("Office agent run detail", () => { await route.continue(); return; } - // deliberate-sleep(latency-injection): NOT a wait. This delay is injected - // into the mocked paginated-runs response so the test can observe the - // pending/loading UI; the delay is the stimulus under test, and removing - // or shortening it destroys the scenario. Proposed as an eighth category - // because none of the existing seven describe "this is not a wait". - await new Promise((resolve) => setTimeout(resolve, 800)); + await injectLatency( + 800, + "slows the mocked paginated-runs response so the pending/loading UI stays observable; the delay is the stimulus under test, not a wait", + ); await route.continue(); }); diff --git a/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts b/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts index bf1eda8c97..f6dcca9d7d 100644 --- a/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts +++ b/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts @@ -1,6 +1,7 @@ import { type Page } from "@playwright/test"; import { test as base, expect } from "../../fixtures/test-base"; import { OfficeApiClient } from "../../helpers/office-api-client"; +import { dwell } from "../../helpers/causal-waits"; /** * Regression test: onboarding with a task title must launch the agent @@ -90,11 +91,11 @@ test.describe("Onboarding task launch", () => { return; } - // deliberate-sleep(poll-interval): sampling interval for the loop above, - // which asserts an invariant on EVERY iteration (the session must never - // reach FAILED/CANCELLED before launch). Converting to expect.poll would - // drop that per-iteration guard, so the loop keeps sampling instead. - await new Promise((r) => setTimeout(r, 1_000)); + await dwell( + 1_000, + "poll-interval", + "sampling interval for the loop above, which asserts an invariant on every iteration (the session must never reach FAILED or CANCELLED before launch); expect.poll would drop that per-iteration guard", + ); } expect( diff --git a/apps/web/e2e/tests/office/realtime-dashboard.spec.ts b/apps/web/e2e/tests/office/realtime-dashboard.spec.ts index 530a276310..c9140a156a 100644 --- a/apps/web/e2e/tests/office/realtime-dashboard.spec.ts +++ b/apps/web/e2e/tests/office/realtime-dashboard.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/office-fixture"; +import { dwell } from "../../helpers/causal-waits"; test.describe("Real-time dashboard updates", () => { test("dashboard metrics update after task creation", async ({ @@ -77,11 +78,12 @@ test.describe("Real-time dashboard updates", () => { workflow_id: otherWf.id, }); - // deliberate-sleep(negative-assertion): the assertion below is that the - // cross-workspace event triggers NO dashboard refetch. There is no event - // for a fetch that must never happen, so the only way to give a regression - // room to occur is to wait past the window in which it would have fired. - await testPage.waitForTimeout(3000); + await dwell( + testPage, + 3000, + "negative-assertion", + "the assertion below is that a cross-workspace event triggers no dashboard refetch; a fetch that must never happen has no event, so a regression needs the window in which it would have fired to elapse", + ); // No additional dashboard fetches should have occurred after the event. const newFetches = fetchTimes.length - baselineCount; diff --git a/apps/web/e2e/tests/plugins/plugin-task-panel.spec.ts b/apps/web/e2e/tests/plugins/plugin-task-panel.spec.ts index 4550ee5cf2..7817d073f2 100644 --- a/apps/web/e2e/tests/plugins/plugin-task-panel.spec.ts +++ b/apps/web/e2e/tests/plugins/plugin-task-panel.spec.ts @@ -16,6 +16,7 @@ import { installFixturePlugin, PLUGIN_ID } from "../../helpers/plugin-fixture"; import { SessionPage } from "../../pages/session-page"; import { KanbanPage } from "../../pages/kanban-page"; import type { ApiClient } from "../../helpers/api-client"; +import { dwell } from "../../helpers/causal-waits"; const PANEL_ID = "notes"; @@ -311,7 +312,12 @@ test.describe("Plugins — task panel / kanban Edit submenu / card indicator", ( // Now let the stale first read land — it must not revert the panel. releaseFirst(); - await testPage.waitForTimeout(500); + await dwell( + testPage, + 500, + "negative-assertion", + "asserts the stale first read never reverts the panel; a value that must not be written renders nothing, so the check needs time for the late response to land and do damage", + ); await expect(notesEditor).toHaveValue("second note"); }); diff --git a/apps/web/e2e/tests/plugins/plugins.spec.ts b/apps/web/e2e/tests/plugins/plugins.spec.ts index 2b5016aa7d..57955454e2 100644 --- a/apps/web/e2e/tests/plugins/plugins.spec.ts +++ b/apps/web/e2e/tests/plugins/plugins.spec.ts @@ -49,6 +49,7 @@ import { expect, test } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import { holdPluginInstallResponse } from "../../helpers/plugin-install"; +import { dwell } from "../../helpers/causal-waits"; const PLUGIN_ID = "kandev-plugin-e2e"; const NAV_ITEM_ID = "e2e-hello"; @@ -578,7 +579,12 @@ test.describe("Plugins — gRPC plugin install/load/live-update/uninstall", () = // every .error, so a polling plugin would file an Error-level backend log // entry per cycle. The report is scheduled in a microtask and sent async, // so give it a real chance to fire before asserting it never did. - await testPage.waitForTimeout(500); + await dwell( + testPage, + 500, + "negative-assertion", + "the report is scheduled in a microtask and sent async, and the assertion is that it never fires; a request that must not happen has no event, so it needs a real chance to arrive", + ); expect(reportRequests).toEqual([]); // Attribution goes to the console instead, matching every other plugin diff --git a/apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts b/apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts index 5368133cc0..2c7ab4be3a 100644 --- a/apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts +++ b/apps/web/e2e/tests/pr/pr-topbar-popover.spec.ts @@ -3,6 +3,7 @@ import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import type { Locator, Page } from "@playwright/test"; +import { dwell } from "../../helpers/causal-waits"; const OWNER = "acme"; const REPO = "demo"; @@ -408,7 +409,12 @@ test.describe("PR top-bar CI popover", () => { // Past the 150ms close delay the popover must still be open and its buttons // clickable. - await testPage.waitForTimeout(600); + await dwell( + testPage, + 600, + "product-timer", + "outlasts our own 150ms hover-intent close delay; the assertion is that the popover is still there afterwards, and a close that must not happen publishes nothing to wait on", + ); await expect(popover).toBeVisible(); await expect(openButton).toBeVisible(); await expect(session.prWorkflowAddContextButton("Lint")).toBeEnabled(); @@ -464,7 +470,12 @@ test.describe("PR top-bar CI popover", () => { // Past the 150ms close delay the popover must still be open and interactive, // not merely mounted — parity with the topbar test. - await testPage.waitForTimeout(600); + await dwell( + testPage, + 600, + "product-timer", + "outlasts our own 150ms hover-intent close delay; the assertion is that the popover survives the gap crossing, and a close that must not happen publishes nothing to wait on", + ); await expect(popover).toBeVisible(); await expect(openButton).toBeVisible(); await expect(popover.getByTestId("pr-workflow-add-context").first()).toBeEnabled(); diff --git a/apps/web/e2e/tests/review/review-file-status.spec.ts b/apps/web/e2e/tests/review/review-file-status.spec.ts index 266f621ff1..acb4c5d04d 100644 --- a/apps/web/e2e/tests/review/review-file-status.spec.ts +++ b/apps/web/e2e/tests/review/review-file-status.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; import { SessionPage } from "../../pages/session-page"; import { REVIEW_SIDEBAR_LIMITS } from "../../../hooks/use-review-sidebar-resize"; @@ -182,7 +183,12 @@ test.describe("Review file status", () => { await expect .poll(() => reviewScroll.evaluate((element) => element.scrollTop)) .toBeGreaterThan(0); - await testPage.waitForTimeout(600); + await dwell( + testPage, + 600, + "negative-assertion", + "asserts that jumping to a file never marks it reviewed; the count staying at zero is the absence of an event, so a regression needs the auto-review window to elapse to have room to fire", + ); await expect(reviewProgress).toHaveText(`0 of ${totalFiles} files reviewed`); await prCapture.screenshot("review-ordered-safe-jump", { caption: "Review keeps tree and diff order aligned without auto-reviewing a file jump", diff --git a/apps/web/e2e/tests/review/submodule-review-helpers.ts b/apps/web/e2e/tests/review/submodule-review-helpers.ts index 0a78e7dcc6..1d7e974159 100644 --- a/apps/web/e2e/tests/review/submodule-review-helpers.ts +++ b/apps/web/e2e/tests/review/submodule-review-helpers.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import path from "node:path"; import { execFileSync } from "node:child_process"; import type { ApiClient } from "../../helpers/api-client"; +import { dwell } from "../../helpers/causal-waits"; import type { SeedData } from "../../fixtures/test-base"; import { makeGitEnv } from "../../helpers/git-helper"; @@ -64,7 +65,11 @@ async function waitForWorktreePath( const worktreePath = session?.worktree_path ?? session?.workspace_path ?? session?.worktrees?.[0]?.worktree_path; if (worktreePath) return worktreePath; - await new Promise((resolve) => setTimeout(resolve, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the loop above; no Page exists here, and the worktree appears in a session record the backend writes without publishing anything this helper can subscribe to", + ); } throw new Error(`Timed out waiting for the nested-review worktree for ${sessionId}`); } diff --git a/apps/web/e2e/tests/review/walkthrough.spec.ts b/apps/web/e2e/tests/review/walkthrough.spec.ts index 8ecaec2e39..48994997d4 100644 --- a/apps/web/e2e/tests/review/walkthrough.spec.ts +++ b/apps/web/e2e/tests/review/walkthrough.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import type { SeedData } from "../../fixtures/test-base"; @@ -419,7 +420,12 @@ test.describe("Code walkthrough", () => { const reviewProgress = reviewDialog.getByText(/^0 of \d+ files reviewed$/); await expect(reviewProgress).toBeVisible({ timeout: 15_000 }); const initialProgress = await reviewProgress.textContent(); - await testPage.waitForTimeout(600); + await dwell( + testPage, + 600, + "negative-assertion", + "asserts the walkthrough behind the dialog never scrolls the review or advances its progress; both checks are absences, so they need the window in which a stray scroll or auto-review would land to elapse first", + ); await expect(reviewDialog.getByTestId("review-diff-scroll")).toHaveJSProperty("scrollTop", 0); await expect(reviewProgress).toHaveText(initialProgress ?? ""); await expectWalkthroughBehindDialog(testPage, reviewDialog, [ diff --git a/apps/web/e2e/tests/search/task-workspace-content-search.spec.ts b/apps/web/e2e/tests/search/task-workspace-content-search.spec.ts index 1b221a386a..9c6c615698 100644 --- a/apps/web/e2e/tests/search/task-workspace-content-search.spec.ts +++ b/apps/web/e2e/tests/search/task-workspace-content-search.spec.ts @@ -6,6 +6,7 @@ import { expect, test } from "../../fixtures/test-base"; import { GitHelper, makeGitEnv } from "../../helpers/git-helper"; import { SessionPage } from "../../pages/session-page"; import { MODIFIER } from "./shared"; +import { dwell } from "../../helpers/causal-waits"; const SEARCH_TERM = "TaskWorkspaceNeedle"; const EXTRA_REPOSITORY_NAME = "content-search-extra"; @@ -59,7 +60,12 @@ async function waitForRepositoryGroups({ if (Date.now() >= deadline) break; await input.fill(""); - await page.waitForTimeout(50); + await dwell( + page, + 50, + "poll-interval", + "spacing between retype attempts in the retry loop above; the search input is debounced and the retry exists to re-trigger it, so there is no completion event to await here", + ); await input.fill(query); } diff --git a/apps/web/e2e/tests/session/long-prepare-panels.spec.ts b/apps/web/e2e/tests/session/long-prepare-panels.spec.ts index e5c987efa1..da1a107783 100644 --- a/apps/web/e2e/tests/session/long-prepare-panels.spec.ts +++ b/apps/web/e2e/tests/session/long-prepare-panels.spec.ts @@ -2,6 +2,7 @@ import fs from "node:fs"; import path from "node:path"; import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; +import { dwell } from "../../helpers/causal-waits"; /** * Regression: when workspace preparation takes longer than the file-tree @@ -58,14 +59,15 @@ test.describe("Long prepare (slow git fetch)", () => { await expect(fileTreeWaiting).toBeVisible({ timeout: 15_000 }); await expect(fileTreeManual).toHaveCount(0); - // deliberate-sleep(product-timer): waits past the pre-fix retry budget - // (1+2+5+10 = 18s). The regression is the file tree transitioning to the - // "manual" (Load Files) state when that budget expires, so the budget's - // expiry is precisely the thing under test and nothing renders to signal - // it. On faster CI runners the 22s fetch may already have completed by - // the time this assertion runs, so do not require the waiting state to - // still be visible here. - await testPage.waitForTimeout(19_000); + // On faster CI runners the 22s fetch may already have completed by the + // time the assertion below runs, so it does not require the waiting state + // to still be visible. + await dwell( + testPage, + 19_000, + "product-timer", + "outlasts our own 1+2+5+10s file-tree retry ladder; the regression under test is the tree falling back to its manual state when that budget expires, and the expiry renders nothing to signal it", + ); await expect(fileTreeManual).toHaveCount(0); // Fetch eventually returns, worktree creation proceeds, agentctl diff --git a/apps/web/e2e/tests/session/session-flicker.spec.ts b/apps/web/e2e/tests/session/session-flicker.spec.ts index 5ec7b5d1fd..a893795958 100644 --- a/apps/web/e2e/tests/session/session-flicker.spec.ts +++ b/apps/web/e2e/tests/session/session-flicker.spec.ts @@ -6,6 +6,7 @@ import type { ApiClient } from "../../helpers/api-client"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import { waitForStableActiveSession } from "../../helpers/session-store"; +import { dwell } from "../../helpers/causal-waits"; /** * Regression: opening / switching to a second agent session in a task that @@ -172,11 +173,12 @@ test.describe("Session flicker", () => { }); await session.sessionTabBySessionId(session2Id).click(); - // deliberate-sleep(negative-assertion): this is the observation window the - // test exists for. It asserts the ABSENCE of sustained oscillation after a - // deliberate switch, so it must sample real elapsed time rather than wait - // for any single event. - await testPage.waitForTimeout(2_500); + await dwell( + testPage, + 2_500, + "negative-assertion", + "the observation window this test exists for: it asserts the absence of sustained oscillation after a deliberate switch, so it has to sample real elapsed time rather than await any single event", + ); const activeLog = await testPage.evaluate( () => (window as unknown as FlickerWindow).__activeLog__ ?? [], @@ -253,11 +255,12 @@ test.describe("Session flicker", () => { await session.sessionTabBySessionId(session1Id).click(); await waitForStableActiveSession(testPage, session1Id); await session.sessionTabBySessionId(session2Id).click(); - // deliberate-sleep(negative-assertion): final observation window. The - // assertions below are that neither tab was stripped and no session panel - // was torn down, which only means something after giving the regression - // real time to occur. - await testPage.waitForTimeout(1_500); + await dwell( + testPage, + 1_500, + "negative-assertion", + "final observation window; the assertions below are that neither tab was stripped and no session panel was torn down, which only mean something after the regression has had real time to occur", + ); // Both tabs must remain present throughout, and the active session's chat // must still be visible (dockview keeps inactive panels mounted-but-hidden, diff --git a/apps/web/e2e/tests/session/session-resume-keeps-review-state.spec.ts b/apps/web/e2e/tests/session/session-resume-keeps-review-state.spec.ts index c64f9d8639..31e341d6cb 100644 --- a/apps/web/e2e/tests/session/session-resume-keeps-review-state.spec.ts +++ b/apps/web/e2e/tests/session/session-resume-keeps-review-state.spec.ts @@ -1,6 +1,7 @@ import { test, expect } from "../../fixtures/test-base"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; +import { dwell } from "../../helpers/causal-waits"; // Regression: when the backend restarts and auto-resumes a task whose // session was WAITING_FOR_INPUT, the task must NEVER transition into the @@ -72,11 +73,12 @@ test.describe("Session resume — task state stability", () => { 0, ); polls++; - // deliberate-sleep(poll-interval): sampling interval for the fixed probe - // window above, not a settle. The assertion is that the task never - // flickers into Running, so the loop must keep sampling across the whole - // window; the polls >= 50 check below guards against it degenerating. - await testPage.waitForTimeout(100); + await dwell( + testPage, + 100, + "poll-interval", + "sampling interval for the fixed probe window above, not a settle; the assertion is that the task never flickers into Running, so the loop keeps sampling across the whole window and the polls >= 50 check guards against it degenerating", + ); } // Ensure we actually polled meaningfully across the window. expect(polls).toBeGreaterThanOrEqual(50); diff --git a/apps/web/e2e/tests/session/session-stream-budget.spec.ts b/apps/web/e2e/tests/session/session-stream-budget.spec.ts index 6079624463..682399fdbc 100644 --- a/apps/web/e2e/tests/session/session-stream-budget.spec.ts +++ b/apps/web/e2e/tests/session/session-stream-budget.spec.ts @@ -6,6 +6,7 @@ import { summarizeGatewayTraffic, type GatewayTrafficFrame, } from "../../helpers/ws-traffic"; +import { dwell } from "../../helpers/causal-waits"; const TASK_COUNT = 27; const SWITCH_COUNT = 5; @@ -116,12 +117,12 @@ test.describe("Session stream budget", () => { }); } - // deliberate-sleep(negative-assertion): a deliberate five-second - // observation window after the five task switches, measuring how much - // gateway traffic keeps arriving. This is a diagnostic total, not a - // compression-dependent correctness threshold, so the window must be real - // elapsed time rather than a wait for any particular frame. - await testPage.waitForTimeout(5_000); + await dwell( + testPage, + 5_000, + "negative-assertion", + "a deliberate observation window measuring how much gateway traffic keeps arriving after five task switches; it is a diagnostic total rather than a wait for any particular frame, so it has to be real elapsed time", + ); const summary = summarizeGatewayTraffic(capture.frames); await test.info().attach("gateway-traffic.json", { diff --git a/apps/web/e2e/tests/session/session-tab-management.spec.ts b/apps/web/e2e/tests/session/session-tab-management.spec.ts index 7bd237d834..c2f68245c6 100644 --- a/apps/web/e2e/tests/session/session-tab-management.spec.ts +++ b/apps/web/e2e/tests/session/session-tab-management.spec.ts @@ -6,6 +6,7 @@ import type { ApiClient } from "../../helpers/api-client"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import { attachGatewayTrafficCapture } from "../../helpers/ws-traffic"; +import { dwell } from "../../helpers/causal-waits"; /** Wire action the kanban WS handler consumes when a task moves step. */ const TASK_UPDATED_ACTION = "task.updated"; @@ -191,10 +192,12 @@ test.describe("Session tab management — close behavior", () => { await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible({ timeout: 15_000 }); // …and stays gone — useAutoSessionTab must not recreate it. - // deliberate-sleep(negative-assertion): the regression is a tab being - // recreated after removal. There is no event for a recreation that must - // never happen, so the check needs real elapsed time to be meaningful. - await testPage.waitForTimeout(800); + await dwell( + testPage, + 800, + "negative-assertion", + "the regression is a tab being recreated after removal; a recreation that must never happen has no event, so the check needs real elapsed time to mean anything", + ); await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible(); await expect(session.sessionTabBySessionId(session2Id)).toBeVisible(); @@ -297,10 +300,12 @@ test.describe("Session tab management — close behavior", () => { // that the remaining session tab is present (and the deleted one didn't come // back), so gate on the surviving session tab instead. await expect(session.sessionTabBySessionId(session2Id)).toBeVisible({ timeout: 15_000 }); - // deliberate-sleep(negative-assertion): the deleted tab must not come back - // after a task round-trip. Nothing is rendered to wait for when the - // expected outcome is "no tab ever appears". - await testPage.waitForTimeout(800); + await dwell( + testPage, + 800, + "negative-assertion", + "the deleted tab must not come back after a task round-trip; nothing is rendered to wait for when the expected outcome is that no tab ever appears", + ); await expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible(); }); @@ -354,9 +359,12 @@ test.describe("Session tab management — close behavior", () => { await expect(session.sessionTabBySessionId(sessionB1Id)).toBeVisible({ timeout: 10_000 }); // …and neither of task A's session tabs should have followed us in. - // deliberate-sleep(negative-assertion): asserts tabs from another task - // never leak in, which has no arrival event to wait on. - await testPage.waitForTimeout(800); + await dwell( + testPage, + 800, + "negative-assertion", + "asserts that tabs from another task never leak in, which has no arrival event to wait on", + ); await expect(session.sessionTabBySessionId(sessionA1Id)).not.toBeVisible(); await expect(session.sessionTabBySessionId(sessionA2Id)).not.toBeVisible(); diff --git a/apps/web/e2e/tests/ssh/sessions-endpoint.spec.ts b/apps/web/e2e/tests/ssh/sessions-endpoint.spec.ts index df2bb69ae8..3aaecd631e 100644 --- a/apps/web/e2e/tests/ssh/sessions-endpoint.spec.ts +++ b/apps/web/e2e/tests/ssh/sessions-endpoint.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/ssh-test-base"; +import { dwell } from "../../helpers/causal-waits"; /** * HTTP contract for GET /api/v1/ssh/executors/:id/sessions. Backs the * SSHSessionsCard table. Filtered to ssh-runtime ExecutorRunning rows for @@ -144,7 +145,11 @@ test.describe("ssh sessions-endpoint contract", () => { const beforeRow = before.find((s) => s.task_id === task.id); expect(beforeRow).toBeDefined(); - await new Promise((resolve) => setTimeout(resolve, 2_000)); + await dwell( + 2_000, + "clock-separation", + "the assertion below is that uptime_seconds grew, so real time has to pass between the two reads; the thing being waited for is the clock, not an app event", + ); const after = await apiClient.listSSHSessions(seedData.sshExecutorId); const afterRow = after.find((s) => s.task_id === task.id); diff --git a/apps/web/e2e/tests/system/disk-usage.spec.ts b/apps/web/e2e/tests/system/disk-usage.spec.ts index 78842a639c..0c4ecaf385 100644 --- a/apps/web/e2e/tests/system/disk-usage.spec.ts +++ b/apps/web/e2e/tests/system/disk-usage.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; // Wait until the backend has a populated disk-usage cache (the initial walk // is async; the first call returns `{data: null, computing: true}` and kicks @@ -15,7 +16,11 @@ async function waitForDiskUsageCached(apiClient: { const body = (await res.json()) as { data: unknown; computing: boolean }; if (body.data) return; } - await new Promise((r) => setTimeout(r, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the disk-usage cache poll above; the cache is computed in the background and only observable by re-reading the endpoint", + ); } throw new Error("disk-usage cache never populated within 20s"); } diff --git a/apps/web/e2e/tests/system/restore-dialog.spec.ts b/apps/web/e2e/tests/system/restore-dialog.spec.ts index ffd9eedfa0..addf113686 100644 --- a/apps/web/e2e/tests/system/restore-dialog.spec.ts +++ b/apps/web/e2e/tests/system/restore-dialog.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { dwell } from "../../helpers/causal-waits"; async function listBackups(apiClient: { rawRequest: (m: string, p: string) => Promise }) { const res = await apiClient.rawRequest("GET", "/api/v1/system/backups"); @@ -48,7 +49,11 @@ test.describe("Restore snapshot dialog", () => { found = true; break; } - await new Promise((r) => setTimeout(r, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the backup-list poll above; snapshot creation is backend work read back over HTTP, with no page bound to this loop", + ); } expect(found).toBeTruthy(); diff --git a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts index b703b5120d..1be916b4ae 100644 --- a/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts +++ b/apps/web/e2e/tests/terminal/mobile-terminal-helpers.ts @@ -3,6 +3,7 @@ // shell-connect path, so the connect/readiness helpers live here to avoid // drift between the two files. import { type Page, expect } from "@playwright/test"; +import { dwell } from "../../helpers/causal-waits"; export async function tapTerminalTab(testPage: Page): Promise { await testPage.getByRole("button", { name: "Terminal" }).tap(); @@ -41,7 +42,20 @@ async function remountTerminalPanel(testPage: Page): Promise { const chatTab = testPage.getByRole("button", { name: "Chat" }); if (await chatTab.isVisible()) { await chatTab.tap(); - await testPage.waitForTimeout(250); + // The point of the detour through Chat is to unmount the terminal panel, so + // wait for it to be gone. Budgeting for it instead would let a lost tap + // fall through to `switchToTerminalPanel`, which finds the panel already + // visible and returns without remounting anything -- turning the retry that + // exists to rescue a dead shell WS into a no-op. + // + // Counted rather than asserted visible/hidden: `terminal-panel` is not + // guaranteed to be a single node (dockview mounts one per terminal panel, + // and the mobile layout mounts its own), so a state assertion on the bare + // locator would be a strict-mode violation the moment a second one exists, + // and would pass on a stale hidden one while a live panel remained mounted. + // Zero of them is what "unmounted" means here, and `toHaveCount` says that + // without having to resolve which instance is the active one. + await expect(testPage.getByTestId("terminal-panel")).toHaveCount(0, { timeout: 5_000 }); } await switchToTerminalPanel(testPage); } @@ -70,7 +84,12 @@ export async function waitForShellReady(testPage: Page, timeout = 45_000): Promi remounts += 1; nextRemountAt = Date.now() + 15_000; } - await testPage.waitForTimeout(1_000); + await dwell( + testPage, + 1_000, + "poll-interval", + "sampling interval for the loop above, which re-taps and remounts between reads; the shell WS connect publishes nothing this side of xterm's buffer filling, so there is no event to await instead", + ); } expect( (await readTerminalBuffer(testPage)).length, diff --git a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts index 3757d5ea5c..657fe403aa 100644 --- a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts +++ b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts @@ -4,6 +4,11 @@ import type { SeedData } from "../../fixtures/test-base"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import type { Page } from "@playwright/test"; +import { + snapshotPersistedLayouts, + waitForPersistedLayoutChange, +} from "../../helpers/dockview-persistence"; +import { dwell } from "../../helpers/causal-waits"; const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; @@ -143,10 +148,16 @@ test.describe("Terminals — dockview UI", () => { .getByTestId("terminal-tab-seq-2") .locator("..") .locator(".dv-default-tab-action"); + const layoutBeforeClose = await snapshotPersistedLayouts(testPage); await seq2Close.click(); await expect(testPage.getByTestId("terminal-tab-seq-2")).toHaveCount(0, { timeout: 5_000 }); - await testPage.waitForTimeout(800); + // The tab is gone from the DOM, but the reload below reads sessionStorage, + // and that write is on a ~300ms debounce with no event. Wait for the write + // rather than past it: if the close never reaches storage, that is the bug + // this test exists to catch, and it should fail here rather than as a + // confusing assertion after the reload. + await waitForPersistedLayoutChange(testPage, layoutBeforeClose); await testPage.reload(); await session.waitForLoad(); @@ -193,14 +204,16 @@ test.describe("Terminals — dockview UI", () => { await session.clickTab("Terminal"); await session.expectTerminalConnected(); + const layoutBeforeNewTerminal = await snapshotPersistedLayouts(testPage); await clickNewTerminalInPlusMenu(testPage, session); await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 10_000 }); await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); - // Layout-save is 300ms-debounced. Wait past the debounce so the - // saved JSON includes the user-created terminal panel before we - // reload — otherwise a fast test races the save. - await testPage.waitForTimeout(800); + // Layout-save is 300ms-debounced and publishes nothing, so the saved JSON + // has to be observed rather than waited out: the reload below reads it, and + // a slow save on a loaded shard would make this fail claiming the panel was + // not preserved. + await waitForPersistedLayoutChange(testPage, layoutBeforeNewTerminal); await testPage.reload(); // After the dockview "preserve restored active tab" change (commit @@ -291,14 +304,22 @@ test.describe("Terminals — dockview UI", () => { // Create a second terminal so we have a non-default row to click. await clickNewTerminalInPlusMenu(testPage, session); await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 10_000 }); - await testPage.waitForTimeout(800); - // Count terminal tab content elements before the focus click. + // Count terminal tab content elements before the focus click. Polling the + // count is the wait: it returns as soon as the second tab has rendered + // instead of after a fixed budget, and it fails if the tab never arrives + // rather than reporting a baseline of 1 that makes the comparison below + // meaningless. const terminalContent = testPage.locator(".dv-default-tab-content").filter({ hasText: /^Terminal$/, }); + await expect + .poll(() => terminalContent.count(), { + timeout: 10_000, + message: "two terminal tabs before clicking reopen", + }) + .toBeGreaterThanOrEqual(2); const before = await terminalContent.count(); - expect(before, "two terminal tabs before clicking reopen").toBeGreaterThanOrEqual(2); // Open the menu and click the seq=2 row — that terminal is already // open as a tab, so the click should focus rather than mint a new @@ -310,9 +331,15 @@ test.describe("Terminals — dockview UI", () => { await expect(seq2Row).toHaveCount(1, { timeout: 10_000 }); await seq2Row.click(); - // Tab count must NOT grow. The menu closes on its own; wait briefly - // for any spurious panel to land. - await testPage.waitForTimeout(500); + // Tab count must NOT grow. There is no event for a panel that must never + // be created, so the only way to give a regression room to happen is to let + // the window in which it would have landed elapse. + await dwell( + testPage, + 500, + "negative-assertion", + "asserts that clicking an already-open terminal focuses rather than minting a second panel; a panel that must not appear publishes nothing to wait on", + ); const after = await terminalContent.count(); expect( after, diff --git a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts index 10645ae72b..31057c28c1 100644 --- a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts @@ -1,6 +1,7 @@ import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; import { WorkflowSettingsPage } from "../../pages/workflow-settings-page"; +import { dwell } from "../../helpers/causal-waits"; async function createProfiles( apiClient: InstanceType, @@ -962,10 +963,11 @@ test.describe("Workflow agent profile switching", () => { expect(stableSessions.length, "no extra session should spawn within stability window").toBe( 2, ); - // deliberate-sleep(poll-interval): sampling interval for the stability - // window above. The assertion is that no extra session appears during - // it, so the loop must keep sampling across real elapsed time. - await new Promise((r) => setTimeout(r, 250)); + await dwell( + 250, + "poll-interval", + "sampling interval for the stability window above; the assertion is that no extra session appears during it, so the loop keeps sampling across real elapsed time", + ); } // Critical assertion: no extra profileA session was spawned after the diff --git a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts index 03b2a8a1bb..094f6ffcca 100644 --- a/apps/web/e2e/tests/workflow/workflow-settings.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-settings.spec.ts @@ -1,6 +1,7 @@ import type { Locator } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { WorkflowSettingsPage } from "../../pages/workflow-settings-page"; +import { dwell } from "../../helpers/causal-waits"; async function maxRingSpread(locator: Locator): Promise { const boxShadow = await locator.evaluate((element) => getComputedStyle(element).boxShadow); @@ -685,9 +686,12 @@ test.describe("Seed protection", () => { // `workflow.created` WS event arrives at the open settings page. await apiClient.e2eCreateHiddenWorkflow(seedData.workspaceId, hiddenName); - // Allow the WS event to propagate and the React effect in - // useWorkflowSettings a chance to (incorrectly) add a card. - await testPage.waitForTimeout(500); + await dwell( + testPage, + 500, + "negative-assertion", + "gives the workflow.created frame and the useWorkflowSettings effect a chance to incorrectly add a card; the assertion is that no card appears, which publishes nothing to wait on", + ); // No new card appeared and the hidden entry is not in the list. const allCards = testPage.locator('[data-testid^="workflow-card-"]'); diff --git a/apps/web/e2e/tests/workflow/workflow-start-step.spec.ts b/apps/web/e2e/tests/workflow/workflow-start-step.spec.ts index 0eb3c8b23d..bfa9a6bc6e 100644 --- a/apps/web/e2e/tests/workflow/workflow-start-step.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-start-step.spec.ts @@ -2,6 +2,7 @@ import { test, expect } from "../../fixtures/test-base"; import { useRegularMode } from "../../helpers/regular-mode"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; +import { dwell } from "../../helpers/causal-waits"; // Exercises the regular task-create dialog (New Task in the sidebar); run with office off. useRegularMode(); @@ -216,8 +217,12 @@ test.describe("Workflow start step placement", () => { }); // --- Toggle off plan mode --- - // Wait for the editor to be fully interactive before sending the shortcut - await testPage.waitForTimeout(1_000); + await dwell( + testPage, + 1_000, + "unverified", + "pre-existing spacing before the plan-mode shortcut; the editor's readiness was not tied to an identified timer or event, so this is labelled as debt rather than given a cause it does not have", + ); await session.togglePlanMode(); await expect(session.planPanel).not.toBeVisible({ timeout: 15_000 }); diff --git a/apps/web/lib/local-storage.ts b/apps/web/lib/local-storage.ts index 19a5b61d37..d576323221 100644 --- a/apps/web/lib/local-storage.ts +++ b/apps/web/lib/local-storage.ts @@ -285,6 +285,8 @@ export function setFilesPanelScrollPosition(sessionId: string, position: number) // Bumping the prefix invalidates legacy saves so every env opens at the // preset defaults once, then resumes per-env persistence. Bumped to v3 to // discard layouts captured with the now-removed dockview sidebar column. +// Mirrored (deliberately, see below) by LAYOUT_KEY_PREFIX in +// `apps/web/e2e/helpers/dockview-persistence.ts`; bump both together. const DOCKVIEW_ENV_LAYOUT_PREFIX = "kandev.dockview.env-layout-v3."; // A serialized Dockview layout is geometry, not evidence of an intentional