diff --git a/apps/web/e2e/helpers/session-store.ts b/apps/web/e2e/helpers/session-store.ts index 0ada2a1ce7..0ec447c159 100644 --- a/apps/web/e2e/helpers/session-store.ts +++ b/apps/web/e2e/helpers/session-store.ts @@ -1,4 +1,4 @@ -import type { Page } from "@playwright/test"; +import { expect, type Page } from "@playwright/test"; type E2EStoreWindow = Window & { __KANDEV_E2E_STORE__?: { @@ -104,3 +104,33 @@ export async function seedAvailableCommands( { sid: sessionId, commandList: commands }, ); } + +/** + * Wait until `sessionId` is the active session AND has stopped changing. + * + * Clicking a session tab settles asynchronously, and the flicker specs install + * their observers straight afterwards: starting to observe mid-settle records + * the tail of the switch as if it were oscillation. Requiring two consecutive + * agreeing samples gives those specs the quiet baseline the fixed sleeps were + * approximating, while returning immediately once the switch is genuinely done. + */ +export async function waitForStableActiveSession( + page: Page, + sessionId: string, + timeout = 15_000, +): Promise { + let previous: string | null = null; + await expect + .poll( + async () => { + const current = await page.evaluate( + () => (window as E2EStoreWindow).__KANDEV_E2E_STORE__?.getState().tasks.activeSessionId, + ); + const stable = current === sessionId && previous === sessionId; + previous = current ?? null; + return stable; + }, + { timeout, message: `active session did not settle on ${sessionId}` }, + ) + .toBe(true); +} 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 a5342b2b94..e5c987efa1 100644 --- a/apps/web/e2e/tests/session/long-prepare-panels.spec.ts +++ b/apps/web/e2e/tests/session/long-prepare-panels.spec.ts @@ -58,11 +58,13 @@ test.describe("Long prepare (slow git fetch)", () => { await expect(fileTreeWaiting).toBeVisible({ timeout: 15_000 }); await expect(fileTreeManual).toHaveCount(0); - // Wait past the pre-fix retry budget (1+2+5+10 = 18s). The file tree - // must NOT have transitioned to the "manual" (Load Files) state — - // that's the regression. 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. + // 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); await expect(fileTreeManual).toHaveCount(0); diff --git a/apps/web/e2e/tests/session/session-flicker.spec.ts b/apps/web/e2e/tests/session/session-flicker.spec.ts index c69e24ddc3..5ec7b5d1fd 100644 --- a/apps/web/e2e/tests/session/session-flicker.spec.ts +++ b/apps/web/e2e/tests/session/session-flicker.spec.ts @@ -5,6 +5,7 @@ import type { SeedData } from "../../fixtures/test-base"; 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"; /** * Regression: opening / switching to a second agent session in a task that @@ -150,7 +151,7 @@ test.describe("Session flicker", () => { ); await session.sessionTabBySessionId(session1Id).click(); - await testPage.waitForTimeout(300); + await waitForStableActiveSession(testPage, session1Id); // Observe every change of the active session over the settle window. await testPage.evaluate(() => { @@ -171,6 +172,10 @@ 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); const activeLog = await testPage.evaluate( @@ -199,7 +204,7 @@ test.describe("Session flicker", () => { ); await session.sessionTabBySessionId(session1Id).click(); - await testPage.waitForTimeout(300); + await waitForStableActiveSession(testPage, session1Id); // Simulate the backend launch race: session #2 ends up with a DIFFERENT // task_environment_id than session #1. Keep re-forcing it so trailing WS @@ -225,15 +230,33 @@ test.describe("Session flicker", () => { if (p.id.startsWith("session:")) removals.push(p.id); }); }, session2Id); - await testPage.waitForTimeout(100); + // The diverge interval above fires every 30ms; wait for its first write to + // land in the store rather than assuming one tick has happened. + await testPage.waitForFunction( + (sid2) => + ( + window as unknown as { + __KANDEV_E2E_STORE__?: { + getState: () => { environmentIdBySessionId: Record }; + }; + } + ).__KANDEV_E2E_STORE__?.getState().environmentIdBySessionId[sid2] === + "diverged-env-for-session-2", + session2Id, + { timeout: 5_000 }, + ); // Switch to session #2 (now on a diverged env) and back — this is what used // to strip the sibling tab and tear the chat down. await session.sessionTabBySessionId(session2Id).click(); - await testPage.waitForTimeout(800); + await waitForStableActiveSession(testPage, session2Id); await session.sessionTabBySessionId(session1Id).click(); - await testPage.waitForTimeout(800); + 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); // Both tabs must remain present throughout, and the active session's chat 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 cd6b31cd67..c64f9d8639 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 @@ -72,6 +72,10 @@ 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); } // Ensure we actually polled meaningfully across the window. 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 9214005b08..6079624463 100644 --- a/apps/web/e2e/tests/session/session-stream-budget.spec.ts +++ b/apps/web/e2e/tests/session/session-stream-budget.spec.ts @@ -116,9 +116,11 @@ test.describe("Session stream budget", () => { }); } - // Leave a deliberate five-second observation window after the five task - // switches. This is a diagnostic total, not a compression-dependent - // correctness threshold. + // 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); const summary = summarizeGatewayTraffic(capture.frames); 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 35bf9b7046..7bd237d834 100644 --- a/apps/web/e2e/tests/session/session-tab-management.spec.ts +++ b/apps/web/e2e/tests/session/session-tab-management.spec.ts @@ -5,6 +5,10 @@ import type { SeedData } from "../../fixtures/test-base"; 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"; + +/** Wire action the kanban WS handler consumes when a task moves step. */ +const TASK_UPDATED_ACTION = "task.updated"; const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; @@ -187,6 +191,9 @@ 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 expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible(); await expect(session.sessionTabBySessionId(session2Id)).toBeVisible(); @@ -290,6 +297,9 @@ 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 expect(session.sessionTabBySessionId(session1Id)).not.toBeVisible(); }); @@ -344,6 +354,8 @@ 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 expect(session.sessionTabBySessionId(sessionA1Id)).not.toBeVisible(); await expect(session.sessionTabBySessionId(sessionA2Id)).not.toBeVisible(); @@ -365,6 +377,10 @@ test.describe("Session tab management — primary session persistence", () => { }) => { test.setTimeout(150_000); + // Attach before the first navigation so the capture sees the websocket from + // the moment it opens. + const traffic = attachGatewayTrafficCapture(testPage); + const { task, session, session1Id, session2Id } = await createTaskWithTwoSessions( testPage, apiClient, @@ -398,10 +414,49 @@ test.describe("Session tab management — primary session persistence", () => { // Trigger kanban.update by moving the task to a non-start step. const otherStep = seedData.steps.find((s) => s.id !== seedData.startStepId); if (!otherStep) throw new Error("Workflow needs at least 2 steps to trigger kanban.update"); + const taskUpdatesBeforeMove = () => + traffic.frames.filter( + (frame) => frame.direction === "received" && frame.action === TASK_UPDATED_ACTION, + ).length; + const taskUpdatesAtMove = taskUpdatesBeforeMove(); await apiClient.moveTask(task.id, seedData.workflowId, otherStep.id); - // Give the WS broadcast time to land. - await testPage.waitForTimeout(500); + // The kanban.update broadcast is what could wrongly move the star, so wait + // for the gateway to actually deliver it instead of budgeting for it. + // The move is delivered as `task.updated`, which `lib/ws/handlers/kanban.ts` + // consumes -- that handler is what this test is named after, and its + // regression was moving the star back to session #1. There is no + // `kanban.update` frame on the wire; waiting for one times out. Verified by + // dumping every received action after moveTask. + await expect + .poll(taskUpdatesBeforeMove, { + timeout: 15_000, + message: `no ${TASK_UPDATED_ACTION} frame was delivered after moveTask`, + }) + .toBeGreaterThan(taskUpdatesAtMove); + + // Observing the frame only proves the gateway delivered it, not that the + // handler under test ran. Both assertions below are already true before the + // move, so asserting in that gap would pass against the pre-move DOM and + // miss the regression entirely. Wait for the handler's own effect on the + // store -- the task's step in `kanban.tasks` -- before asserting. + await testPage.waitForFunction( + ({ taskId, stepId }) => { + const store = ( + window as Window & { + __KANDEV_E2E_STORE__?: { + getState: () => { kanban: { tasks: Array<{ id: string; workflowStepId: string }> } }; + }; + } + ).__KANDEV_E2E_STORE__; + if (!store) throw new Error("E2E store bridge missing"); + return ( + store.getState().kanban.tasks.find((t) => t.id === taskId)?.workflowStepId === stepId + ); + }, + { taskId: task.id, stepId: otherStep.id }, + { timeout: 15_000 }, + ); // Star must still be on session #2 (would jump back to #1 before the kanban.ts fix). await expect(starInTab(session, session2Id)).toBeVisible({ timeout: 5_000 }); 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 ebe3f9071e..10645ae72b 100644 --- a/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-agent-switch.spec.ts @@ -23,14 +23,17 @@ async function pollSessions( expectedCount: number, timeoutMs = 30_000, ) { - const start = Date.now(); - while (Date.now() - start < timeoutMs) { - const { sessions } = await apiClient.listTaskSessions(taskId); - if (sessions.length >= expectedCount) return sessions; - await new Promise((r) => setTimeout(r, 500)); - } - const { sessions } = await apiClient.listTaskSessions(taskId); - return sessions; + let latest: Awaited>["sessions"] = []; + await expect + .poll( + async () => { + latest = (await apiClient.listTaskSessions(taskId)).sessions; + return latest.length; + }, + { timeout: timeoutMs, message: `task ${taskId} never reached ${expectedCount} session(s)` }, + ) + .toBeGreaterThanOrEqual(expectedCount); + return latest; } async function waitForSessionEnvironmentId( @@ -39,20 +42,33 @@ async function waitForSessionEnvironmentId( agentProfileId: string, timeoutMs = 30_000, ) { - const start = Date.now(); - while (Date.now() - start < timeoutMs) { - const { sessions } = await apiClient.listTaskSessions(taskId); - const environmentId = sessions.find( - (s) => s.agent_profile_id === agentProfileId, - )?.task_environment_id; - if (environmentId) return environmentId; - await new Promise((r) => setTimeout(r, 500)); - } - const { sessions } = await apiClient.listTaskSessions(taskId); - const details = sessions - .map((s) => `${s.id}:${s.agent_profile_id}:${s.state}:${s.task_environment_id ?? "none"}`) - .join(", "); - throw new Error(`session for profile ${agentProfileId} did not get environment id: ${details}`); + let environmentId = ""; + let details = ""; + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(taskId); + // Captured on every attempt so the failure below describes the last + // observed state rather than requiring an extra request. + details = sessions + .map((s) => `${s.id}:${s.agent_profile_id}:${s.state}:${s.task_environment_id ?? "none"}`) + .join(", "); + environmentId = + sessions.find((s) => s.agent_profile_id === agentProfileId)?.task_environment_id ?? ""; + return environmentId !== ""; + }, + { + timeout: timeoutMs, + message: `session for profile ${agentProfileId} did not get environment id`, + }, + ) + .toBe(true) + .catch((err: Error) => { + // expect.poll's own message reports only the final polled value, so the + // captured session states have to be re-thrown here to reach the log. + throw new Error(`${err.message}; last observed sessions: ${details}`); + }); + return environmentId; } async function pollSessionsForEnvironmentInheritance( @@ -62,21 +78,36 @@ async function pollSessionsForEnvironmentInheritance( targetProfileId: string, timeoutMs = 30_000, ) { - const start = Date.now(); let latestSessions: Awaited> = []; - while (Date.now() - start < timeoutMs) { - const { sessions } = await apiClient.listTaskSessions(taskId); - latestSessions = sessions; - const sourceSession = sessions.find((s) => s.agent_profile_id === sourceProfileId); - const targetSession = sessions.find((s) => s.agent_profile_id === targetProfileId); - if ( - sourceSession?.task_environment_id && - targetSession?.task_environment_id === sourceSession.task_environment_id - ) { - return sessions; - } - await new Promise((r) => setTimeout(r, 500)); - } + let lastRequestError: unknown; + // Returns the last observed sessions either way: callers assert on the + // inheritance themselves, so a timeout here must not throw. + await expect + .poll( + async () => { + try { + const { sessions } = await apiClient.listTaskSessions(taskId); + lastRequestError = undefined; + latestSessions = sessions; + const sourceSession = sessions.find((s) => s.agent_profile_id === sourceProfileId); + const targetSession = sessions.find((s) => s.agent_profile_id === targetProfileId); + return Boolean( + sourceSession?.task_environment_id && + targetSession?.task_environment_id === sourceSession.task_environment_id, + ); + } catch (err) { + // Retry a transient request failure, but remember the last one: the + // blanket catch below is meant to tolerate "inheritance not visible + // yet", not to turn a dead endpoint into an empty session list. + lastRequestError = err; + return false; + } + }, + { timeout: timeoutMs }, + ) + .toBe(true) + .catch(() => undefined); + if (lastRequestError) throw lastRequestError; return latestSessions; } @@ -120,8 +151,18 @@ test.describe("Workflow agent profile switching", () => { expect(initialSessions.length).toBeGreaterThanOrEqual(1); expect(initialSessions[0].agent_profile_id).toBe(profileA.id); - // Wait for agent to be ready before moving - await new Promise((r) => setTimeout(r, 3000)); + // Wait for the agent to actually settle rather than budgeting 3s for it. + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some( + (s) => s.state === "WAITING_FOR_INPUT" || s.state === "IDLE" || s.state === "RUNNING", + ); + }, + { timeout: 30_000, message: "first session never became ready" }, + ) + .toBe(true); // Move task to Step2 — should create new session with profileB await apiClient.moveTask(task.id, workflow.id, step2.id); @@ -237,11 +278,15 @@ test.describe("Workflow agent profile switching", () => { }); // Wait for the agent to be ready (WAITING_FOR_INPUT) before moving - for (let i = 0; i < 20; i++) { - const { sessions } = await apiClient.listTaskSessions(task.id); - if (sessions.some((s) => s.state === "WAITING_FOR_INPUT")) break; - await new Promise((r) => setTimeout(r, 500)); - } + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 10_000, message: "no session reached WAITING_FOR_INPUT" }, + ) + .toBe(true); // Move task to Step2 — should create new session with profileB await apiClient.moveTask(task.id, workflow.id, step2.id); @@ -366,11 +411,15 @@ test.describe("Workflow agent profile switching", () => { ); // Wait for Step1 agent to finish - for (let i = 0; i < 20; i++) { - const { sessions } = await apiClient.listTaskSessions(task.id); - if (sessions.some((s) => s.state === "WAITING_FOR_INPUT")) break; - await new Promise((r) => setTimeout(r, 500)); - } + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 10_000, message: "no session reached WAITING_FOR_INPUT" }, + ) + .toBe(true); // Move to Step2 — should auto-launch agent despite no auto_start_agent await apiClient.moveTask(task.id, workflow.id, step2.id); @@ -426,11 +475,15 @@ test.describe("Workflow agent profile switching", () => { ); // Wait for Step1 agent to finish - for (let i = 0; i < 20; i++) { - const { sessions } = await apiClient.listTaskSessions(task.id); - if (sessions.some((s) => s.state === "WAITING_FOR_INPUT")) break; - await new Promise((r) => setTimeout(r, 500)); - } + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return sessions.some((s) => s.state === "WAITING_FOR_INPUT"); + }, + { timeout: 10_000, message: "no session reached WAITING_FOR_INPUT" }, + ) + .toBe(true); const step1EnvironmentId = await waitForSessionEnvironmentId(apiClient, task.id, profileA.id); // Move to Step2 @@ -909,6 +962,9 @@ 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)); } @@ -943,7 +999,6 @@ test.describe("Workflow agent profile switching", () => { // Click first step to open config panel const stepNodes = card.locator(".group.relative"); await stepNodes.first().click(); - await testPage.waitForTimeout(500); // Reset context checkbox should be enabled (no agent profile set) const resetCheckbox = card.getByRole("checkbox", { name: "Reset agent context" }); @@ -957,7 +1012,6 @@ test.describe("Workflow agent profile switching", () => { const reloadedCard = await page.findWorkflowCard("E2E Workflow"); const reloadedSteps = reloadedCard.locator(".group.relative"); await reloadedSteps.first().click(); - await testPage.waitForTimeout(500); // Reset context checkbox should be disabled const reloadedCheckbox = reloadedCard.getByRole("checkbox", { diff --git a/apps/web/e2e/tests/workflow/workflow-step-autocomplete.spec.ts b/apps/web/e2e/tests/workflow/workflow-step-autocomplete.spec.ts index 0fa5e74cb1..4949cd97d7 100644 --- a/apps/web/e2e/tests/workflow/workflow-step-autocomplete.spec.ts +++ b/apps/web/e2e/tests/workflow/workflow-step-autocomplete.spec.ts @@ -15,7 +15,6 @@ test.describe("Workflow step prompt autocomplete", () => { // Click first step to open config panel const stepNodes = card.locator(".group.relative"); await stepNodes.first().click(); - await testPage.waitForTimeout(500); // Wait for the ScriptEditor (Monaco) to mount inside the step config panel const monacoEditor = card.locator(".monaco-editor"); @@ -23,11 +22,16 @@ test.describe("Workflow step prompt autocomplete", () => { // Click into the editor to focus it await monacoEditor.click(); - await testPage.waitForTimeout(300); + // Typing before Monaco owns focus silently goes nowhere, so wait for the + // focus rather than budget for it. The selector is load-bearing and was + // established by probing the live DOM: this Monaco build uses the + // EditContext API, so the focus target is `div.native-edit-context`. There + // is no `textarea.inputarea`, and the only textarea present is a readonly + // `ime-text-area` that never receives focus. + await expect(monacoEditor.locator(".native-edit-context")).toBeFocused({ timeout: 5_000 }); // Type {{ to trigger autocomplete await testPage.keyboard.type("{{"); - await testPage.waitForTimeout(500); // The Monaco suggest widget should appear with {{task_prompt}} const suggestWidget = testPage.locator(".monaco-editor .suggest-widget"); @@ -57,7 +61,6 @@ test.describe("Workflow step prompt autocomplete", () => { // Click first step to open config panel const stepNodes = card.locator(".group.relative"); await stepNodes.first().click(); - await testPage.waitForTimeout(500); // Wait for the ScriptEditor (Monaco) to mount inside the step config panel const monacoEditor = card.locator(".monaco-editor"); @@ -65,11 +68,12 @@ test.describe("Workflow step prompt autocomplete", () => { // Click into the editor to focus it await monacoEditor.click(); - await testPage.waitForTimeout(300); + // Focus target is `div.native-edit-context` (EditContext API), not a + // textarea. See the note in the first test. + await expect(monacoEditor.locator(".native-edit-context")).toBeFocused({ timeout: 5_000 }); // Type @ to trigger the prompt-mention autocomplete await testPage.keyboard.type("@"); - await testPage.waitForTimeout(500); // The Monaco suggest widget should appear with the seeded prompt const suggestWidget = testPage.locator(".monaco-editor .suggest-widget"); @@ -82,7 +86,6 @@ test.describe("Workflow step prompt autocomplete", () => { // Accept the suggestion and verify the editor content now contains the mention. await suggestion.first().click(); - await testPage.waitForTimeout(300); await expect(monacoEditor).toContainText(`@${promptName}`); } finally { @@ -108,7 +111,6 @@ test.describe("Workflow step prompt autocomplete", () => { // Click first step to open config panel const stepNodes = card.locator(".group.relative"); await stepNodes.first().click(); - await testPage.waitForTimeout(500); // Find the step agent profile select const agentSelect = card.getByTestId("step-agent-profile-select"); @@ -120,10 +122,23 @@ test.describe("Workflow step prompt autocomplete", () => { // Click to open the dropdown await agentSelect.click(); - await testPage.waitForTimeout(300); - // Select the first non-"none" option (skip "No profile override") + // Select the first non-"none" option (skip "No profile override"). + // `count()` is a one-shot read, not an auto-retrying assertion, so gate on + // the listbox being populated before counting instead of sleeping first. + // + // Visibility of the *first* option is not that gate: "No profile override" + // is static markup and is already there while the settings bootstrap is + // still loading profiles, so a count() taken on it reads 1 and skips the + // test. Poll the count itself, which is the thing the branch below reads. const options = testPage.getByRole("option"); + await expect(options.first()).toBeVisible({ timeout: 5_000 }); + await expect + .poll(() => options.count(), { timeout: 5_000 }) + .toBeGreaterThan(1) + // A workspace with genuinely no profiles is a legitimate skip, so this + // stays tolerant; the poll only removes the race with a slow bootstrap. + .catch(() => undefined); const optionCount = await options.count(); // Need at least 2 options (none + at least one profile) if (optionCount < 2) { @@ -134,12 +149,12 @@ test.describe("Workflow step prompt autocomplete", () => { const profileOption = options.nth(1); const profileName = await profileOption.textContent(); await profileOption.click(); - await testPage.waitForTimeout(1000); - // The select should now show the selected profile, not revert to "No profile override" - const updatedText = await agentSelect.textContent(); - expect(updatedText).toContain(profileName?.trim() ?? ""); - expect(updatedText).not.toContain("No profile override"); + // The select should now show the selected profile, not revert to "No + // profile override". Assert both halves with auto-retrying matchers rather + // than sleeping and then taking a single non-retrying textContent sample. + await expect(agentSelect).toContainText(profileName?.trim() ?? "", { timeout: 10_000 }); + await expect(agentSelect).not.toContainText("No profile override"); await page.saveChanges(); // Reload the page and verify it persisted @@ -150,12 +165,10 @@ test.describe("Workflow step prompt autocomplete", () => { // Click the same step again const reloadedSteps = reloadedCard.locator(".group.relative"); await reloadedSteps.first().click(); - await testPage.waitForTimeout(500); const reloadedSelect = reloadedCard.getByTestId("step-agent-profile-select"); await expect(reloadedSelect).toBeVisible(); - const persistedText = await reloadedSelect.textContent(); - expect(persistedText).toContain(profileName?.trim() ?? ""); + await expect(reloadedSelect).toContainText(profileName?.trim() ?? "", { timeout: 10_000 }); // Clean up: reset the step agent profile const stepId = seedData.steps[0]?.id;