diff --git a/apps/web/e2e/helpers/settled-box.ts b/apps/web/e2e/helpers/settled-box.ts new file mode 100644 index 0000000000..dc78e14ca7 --- /dev/null +++ b/apps/web/e2e/helpers/settled-box.ts @@ -0,0 +1,44 @@ +import { expect, type Locator } from "@playwright/test"; + +type BoundingBox = { x: number; y: number; width: number; height: number }; + +/** + * Scroll `locator` into view and resolve its bounding box once it stops moving. + * + * `scrollIntoViewIfNeeded()` resolves before an animated or momentum scroll has + * finished, so reading `boundingBox()` straight afterwards can capture + * coordinates that are already stale by the time a pointer event is dispatched + * at them. Drag helpers that then aim at those coordinates miss their target + * intermittently. + * + * Poll until two consecutive samples agree rather than sleeping for a budget + * that is guessed rather than measured: this returns as soon as the element is + * actually still, and keeps waiting when the scroll is slow. + */ +export async function settledBoundingBox(locator: Locator, timeout = 5_000): Promise { + await locator.scrollIntoViewIfNeeded(); + + let previous: string | null = null; + let latest: BoundingBox | null = null; + let agreeingReads = 0; + + // Three consecutive agreeing reads, not two. With two, both samples can land + // before an animated scroll has begun moving the element at all, in which + // case they agree on the pre-scroll position and this returns exactly the + // stale coordinates it exists to avoid. + await expect + .poll( + async () => { + latest = await locator.boundingBox(); + const current = latest ? `${Math.round(latest.x)},${Math.round(latest.y)}` : null; + agreeingReads = current !== null && current === previous ? agreeingReads + 1 : 0; + previous = current; + return agreeingReads >= 2; + }, + { timeout, message: "element position did not settle" }, + ) + .toBe(true); + + if (!latest) throw new Error("element has no bounding box after settling"); + return latest; +} diff --git a/apps/web/e2e/helpers/type-while-busy.ts b/apps/web/e2e/helpers/type-while-busy.ts index 312c0c10bd..100545b404 100644 --- a/apps/web/e2e/helpers/type-while-busy.ts +++ b/apps/web/e2e/helpers/type-while-busy.ts @@ -1,4 +1,27 @@ -import { type Locator, type Page } from "@playwright/test"; +import { expect, type Locator, type Page } from "@playwright/test"; + +/** + * Wait until the composer has actually entered queue ("busy") mode. + * + * The `Agent is starting|running` status indicator renders straight off + * `session.state`, but the composer's queue affordance comes from a separate + * derivation (`deriveSessionInputMode`: state + foreground_activity + + * supports_steering). Asserting the status alone can therefore return before + * the toolbar has re-rendered into queue mode, which is why these call sites + * used to chase it with a fixed sleep. + * + * The cancel button is the rendered signal of that second derivation, so + * waiting on it gives the same guarantee reactively: it returns immediately + * when the composer is already busy and keeps waiting when the store is slow, + * instead of spending a fixed budget that is simultaneously too long and too + * short. + */ +export async function waitForComposerQueueMode( + scope: Page | Locator, + timeout = 15_000, +): Promise { + await expect(scope.getByTestId("cancel-agent-button")).toBeVisible({ timeout }); +} /** * Type text into the TipTap editor while the agent is busy. @@ -12,14 +35,22 @@ export async function typeWhileBusy(page: Page, editor: Locator, text: string): const box = await editor.boundingBox(); if (!box) throw new Error("Editor bounding box not found"); await page.mouse.click(box.x + 20, box.y + box.height / 2); + // deliberate-sleep(unverified): pre-existing spacing in this retry loop. + // Unlike the other retained sleeps these are not tied to an identified + // timer, so they are debt rather than intent: the likely reactive + // replacement is waiting for the editor to take focus. They are left alone + // here only because this helper is on the hot path of ~16 specs that were + // just verified against it, and changing it would invalidate that run. await page.waitForTimeout(200); await page.keyboard.type(text); + // deliberate-sleep(unverified): as above. await page.waitForTimeout(100); const content = await editor.textContent(); if (content?.includes(text)) return; // Text wasn't entered; select all and clear for retry await page.keyboard.press(`${modifier}+a`); await page.keyboard.press("Backspace"); + // deliberate-sleep(unverified): as above. await page.waitForTimeout(200); } throw new Error(`Failed to type "${text}" into editor after 3 attempts`); diff --git a/apps/web/e2e/tests/chat/agent-message-attribution.spec.ts b/apps/web/e2e/tests/chat/agent-message-attribution.spec.ts index 217b952462..42a7a76b56 100644 --- a/apps/web/e2e/tests/chat/agent-message-attribution.spec.ts +++ b/apps/web/e2e/tests/chat/agent-message-attribution.spec.ts @@ -44,21 +44,28 @@ async function waitForCrossTaskMessage( sessionId: string, timeoutMs = 30_000, ): Promise<{ id: string; content: string; metadata?: Record }> { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const { messages } = await apiClient.listSessionMessages(sessionId); - const match = messages.find( - (m) => - m.author_type === "user" && - m.metadata && - (m.metadata as Record).sender_task_id, - ); - if (match) return match; - await new Promise((r) => setTimeout(r, 250)); - } - throw new Error( - `No cross-task user message recorded on session ${sessionId} within ${timeoutMs}ms`, - ); + let match: + | Awaited>["messages"][number] + | undefined; + await expect + .poll( + async () => { + const { messages } = await apiClient.listSessionMessages(sessionId); + match = messages.find( + (m) => + m.author_type === "user" && + m.metadata && + (m.metadata as Record).sender_task_id, + ); + return Boolean(match); + }, + { + timeout: timeoutMs, + message: `No cross-task user message recorded on session ${sessionId}`, + }, + ) + .toBe(true); + return match!; } /** Create a target task that immediately enters WAITING_FOR_INPUT — the @@ -309,12 +316,15 @@ test.describe("Cross-task agent message attribution", () => { // Wait for the agent's tool call + final message to complete, then assert // the receiving session never gained a cross-task user message. - const deadline = Date.now() + 30_000; - while (Date.now() < deadline) { - const { messages } = await apiClient.listSessionMessages(selfTask.session_id); - if (messages.some((m) => m.content.includes("attempted"))) break; - await new Promise((r) => setTimeout(r, 250)); - } + await expect + .poll( + async () => { + const { messages } = await apiClient.listSessionMessages(selfTask.session_id!); + return messages.some((m) => m.content.includes("attempted")); + }, + { timeout: 30_000, message: "agent turn did not record its attempt" }, + ) + .toBe(true); const { messages } = await apiClient.listSessionMessages(selfTask.session_id); const senderRow = messages.find( (m) => @@ -350,16 +360,19 @@ test.describe("Cross-task agent message attribution", () => { // Wait until the agent has run its turn — at least one agent-authored // message must appear (text reply, tool call, or both). Then assert the // backend rejection text shows up somewhere in the recorded payload. - const deadline = Date.now() + 30_000; - while (Date.now() < deadline) { - const { messages } = await apiClient.listSessionMessages(sender.session_id); - // Wait for the agent's "done" text reply (emitted *after* the failed - // tool call), so we know the tool result has been persisted by the time - // we look at the metadata. Filtering on author_type avoids matching the - // user-authored description that contains the script source. - if (messages.some((m) => m.author_type === "agent" && m.content.includes("done"))) break; - await new Promise((r) => setTimeout(r, 250)); - } + // Wait for the agent's "done" text reply (emitted *after* the failed tool + // call), so we know the tool result has been persisted by the time we look + // at the metadata. Filtering on author_type avoids matching the + // user-authored description that contains the script source. + await expect + .poll( + async () => { + const { messages } = await apiClient.listSessionMessages(sender.session_id!); + return messages.some((m) => m.author_type === "agent" && m.content.includes("done")); + }, + { timeout: 30_000, message: "agent turn did not emit its done reply" }, + ) + .toBe(true); const { messages } = await apiClient.listSessionMessages(sender.session_id); // Search the entire payload for the rejection text — the mock-agent stuffs // it into the tool's output map, but the exact metadata shape varies by diff --git a/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts b/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts index b4ce47850e..96f77692b6 100644 --- a/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts +++ b/apps/web/e2e/tests/chat/auto-scroll-toggle.spec.ts @@ -285,6 +285,10 @@ test.describe("Transcript auto-scroll toggle", () => { // was already scrolled away from before disabling, not progression. await toggleAfter.click(); await expect(toggleAfter).toHaveAttribute("aria-pressed", "true"); + // deliberate-sleep(negative-assertion): asserts a *negative* (no jump to + // bottom). There is no event for "the scroll that must not happen", so the + // only way to give a regression room to occur is to wait out the + // smooth-scroll window before sampling the position. await testPage.waitForTimeout(300); expect(await listAfter.evaluate((el) => el.scrollTop)).toBeGreaterThan(targetScrollTop - 20); }); @@ -317,6 +321,10 @@ test.describe("Transcript auto-scroll toggle", () => { // already existed below their view before they disabled. await toggle.click(); await expect(toggle).toHaveAttribute("aria-pressed", "true"); + // deliberate-sleep(negative-assertion): asserts a *negative* (no jump to + // bottom). There is no event for "the scroll that must not happen", so the + // only way to give a regression room to occur is to wait out the + // smooth-scroll window before sampling the position. await testPage.waitForTimeout(300); expect(await list.evaluate((el) => el.scrollTop)).toBeGreaterThan(targetScrollTop - 20); expect(await list.evaluate((el) => el.scrollTop)).toBeLessThan(targetScrollTop + 20); @@ -353,6 +361,10 @@ test.describe("Transcript auto-scroll toggle", () => { await toggle.click(); await expect(toggle).toHaveAttribute("aria-pressed", "true"); + // deliberate-sleep(negative-assertion): asserts a *negative* (no jump to + // bottom). There is no event for "the scroll that must not happen", so the + // only way to give a regression room to occur is to wait out the + // smooth-scroll window before sampling the position. await testPage.waitForTimeout(300); expect(await list.evaluate((el) => el.scrollTop)).toBe(0); }); diff --git a/apps/web/e2e/tests/chat/busy-signal.spec.ts b/apps/web/e2e/tests/chat/busy-signal.spec.ts index a26f0bd87a..091fed608d 100644 --- a/apps/web/e2e/tests/chat/busy-signal.spec.ts +++ b/apps/web/e2e/tests/chat/busy-signal.spec.ts @@ -53,10 +53,17 @@ test.describe("Coarse RUNNING busy signal", () => { await expect(testPage.getByText("Kicking off background work")).toBeVisible({ timeout: 15_000, }); - // The foreground-idle frame follows this text in the mock's ordered ACP - // stream. Allow the subsequent WS publication to settle before asserting - // the stable composer contract. - await testPage.waitForTimeout(500); + // No wait is needed between the marker text and the assertions below, and + // the 500ms sleep that used to sit here was covering nothing. + // + // Measured: by the time the marker text is visible, BOTH + // `session.activity_changed` frames for this turn have already been + // received, and no further one arrives in the next 8s. The original comment + // here claimed the foreground-idle frame "follows this text"; it does not. + // So the assertions below already run against the post-transition state, + // and `waitForActiveSessionForegroundActivity` is itself the real check: + // if a regression let the idle frame downgrade the public contract, it + // would time out waiting for "generating" rather than pass silently. // The private tracker may identify background work, but the public // contract remains coarse for the entire RUNNING turn. diff --git a/apps/web/e2e/tests/chat/kandev-system-wrap.spec.ts b/apps/web/e2e/tests/chat/kandev-system-wrap.spec.ts index c06e43af86..92cd2c743d 100644 --- a/apps/web/e2e/tests/chat/kandev-system-wrap.spec.ts +++ b/apps/web/e2e/tests/chat/kandev-system-wrap.spec.ts @@ -23,14 +23,23 @@ async function waitForUserMessage( sessionId: string, timeoutMs = 20_000, ): Promise<{ content: string; raw_content?: string; metadata?: Record }> { - const deadline = Date.now() + timeoutMs; - while (Date.now() < deadline) { - const { messages } = await apiClient.listSessionMessages(sessionId); - const user = messages.find((m) => m.author_type === "user"); - if (user) return user; - await new Promise((r) => setTimeout(r, 250)); - } - throw new Error(`No user message recorded on session ${sessionId} within ${timeoutMs}ms`); + let user: + | Awaited>["messages"][number] + | undefined; + await expect + .poll( + async () => { + const { messages } = await apiClient.listSessionMessages(sessionId); + user = messages.find((m) => m.author_type === "user"); + return Boolean(user); + }, + { + timeout: timeoutMs, + message: `No user message recorded on session ${sessionId}`, + }, + ) + .toBe(true); + return user!; } test.describe("Kandev system prompt wrap on first launch", () => { diff --git a/apps/web/e2e/tests/chat/last-prompt-scroll.spec.ts b/apps/web/e2e/tests/chat/last-prompt-scroll.spec.ts index 6560c6f723..bdfa51aa0c 100644 --- a/apps/web/e2e/tests/chat/last-prompt-scroll.spec.ts +++ b/apps/web/e2e/tests/chat/last-prompt-scroll.spec.ts @@ -285,7 +285,24 @@ test.describe("@chat last prompt scroll affordance", () => { .getByTestId("scroll-to-last-prompt-button"); await scrollButton.click(); await expect(marker).toBeInViewport({ timeout: 10_000 }); - await testPage.waitForTimeout(500); + // The scroll is animated, so `toBeInViewport` can pass mid-flight. Wait + // for the container's scrollTop to stop changing before sampling geometry + // from it, rather than assuming the animation fits in a fixed budget. + let previousScrollTop = Number.NaN; + await expect + .poll( + async () => { + const current = await chat.evaluate( + (root) => + root.querySelector(".chat-message-list")?.scrollTop ?? Number.NaN, + ); + const settled = current === previousScrollTop; + previousScrollTop = current; + return settled; + }, + { timeout: 10_000, message: "scroll animation did not settle" }, + ) + .toBe(true); const partialGeometry = await chat.evaluate((root, markerText) => { const scrollContainer = root.querySelector(".chat-message-list"); diff --git a/apps/web/e2e/tests/chat/markdown-preview.spec.ts b/apps/web/e2e/tests/chat/markdown-preview.spec.ts index a7b0d705b3..2c55686bef 100644 --- a/apps/web/e2e/tests/chat/markdown-preview.spec.ts +++ b/apps/web/e2e/tests/chat/markdown-preview.spec.ts @@ -232,8 +232,9 @@ test.describe("Markdown preview", () => { expect(mdTab).toBeTruthy(); expect(mdTab.markdownPreview).toBe(true); - // Brief pause to let the sessionStorage write settle before reload - await testPage.waitForTimeout(500); + // No settle needed before the reload: the assertions above already read + // the flag back out of sessionStorage, so the write has demonstrably + // landed by the time we get here. // Reload the page — sessionStorage survives same-URL reload. // After reload, the restored file tab becomes active (not the chat), diff --git a/apps/web/e2e/tests/chat/message-queue-pin.spec.ts b/apps/web/e2e/tests/chat/message-queue-pin.spec.ts index 2b7c497f2a..8d42c1dfbe 100644 --- a/apps/web/e2e/tests/chat/message-queue-pin.spec.ts +++ b/apps/web/e2e/tests/chat/message-queue-pin.spec.ts @@ -2,7 +2,7 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; /** @@ -34,7 +34,7 @@ async function seedPinnedQueueTask( await session.sendMessage("/slow 30s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = testPage.locator(".tiptap.ProseMirror").first(); await typeWhileBusy(testPage, editor, "queued while pinned"); diff --git a/apps/web/e2e/tests/chat/message-queue-reorder.spec.ts b/apps/web/e2e/tests/chat/message-queue-reorder.spec.ts index 527388a3ba..04b5d6e070 100644 --- a/apps/web/e2e/tests/chat/message-queue-reorder.spec.ts +++ b/apps/web/e2e/tests/chat/message-queue-reorder.spec.ts @@ -2,10 +2,13 @@ import { expect, type Locator, type Page } from "@playwright/test"; import { test } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; import { registerSeparateQueueRows } from "../../helpers/message-queue-settings"; +/** Fragment of dnd-kit's default `onDragOver` accessibility announcement. */ +const MOVED_OVER = "was moved over droppable area"; + registerSeparateQueueRows(test); /** Create a task whose agent finishes its intro turn and becomes idle. */ @@ -101,7 +104,7 @@ test.describe("Queue reorder", () => { const session = await seedIdleTask(testPage, apiClient, seedData, "Queue reorder drag"); await session.sendMessage("/slow 30s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = session.activeChat().locator(".tiptap.ProseMirror:visible").first(); await queueMessagesWhileBusy(testPage, editor, [ @@ -163,7 +166,7 @@ test.describe("Queue reorder", () => { const session = await seedIdleTask(testPage, apiClient, seedData, "Queue reorder keyboard"); await session.sendMessage("/slow 30s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = session.activeChat().locator(".tiptap.ProseMirror:visible").first(); await queueMessagesWhileBusy(testPage, editor, [ @@ -182,11 +185,25 @@ test.describe("Queue reorder", () => { await handle.focus(); await testPage.keyboard.press("Space"); await expect(row).toHaveClass(/opacity-40/, { timeout: 5_000 }); + // dnd-kit announces every resolved drag target in its accessibility live + // region. That makes the two internal steps below observable rather than + // guessed at: droppable measuring after the drag starts, and target + // resolution after the arrow key. + const announcedTarget = async () => + (await testPage.locator('[id^="DndLiveRegion"]').allTextContents()).find((text) => + text.includes(MOVED_OVER), + ) ?? ""; + // Droppable measuring happens asynchronously after the drag starts; an - // arrow press before it completes computes no target. Settle briefly. - await testPage.waitForTimeout(200); + // arrow press before a droppable resolves computes no target. Measuring is + // done once dnd-kit announces a target for the held row. + await expect.poll(announcedTarget, { timeout: 5_000 }).toContain(MOVED_OVER); + const targetBeforeArrow = await announcedTarget(); + await testPage.keyboard.press("ArrowDown"); - await testPage.waitForTimeout(100); + // The arrow has resolved the next droppable once the announced target changes. + await expect.poll(announcedTarget, { timeout: 5_000 }).not.toBe(targetBeforeArrow); + await testPage.keyboard.press("Space"); await expect(row).not.toHaveClass(/opacity-40/, { timeout: 5_000 }); diff --git a/apps/web/e2e/tests/chat/message-queue.spec.ts b/apps/web/e2e/tests/chat/message-queue.spec.ts index dd66f39fab..98b0d52836 100644 --- a/apps/web/e2e/tests/chat/message-queue.spec.ts +++ b/apps/web/e2e/tests/chat/message-queue.spec.ts @@ -2,7 +2,7 @@ import { type Locator, type Page } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; import { expectFullQueueScrolls, seedFullQueueTask } from "./message-queue-scroll-helpers"; import { @@ -101,7 +101,7 @@ test.describe("Quick chat queue", () => { timeout: 15_000, }, ); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(dialog); await typeWhileBusy(testPage, editor, "hello world"); await testPage.keyboard.press(`${modifier}+Enter`); @@ -142,7 +142,7 @@ test.describe("Quick chat queue", () => { timeout: 15_000, }, ); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(dialog); // Before typing, only the cancel button should be visible (no send button). const submitBtn = dialog.getByTestId("submit-message-button"); @@ -288,7 +288,7 @@ test.describe("Task session queue", () => { // Send a slow command to keep the agent busy. await session.sendMessage("/slow 5s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); // Type a message while agent is busy. const editor = testPage.locator(".tiptap.ProseMirror").first(); @@ -382,7 +382,7 @@ test.describe("Task session queue", () => { ); await session.sendMessage("/slow 30s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = testPage.locator(".tiptap.ProseMirror").first(); await queueMessagesWhileBusy(testPage, editor, ["bulk first", "bulk second", "bulk third"]); @@ -431,7 +431,7 @@ test.describe("Task session queue", () => { // Send a slow command to keep the agent busy. await session.sendMessage("/slow 10s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); // Type a short message while agent is busy and queue it. const editor = testPage.locator(".tiptap.ProseMirror").first(); @@ -464,20 +464,20 @@ test.describe("Task session queue", () => { el.dispatchEvent(new Event("change", { bubbles: true })); }, longText); - // Allow layout to settle after content change. - await testPage.waitForTimeout(300); - // Verify the textarea has a constrained max-height and is scrollable. - const metrics = await textarea.evaluate((el: HTMLTextAreaElement) => ({ - scrollHeight: el.scrollHeight, - clientHeight: el.clientHeight, - maxHeight: getComputedStyle(el).maxHeight, - overflowY: getComputedStyle(el).overflowY, - })); - - expect(metrics.scrollHeight).toBeGreaterThan(metrics.clientHeight); - expect(metrics.maxHeight).toBe("200px"); - expect(metrics.overflowY).toBe("auto"); + // The auto-grow effect re-measures after React commits the new value, so + // poll the metrics rather than sampling them once behind a fixed settle. + await expect + .poll( + async () => + textarea.evaluate((el: HTMLTextAreaElement) => ({ + scrollable: el.scrollHeight > el.clientHeight, + maxHeight: getComputedStyle(el).maxHeight, + overflowY: getComputedStyle(el).overflowY, + })), + { timeout: 5_000 }, + ) + .toEqual({ scrollable: true, maxHeight: "200px", overflowY: "auto" }); }); test("merges a queued message into the message above it", async ({ @@ -492,7 +492,7 @@ test.describe("Task session queue", () => { // Send a slow command to keep the agent busy while we queue two messages. await session.sendMessage("/slow 10s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = testPage.locator(".tiptap.ProseMirror").first(); await typeWhileBusy(testPage, editor, "first message"); @@ -539,7 +539,7 @@ test.describe("Task session queue", () => { // Send a slow command to keep the agent busy. await session.sendMessage("/slow 5s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); // In plan mode with no typed text, only the cancel button should be visible. // The auto-added plan context should NOT cause the send button to appear. @@ -592,7 +592,7 @@ test.describe("Queue affordance", () => { await expect(testPage.getByRole("status", { name: /Agent is (starting|running)/ })).toBeVisible( { timeout: 15_000 }, ); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(dialog); await typeWhileBusy(testPage, editor, "first queued"); await testPage.keyboard.press(`${modifier}+Enter`); @@ -629,7 +629,7 @@ test.describe("Queue affordance", () => { await expect(testPage.getByRole("status", { name: /Agent is (starting|running)/ })).toBeVisible( { timeout: 15_000 }, ); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(dialog); await typeWhileBusy(testPage, editor, "queued for esc"); await testPage.keyboard.press(`${modifier}+Enter`); @@ -659,7 +659,7 @@ test.describe("Queue affordance", () => { await session.sendMessage("/slow 10s"); await expect(session.agentStatus()).toBeVisible({ timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); const editor = testPage.locator(".tiptap.ProseMirror").first(); await typeWhileBusy(testPage, editor, "to be cleared"); diff --git a/apps/web/e2e/tests/chat/message-timestamp-tooltip.spec.ts b/apps/web/e2e/tests/chat/message-timestamp-tooltip.spec.ts index fb7eb6d719..07a9749acb 100644 --- a/apps/web/e2e/tests/chat/message-timestamp-tooltip.spec.ts +++ b/apps/web/e2e/tests/chat/message-timestamp-tooltip.spec.ts @@ -41,6 +41,10 @@ test.describe("Chat message timestamp tooltip", () => { .locator("xpath=.."); const timestamp = messageRow.locator("time[datetime]"); await timestamp.hover(); + // deliberate-sleep(browser-chrome): the only thing this dwell waits for is + // Chrome's native `title` tooltip delay, which is browser chrome rather + // than DOM, so there is nothing in the page to observe. Kept solely so the + // capture below shows the same hover dwell a real user would see. await testPage.waitForTimeout(1500); await prCapture.screenshot("message-relative-timestamp-hover", { caption: diff --git a/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts b/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts index 9f4a0edcbd..82e2be18af 100644 --- a/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-busy-signal.spec.ts @@ -32,7 +32,10 @@ test.describe("Mobile coarse RUNNING busy signal", () => { await expect(testPage.getByText("Kicking off background work")).toBeVisible({ timeout: 20_000, }); - await testPage.waitForTimeout(500); + // No wait is needed here; see busy-signal.spec.ts for the measurement. The + // activity_changed frames for the turn land before the marker text is + // visible, so the assertions below already run against the post-transition + // state and `waitForActiveSessionForegroundActivity` is the real check. await waitForActiveSessionForegroundActivity(testPage, "generating"); await expect(session.idleInput()).not.toBeVisible(); diff --git a/apps/web/e2e/tests/chat/mobile-message-queue-management.spec.ts b/apps/web/e2e/tests/chat/mobile-message-queue-management.spec.ts index cbc8411116..1d7e534296 100644 --- a/apps/web/e2e/tests/chat/mobile-message-queue-management.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-message-queue-management.spec.ts @@ -2,7 +2,7 @@ import { expect, type Locator, type Page } from "@playwright/test"; import { test } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; import { expectFullQueueScrolls, seedFullQueueTask } from "./message-queue-scroll-helpers"; import { registerSeparateQueueRows } from "../../helpers/message-queue-settings"; @@ -39,7 +39,7 @@ async function seedBusyQueueTask( await session.waitForChatIdle({ timeout: 30_000 }); await session.sendMessageViaButton("/slow 30s"); await session.agentStatus().waitFor({ state: "visible", timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); return { session, taskId: task.id }; } diff --git a/apps/web/e2e/tests/chat/mobile-message-queue-reorder.spec.ts b/apps/web/e2e/tests/chat/mobile-message-queue-reorder.spec.ts index d8eb2b184a..e737512546 100644 --- a/apps/web/e2e/tests/chat/mobile-message-queue-reorder.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-message-queue-reorder.spec.ts @@ -2,10 +2,13 @@ import { expect, type Locator, type Page } from "@playwright/test"; import { test } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; -import { typeWhileBusy } from "../../helpers/type-while-busy"; +import { typeWhileBusy, waitForComposerQueueMode } from "../../helpers/type-while-busy"; import { SessionPage } from "../../pages/session-page"; import { registerSeparateQueueRows } from "../../helpers/message-queue-settings"; +/** Fragment of dnd-kit's default `onDragOver` accessibility announcement. */ +const MOVED_OVER = "was moved over droppable area"; + registerSeparateQueueRows(test); async function expectTouchTarget(locator: Locator): Promise { @@ -39,7 +42,7 @@ async function seedBusyQueueTask( await session.waitForChatIdle({ timeout: 60_000 }); await session.sendMessageViaButton("/slow 30s"); await session.agentStatus().waitFor({ state: "visible", timeout: 15_000 }); - await testPage.waitForTimeout(500); + await waitForComposerQueueMode(testPage); return session; } @@ -98,19 +101,40 @@ async function touchDragTo( clientX: start.x, clientY: start.y, }; + // dnd-kit announces each resolved drag target in its accessibility live + // region; that announcement is the observable signal for the drag-start and + // target-resolution steps between these dispatches. + const announcedTarget = async () => + (await page.locator('[id^="DndLiveRegion"]').allTextContents()).find((text) => + text.includes(MOVED_OVER), + ) ?? ""; + await source.dispatchEvent("pointerdown", down); - // Each event is a separate task; the gaps let React commit the drag start - // and dnd-kit finish droppable measuring before the move dispatches - // DragMove — otherwise the drop resolves no target. + // deliberate-sleep(library-timer): dnd-kit's PointerSensor arms on + // pointerdown and only activates once a later move clears its distance + // constraint. Nothing is rendered in between, so there is no DOM signal to + // wait for; this just lets React commit the pointerdown before the + // activating move lands in the same task. await page.waitForTimeout(100); await page.dispatchEvent("body", "pointermove", { ...down, clientX: end.x, clientY: end.y }); - await page.waitForTimeout(100); + + // The first move activates the drag; measuring is complete once dnd-kit can + // announce a droppable for it. Dispatching DragMove before that resolves no + // target and the drop silently becomes a no-op. + await expect.poll(announcedTarget, { timeout: 5_000 }).toContain(MOVED_OVER); + + // The second move is a 2px nudge that re-dispatches DragMove against the + // same droppable, so it produces no NEW announcement to wait on: a second + // `toContain(MOVED_OVER)` here would be satisfied by the first move's + // announcement and assert nothing, and a `.not.toBe(previous)` would never + // become true. The wait above already proves the drag is live and measured, + // which is the precondition pointerup actually needs. await page.dispatchEvent("body", "pointermove", { ...down, clientX: end.x, clientY: end.y + 2, }); - await page.waitForTimeout(100); + await page.dispatchEvent("body", "pointerup", { ...down, clientX: end.x, diff --git a/apps/web/e2e/tests/chat/tool-execute-output.spec.ts b/apps/web/e2e/tests/chat/tool-execute-output.spec.ts index 3f5c521bb3..e5c6d73633 100644 --- a/apps/web/e2e/tests/chat/tool-execute-output.spec.ts +++ b/apps/web/e2e/tests/chat/tool-execute-output.spec.ts @@ -182,6 +182,9 @@ test.describe("shell command output", () => { await expect.poll(() => requestCount, { timeout: 4_000 }).toBe(2); await expect(chat.getByText("final running transcript")).toBeVisible(); await expect(chat.getByText("Exit code 0")).toBeVisible(); + // deliberate-sleep(negative-assertion): asserts that *no further* fetches + // occur after the stream completes. A negative like this has no event to + // wait on, so it needs a real dwell past the polling interval. await testPage.waitForTimeout(1_250); expect(requestCount).toBe(2); }); diff --git a/apps/web/e2e/tests/chat/toolbar-overflow.spec.ts b/apps/web/e2e/tests/chat/toolbar-overflow.spec.ts index cafba0ad14..ec9653878d 100644 --- a/apps/web/e2e/tests/chat/toolbar-overflow.spec.ts +++ b/apps/web/e2e/tests/chat/toolbar-overflow.spec.ts @@ -39,17 +39,26 @@ async function seedAndNavigate( /** Force the toolbar container to a specific max-width via inline style. */ async function constrainToolbar(page: Page, maxWidth: string | null) { + // Wait on the ResizeObserver callback itself rather than a fixed budget: + // observe the toolbar first, apply the width, and resolve on the resize the + // change produces. The trailing rAF lets the observer's own re-render commit + // before the caller asserts against the toolbar. await page.evaluate((mw) => { const el = document.querySelector('[data-testid="chat-input-toolbar"]') as HTMLElement; - if (!el) return; - if (mw) { - el.style.maxWidth = mw; - } else { - el.style.removeProperty("max-width"); - } + if (!el) return undefined; + return new Promise((resolve) => { + const observer = new ResizeObserver(() => { + observer.disconnect(); + requestAnimationFrame(() => requestAnimationFrame(() => resolve())); + }); + observer.observe(el); + if (mw) { + el.style.maxWidth = mw; + } else { + el.style.removeProperty("max-width"); + } + }); }, maxWidth); - // Allow ResizeObserver to fire - await page.waitForTimeout(200); } test.describe("Toolbar overflow menu", () => { diff --git a/apps/web/e2e/tests/task/create-task-repository-tooltip.spec.ts b/apps/web/e2e/tests/task/create-task-repository-tooltip.spec.ts index a8a25455c3..e114b63191 100644 --- a/apps/web/e2e/tests/task/create-task-repository-tooltip.spec.ts +++ b/apps/web/e2e/tests/task/create-task-repository-tooltip.spec.ts @@ -45,6 +45,10 @@ test.describe("Create task repository tooltip", () => { const tooltip = testPage.getByRole("tooltip").filter({ hasText: longRepoDir }); expect(await trigger.evaluate((element) => element.matches(":hover"))).toBe(true); + // deliberate-sleep(negative-assertion): selecting from the menu must not + // leave a tooltip open even though the trigger is still hovered. There is + // no event for a tooltip that must never open, so the only way to give a + // regression room to appear is to wait out Radix's open delay. await testPage.waitForTimeout(300); await expect(tooltip).toBeHidden(); @@ -103,7 +107,10 @@ test.describe("Create task repository tooltip", () => { repository.id, ); - await testPage.waitForTimeout(300); + // Selecting from the menu must settle to "no tooltip" before the re-hover + // below, or the assertion could pass on a leftover tooltip from the first + // hover. Assert that closed state directly instead of sleeping toward it. + await expect(testPage.getByRole("tooltip")).toBeHidden(); await testPage.mouse.move(0, 0); await trigger.hover(); await expect(testPage.getByRole("tooltip").filter({ hasText: repositoryDir })).toBeVisible(); diff --git a/apps/web/e2e/tests/task/file-tree-rename.spec.ts b/apps/web/e2e/tests/task/file-tree-rename.spec.ts index b0717cedd0..6a39812e7a 100644 --- a/apps/web/e2e/tests/task/file-tree-rename.spec.ts +++ b/apps/web/e2e/tests/task/file-tree-rename.spec.ts @@ -151,8 +151,9 @@ test.describe("File tree inline rename", () => { const input = await startRenameViaContextMenu(testPage, node); await input.press("ControlOrMeta+A"); await input.fill("blur-final.ts"); - // The blur gate is ~400ms after isRenaming flips. Wait that out before - // moving focus elsewhere so the blur handler actually fires the commit. + // deliberate-sleep(product-timer): the product gates blur-commit on a + // ~400ms timer after isRenaming flips. That timer is the thing being waited + // for and it publishes nothing to observe, so the wait has to outlast it. await testPage.waitForTimeout(500); // Click another file to blur the input. The other node also belongs to // the tree, so we don't lose tree-container focus state. diff --git a/apps/web/e2e/tests/task/mobile-create-task-repository-selection.spec.ts b/apps/web/e2e/tests/task/mobile-create-task-repository-selection.spec.ts index 883e418993..96e583f94b 100644 --- a/apps/web/e2e/tests/task/mobile-create-task-repository-selection.spec.ts +++ b/apps/web/e2e/tests/task/mobile-create-task-repository-selection.spec.ts @@ -28,6 +28,9 @@ test.describe("Create task workspace repository picker on mobile", () => { await expect(selectedElsewhere.getByTestId("already-added-repository-marker")).toBeVisible(); await selectedElsewhere.tap(); await expect(repositoryChips.nth(1)).toContainText("E2E Repo"); + // deliberate-sleep(negative-assertion): a tap must not leave a hover + // tooltip behind on touch. Nothing is rendered to wait for, so the only + // meaningful check is to outlast Radix's open delay first. await testPage.waitForTimeout(300); await expect(testPage.getByRole("tooltip")).toBeHidden(); await assertNoDocumentHorizontalOverflow(testPage, "repository picker selection"); diff --git a/apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts b/apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts index 8df635a326..6c7e4c9426 100644 --- a/apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts +++ b/apps/web/e2e/tests/task/mobile-subtask-reparent-drag-drop.spec.ts @@ -1,5 +1,6 @@ import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; +import { settledBoundingBox } from "../../helpers/settled-box"; /** * Touch-drags a task row's handle onto a nest drop zone via CDP touch events. @@ -15,29 +16,26 @@ async function touchDragRowToNestZone( handleLocator: ReturnType, zoneLocator: ReturnType, ) { - await handleLocator.scrollIntoViewIfNeeded(); - await page.waitForTimeout(100); - const handleBox = await handleLocator.boundingBox(); - if (!handleBox) throw new Error("drag source handle has no bounding box"); + const handleBox = await settledBoundingBox(handleLocator); const startX = handleBox.x + handleBox.width / 2; const startY = handleBox.y + handleBox.height / 2; await cdp.send("Input.dispatchTouchEvent", { type: "touchStart", touchPoints: [{ x: startX, y: startY }], }); - // Hold still past the sensor delay, then move past the 5px tolerance to - // activate the drag. + // deliberate-sleep(library-timer): dnd-kit's TouchSensor arms a 250ms + // activation timer on touchStart and only starts the drag if the touch is + // still held when it fires. Nothing renders while that timer runs, so it has + // to be waited out rather than observed. await page.waitForTimeout(350); await cdp.send("Input.dispatchTouchEvent", { type: "touchMove", touchPoints: [{ x: startX + 14, y: startY }], }); - await page.waitForTimeout(50); + // The zone appearing is the signal that the drag went live, and asserting + // it is itself the wait -- no settle needed before or after. await expect(zoneLocator).toBeVisible(); - await zoneLocator.scrollIntoViewIfNeeded(); - await page.waitForTimeout(50); - const to = await zoneLocator.boundingBox(); - if (!to) throw new Error("nest drop zone has no bounding box"); + const to = await settledBoundingBox(zoneLocator); const endX = to.x + to.width / 2; const endY = to.y + to.height / 2; for (let i = 1; i <= 10; i++) { diff --git a/apps/web/e2e/tests/task/plan-checkpointing.spec.ts b/apps/web/e2e/tests/task/plan-checkpointing.spec.ts index db27184d57..f614216adf 100644 --- a/apps/web/e2e/tests/task/plan-checkpointing.spec.ts +++ b/apps/web/e2e/tests/task/plan-checkpointing.spec.ts @@ -3,6 +3,7 @@ import { test, expect } from "../../fixtures/test-base"; import type { SeedData } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; import { SessionPage } from "../../pages/session-page"; +import { attachGatewayTrafficCapture } from "../../helpers/ws-traffic"; // --------------------------------------------------------------------------- // Overview @@ -323,6 +324,10 @@ test.describe("Plan checkpointing — rewind UI", () => { }) => { test.setTimeout(120_000); + // Attach before the first navigation so the capture sees the websocket + // from the moment it opens. + const traffic = attachGatewayTrafficCapture(testPage); + const script = planWriteScript([{ content: "Agent wrote this" }]); const { session } = await seedTaskAndWaitForIdle( testPage, @@ -331,6 +336,10 @@ test.describe("Plan checkpointing — rewind UI", () => { "Checkpoint author switch", script, ); + const planUpdatesBeforeEdit = () => + traffic.frames.filter((frame) => frame.action === "task.plan.update").length; + const planUpdatesAtStart = planUpdatesBeforeEdit(); + await openPlanPanel(session); await expect(session.planPanel.getByText("Agent wrote this", { exact: false })).toBeVisible({ timeout: 10_000, @@ -342,8 +351,15 @@ test.describe("Plan checkpointing — rewind UI", () => { const modifier = process.platform === "darwin" ? "Meta" : "Control"; await testPage.keyboard.press(`${modifier}+a`); await testPage.keyboard.type("User overwrote it"); - // Wait past autosave debounce + short settle time. - await testPage.waitForTimeout(2_000); + // Autosave is debounced, but the debounce firing is observable: the save + // travels as a `task.plan.update` frame on the gateway socket. Wait for + // that frame rather than for a budget chosen to outlast the debounce. + await expect + .poll(planUpdatesBeforeEdit, { + timeout: 15_000, + message: "plan autosave never sent a task.plan.update frame", + }) + .toBeGreaterThan(planUpdatesAtStart); await session.openRewind(); await expectRevisionCount(session, 2); diff --git a/apps/web/e2e/tests/task/sidebar-layout.spec.ts b/apps/web/e2e/tests/task/sidebar-layout.spec.ts index 7dbaf06e13..617748a5da 100644 --- a/apps/web/e2e/tests/task/sidebar-layout.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-layout.spec.ts @@ -342,13 +342,16 @@ test.describe("Sidebar layout — task timestamps and sorting", () => { workflow_step_id: seedData.startStepId, repository_ids: [seedData.repositoryId], }); - // Small delay so timestamps differ + // deliberate-sleep(clock-separation): this test asserts sidebar ordering by + // creation time, so consecutive creations need distinguishable `created_at` + // values. The thing being waited for is the clock, not an app event. await new Promise((r) => setTimeout(r, 50)); await apiClient.createTask(seedData.workspaceId, "Sort Task Middle", { workflow_id: seedData.workflowId, workflow_step_id: seedData.startStepId, repository_ids: [seedData.repositoryId], }); + // deliberate-sleep(clock-separation): same reason as above. await new Promise((r) => setTimeout(r, 50)); await apiClient.createTask(seedData.workspaceId, "Sort Task Newest", { workflow_id: seedData.workflowId, diff --git a/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts b/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts index fc6f2d4dde..26a4a93bbb 100644 --- a/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts +++ b/apps/web/e2e/tests/task/sidebar-scroll-preservation.spec.ts @@ -419,8 +419,10 @@ test.describe("sidebar scrolling", () => { }); await expect(navigationDialog).toBeVisible(); - // Let the old 60-frame sidebar reveal budget expire while the settings - // blocker keeps the task sidebar out of the DOM. + // deliberate-sleep(product-timer): the regression under test is a + // frame-count reveal budget expiring while the sidebar is absent from the + // DOM. The budget's expiry is the event, and nothing renders to signal + // it, so it has to be waited out rather than observed. await testPage.waitForTimeout(1_500); await navigationDialog.getByRole("button", { name: "Discard and leave" }).click(); diff --git a/apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts b/apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts index d03f85c7a4..c9a3eb7f20 100644 --- a/apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts +++ b/apps/web/e2e/tests/task/subtask-reparent-drag-drop.spec.ts @@ -1,6 +1,7 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../fixtures/test-base"; import { SessionPage } from "../../pages/session-page"; +import { settledBoundingBox } from "../../helpers/settled-box"; /** * Drags a sidebar task row's handle onto a nest drop zone using pointer @@ -13,10 +14,7 @@ async function dragRowToNestZone( handleLocator: ReturnType, zoneLocator: ReturnType, ) { - await handleLocator.scrollIntoViewIfNeeded(); - await page.waitForTimeout(100); - const from = await handleLocator.boundingBox(); - if (!from) throw new Error("drag source handle has no bounding box"); + const from = await settledBoundingBox(handleLocator); const startX = from.x + from.width / 2; const startY = from.y + from.height / 2; await page.mouse.move(startX, startY); diff --git a/apps/web/e2e/tests/task/task-default-layout.spec.ts b/apps/web/e2e/tests/task/task-default-layout.spec.ts index dcb3083949..255d61fd82 100644 --- a/apps/web/e2e/tests/task/task-default-layout.spec.ts +++ b/apps/web/e2e/tests/task/task-default-layout.spec.ts @@ -63,7 +63,30 @@ test.describe("Task default layout shape", () => { const session = new SessionPage(testPage); await session.waitForLoad(); await session.waitForDockviewReady(); - await testPage.waitForTimeout(500); // let the session-tab swap settle + + // The session-tab swap re-lays-out the dockview groups after it reports + // ready, so their geometry is still moving here. Wait for the group + // rectangles to stop changing instead of guessing how long that takes: + // the assertion below reads those exact rectangles. + let previousGroups: string | null = null; + await expect + .poll( + async () => { + const current = await testPage.evaluate(() => + JSON.stringify( + Array.from(document.querySelectorAll(".dv-groupview")).map((el) => { + const r = el.getBoundingClientRect(); + return [Math.round(r.x), Math.round(r.width)]; + }), + ), + ); + const settled = current === previousGroups && current !== "[]"; + previousGroups = current; + return settled; + }, + { timeout: 10_000, message: "dockview group layout did not settle" }, + ) + .toBe(true); const container = await testPage.getByTestId("dockview-task-layout").evaluate((el) => { const r = el.getBoundingClientRect(); diff --git a/apps/web/e2e/tests/task/task-dependencies.spec.ts b/apps/web/e2e/tests/task/task-dependencies.spec.ts index 5f4fe666ce..137fdb7227 100644 --- a/apps/web/e2e/tests/task/task-dependencies.spec.ts +++ b/apps/web/e2e/tests/task/task-dependencies.spec.ts @@ -48,6 +48,9 @@ async function expectStaysUnstarted( const deadline = Date.now() + 6_000; while (Date.now() < deadline) { expect(await sessionCount(apiClient, taskId), `${label} must not be started`).toBe(0); + // deliberate-sleep(poll-interval): interval for the dwell described above, + // not a settle. The assertion is that nothing starts during the window, so + // the loop has to keep sampling across it rather than await one event. await new Promise((resolve) => setTimeout(resolve, 500)); } }