From efd0310e6cb3607dac04967ae72181417b5281c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Thu, 13 Aug 2026 21:38:27 +0100 Subject: [PATCH] test(e2e): replace unconditional sleeps in office specs with reactive polls (#2640) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- apps/web/e2e/helpers/office-launch.ts | 52 +++++++++++++++++++ .../e2e/tests/office/advanced-mode.spec.ts | 27 ++-------- .../e2e/tests/office/agent-run-detail.spec.ts | 5 ++ .../web/e2e/tests/office/mobile-tasks.spec.ts | 23 ++++---- .../office-routing-agent-override.spec.ts | 31 +++++------ .../office/office-routing-blocked.spec.ts | 23 ++++---- .../office/office-routing-fallback.spec.ts | 31 +++++------ .../office/office-routing-recovery.spec.ts | 41 ++++++++------- .../office/onboarding-task-launch.spec.ts | 4 ++ .../tests/office/realtime-dashboard.spec.ts | 22 +++++--- apps/web/e2e/tests/office/task-chat.spec.ts | 52 +++++++------------ 11 files changed, 175 insertions(+), 136 deletions(-) create mode 100644 apps/web/e2e/helpers/office-launch.ts diff --git a/apps/web/e2e/helpers/office-launch.ts b/apps/web/e2e/helpers/office-launch.ts new file mode 100644 index 0000000000..44ee297c66 --- /dev/null +++ b/apps/web/e2e/helpers/office-launch.ts @@ -0,0 +1,52 @@ +import { expect } from "@playwright/test"; +import type { ApiClient } from "./api-client"; + +/** Session states that mean the agent runtime is up and usable by a spec. */ +const LIVE_SESSION_STATES = new Set(["RUNNING", "WAITING_FOR_INPUT", "IDLE", "COMPLETED"]); + +/** Session states from which the agent will never become live. */ +const TERMINAL_SESSION_STATES = new Set(["FAILED", "CANCELLED"]); + +/** + * Wait until the agent session for an onboarding-created office task is live. + * + * The office task's own `status` is workflow-owned and intentionally stays + * `created` until an agent moves it, so it is *not* a launch signal — polling + * it for `in_progress`/`done` can never succeed on this path. The session + * state is the signal, and it is the same contract asserted directly by + * `tests/office/onboarding-task-launch.spec.ts`. + * + * The 45s budget is 3x the 15s that `onboarding-task-launch.spec.ts` already + * enforces for this same signal (it asserts after its loop, so CI has actually + * been holding it to that budget). The extra margin is because this wait lives + * in a worker-scoped fixture, where one timeout fails every test in the file. + * It also has to stay comfortably under the 60s project timeout: worker fixture + * setup is bounded by that, not by a test's own `test.setTimeout()`, so a + * budget at or above 60s could never report its own diagnostic. + */ +export async function waitForOfficeTaskSessionLive( + apiClient: ApiClient, + taskId: string, + timeoutMs = 45_000, +): Promise { + let observed = "none"; + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(taskId); + // Captured every attempt so the failure below can describe the last + // observed state without issuing an extra request. + observed = sessions.map((s) => `${s.id}:${s.state}`).join(", ") || "none"; + const state = sessions[0]?.state ?? ""; + if (TERMINAL_SESSION_STATES.has(state)) { + throw new Error(`agent session terminated before launch: ${observed}`); + } + return LIVE_SESSION_STATES.has(state); + }, + { timeout: timeoutMs, message: `task ${taskId} agent session never reached a live state` }, + ) + .toBe(true) + .catch((err: Error) => { + throw new Error(`${err.message}; last observed sessions: ${observed}`); + }); +} diff --git a/apps/web/e2e/tests/office/advanced-mode.spec.ts b/apps/web/e2e/tests/office/advanced-mode.spec.ts index 3d9e69016d..843ad2ed8e 100644 --- a/apps/web/e2e/tests/office/advanced-mode.spec.ts +++ b/apps/web/e2e/tests/office/advanced-mode.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 { waitForOfficeTaskSessionLive } from "../../helpers/office-launch"; /** * E2E tests for the office advanced mode dockview layout. @@ -31,7 +32,7 @@ const test = base.extend<{ testPage: Page }, AdvancedModeFixtures>({ ], advancedSeed: [ - async ({ officeApi, seedData }, use) => { + async ({ officeApi, apiClient, seedData }, use) => { const result = (await officeApi.completeOnboarding({ workspaceName: "Advanced Mode Workspace", taskPrefix: "AM", @@ -46,30 +47,10 @@ const test = base.extend<{ testPage: Page }, AdvancedModeFixtures>({ throw new Error("completeOnboarding did not return a taskId"); } - // Wait for the agent to leave the pre-launch states. The task - // state surfaces from the API in canonical lowercase form - // (`in_progress`, `in_review`, `done`, …); legacy SCREAMING_SNAKE_CASE - // values are accepted defensively. We only need the agent to have + // Wait for the agent runtime to come up. We only need it to have // *started* — the test pages exercise the live session once the // dockview mounts, they don't require a finished turn. - const launched = new Set([ - "in_progress", - "in_review", - "done", - "completed", - "waiting_for_input", - "review", - ]); - const deadline = Date.now() + 25_000; - while (Date.now() < deadline) { - const issue = await officeApi.getTask(result.taskId); - const raw = issue as Record; - const inner = (raw.task as Record) ?? raw; - const state = ((inner.state as string) ?? (inner.status as string) ?? "").toLowerCase(); - if (state === "failed") throw new Error("Task entered FAILED state"); - if (launched.has(state)) break; - await new Promise((r) => setTimeout(r, 500)); - } + await waitForOfficeTaskSessionLive(apiClient, result.taskId); await use({ workspaceId: result.workspaceId, 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 51b63d059d..cb8f7800ea 100644 --- a/apps/web/e2e/tests/office/agent-run-detail.spec.ts +++ b/apps/web/e2e/tests/office/agent-run-detail.spec.ts @@ -102,6 +102,11 @@ 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 route.continue(); }); diff --git a/apps/web/e2e/tests/office/mobile-tasks.spec.ts b/apps/web/e2e/tests/office/mobile-tasks.spec.ts index 41eed37b14..05b2799f87 100644 --- a/apps/web/e2e/tests/office/mobile-tasks.spec.ts +++ b/apps/web/e2e/tests/office/mobile-tasks.spec.ts @@ -140,16 +140,19 @@ async function waitForRoutingProfile( workspaceId: string, profileId: string, ): Promise<{ id: string; model: string }> { - const deadline = Date.now() + ROUTING_PROFILE_WAIT_MS; - while (Date.now() < deadline) { - const routing = await officeApi.getRouting(workspaceId); - const profile = routing.execution_profiles.find((candidate) => candidate.id === profileId); - if (profile) return profile; - await new Promise((resolve) => setTimeout(resolve, 250)); - } - throw new Error( - `Routing profile ${profileId} did not appear within ${ROUTING_PROFILE_WAIT_MS}ms`, - ); + let profile: { id: string; model: string } | undefined; + await expect + .poll( + async () => { + const routing = await officeApi.getRouting(workspaceId); + profile = routing.execution_profiles.find((candidate) => candidate.id === profileId); + return Boolean(profile); + }, + { timeout: ROUTING_PROFILE_WAIT_MS, message: `routing profile ${profileId} did not appear` }, + ) + .toBe(true); + if (!profile) throw new Error(`Routing profile ${profileId} did not appear`); + return profile; } type TestableApiClient = { diff --git a/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts b/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts index bcc730bd60..f924933510 100644 --- a/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts +++ b/apps/web/e2e/tests/office/office-routing-agent-override.spec.ts @@ -92,22 +92,23 @@ test.describe("Office provider routing — agent override", () => { // PATCH the assignee to fire the task_assigned dispatcher. await officeApi.assignTask(task.id!, officeSeed.agentId); - const deadline = Date.now() + 20_000; let attempts: Array<{ provider_id: string; outcome: string }> = []; - while (Date.now() < deadline) { - const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { - runs?: Array<{ id: string; task_id?: string }>; - }; - const run = (runs.runs ?? []).find((r) => r.task_id === task.id); - if (run?.id) { - const list = await officeApi.listRouteAttempts(run.id); - attempts = list.attempts; - if (attempts.length > 0) break; - } - await new Promise((r) => setTimeout(r, 500)); - } - - expect(attempts.length).toBeGreaterThan(0); + // `attempts` is captured on each poll so the assertions below read the last + // observed value rather than needing another request. + await expect + .poll( + async () => { + const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { + runs?: Array<{ id: string; task_id?: string }>; + }; + const run = (runs.runs ?? []).find((r) => r.task_id === task.id); + if (!run?.id) return 0; + attempts = (await officeApi.listRouteAttempts(run.id)).attempts; + return attempts.length; + }, + { timeout: 20_000, message: "run never recorded a route attempt" }, + ) + .toBeGreaterThan(0); // No codex-acp attempt — override is single-provider. expect(attempts.every((a) => a.provider_id === "claude-acp")).toBe(true); }); diff --git a/apps/web/e2e/tests/office/office-routing-blocked.spec.ts b/apps/web/e2e/tests/office/office-routing-blocked.spec.ts index b8d873d7ea..24b0cc3fbb 100644 --- a/apps/web/e2e/tests/office/office-routing-blocked.spec.ts +++ b/apps/web/e2e/tests/office/office-routing-blocked.spec.ts @@ -60,18 +60,17 @@ test.describe("Office provider routing — blocked", () => { // 40s tolerates heavy parallel-suite load — the dispatcher walks // each provider in turn and only flips the blocked_status after // every routable provider has parked. - const deadline = Date.now() + 40_000; - let blockedStatus = ""; - while (Date.now() < deadline) { - const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { - runs?: Array<{ id: string; task_id?: string; routing_blocked_status?: string }>; - }; - const run = (runs.runs ?? []).find((r) => r.task_id === task.id); - blockedStatus = run?.routing_blocked_status ?? ""; - if (blockedStatus === "blocked_provider_action_required") break; - await new Promise((r) => setTimeout(r, 500)); - } - expect(blockedStatus).toBe("blocked_provider_action_required"); + await expect + .poll( + async () => { + const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { + runs?: Array<{ id: string; task_id?: string; routing_blocked_status?: string }>; + }; + return (runs.runs ?? []).find((r) => r.task_id === task.id)?.routing_blocked_status ?? ""; + }, + { timeout: 40_000, message: "run never reached blocked_provider_action_required" }, + ) + .toBe("blocked_provider_action_required"); const inbox = (await officeApi.getInbox(officeSeed.workspaceId)) as { items?: Array<{ type?: string; provider_id?: string }>; diff --git a/apps/web/e2e/tests/office/office-routing-fallback.spec.ts b/apps/web/e2e/tests/office/office-routing-fallback.spec.ts index a778e8fefc..3f207dad1d 100644 --- a/apps/web/e2e/tests/office/office-routing-fallback.spec.ts +++ b/apps/web/e2e/tests/office/office-routing-fallback.spec.ts @@ -73,27 +73,28 @@ test.describe("Office provider routing — fallback", () => { // KANDEV_MOCK_PROVIDERS needs the full agent-registry rediscovery // walk to complete first — 40s rides that out without affecting // the happy path (resolves in <5s in isolation). - const deadline = Date.now() + 40_000; let attempts: Array<{ provider_id: string; execution_profile_id: string; outcome: string; error_code?: string; }> = []; - while (Date.now() < deadline) { - const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { - runs?: Array<{ id: string; task_id?: string }>; - }; - const run = (runs.runs ?? []).find((r) => r.task_id === task.id); - if (run?.id) { - const list = await officeApi.listRouteAttempts(run.id); - attempts = list.attempts; - if (attempts.length >= 2) break; - } - await new Promise((r) => setTimeout(r, 500)); - } - - expect(attempts.length).toBeGreaterThanOrEqual(2); + // `attempts` is captured on each poll so the assertions below read the last + // observed value rather than needing another request. + await expect + .poll( + async () => { + const runs = (await officeApi.listRuns(officeSeed.workspaceId)) as { + runs?: Array<{ id: string; task_id?: string }>; + }; + const run = (runs.runs ?? []).find((r) => r.task_id === task.id); + if (!run?.id) return 0; + attempts = (await officeApi.listRouteAttempts(run.id)).attempts; + return attempts.length; + }, + { timeout: 40_000, message: "run never recorded 2 route attempts" }, + ) + .toBeGreaterThanOrEqual(2); expect(attempts[0].provider_id).toBe("claude-acp"); expect(attempts[0].execution_profile_id).toBe( routingConfig.provider_profiles["claude-acp"].execution_profile_ids.balanced, diff --git a/apps/web/e2e/tests/office/office-routing-recovery.spec.ts b/apps/web/e2e/tests/office/office-routing-recovery.spec.ts index bb91441a06..798f54091a 100644 --- a/apps/web/e2e/tests/office/office-routing-recovery.spec.ts +++ b/apps/web/e2e/tests/office/office-routing-recovery.spec.ts @@ -54,14 +54,17 @@ test.describe("Office provider routing — recovery", () => { await officeApi.assignTask(degradeTask.id!, officeSeed.agentId); // Wait for claude-acp to enter degraded state. - const degradedDeadline = Date.now() + 20_000; - while (Date.now() < degradedDeadline) { - const health = await officeApi.listRoutingHealth(officeSeed.workspaceId); - if (health.health.some((h) => h.provider_id === "claude-acp" && h.state === "degraded")) { - break; - } - await new Promise((r) => setTimeout(r, 500)); - } + await expect + .poll( + async () => { + const health = await officeApi.listRoutingHealth(officeSeed.workspaceId); + return health.health.some( + (h) => h.provider_id === "claude-acp" && h.state === "degraded", + ); + }, + { timeout: 20_000, message: "claude-acp never became degraded" }, + ) + .toBe(true); // Step 2: clear injection and call /routing/retry to fast-forward // the recovery probe. @@ -77,17 +80,15 @@ test.describe("Office provider routing — recovery", () => { // E2E mock providers register a ProviderProber wired to the // injection map, so /routing/retry flips the row without needing // a follow-up launch. - const healthyDeadline = Date.now() + 20_000; - let recovered = false; - while (Date.now() < healthyDeadline) { - const health = await officeApi.listRoutingHealth(officeSeed.workspaceId); - const claude = health.health.find((h) => h.provider_id === "claude-acp"); - if (!claude || claude.state === "healthy") { - recovered = true; - break; - } - await new Promise((r) => setTimeout(r, 500)); - } - expect(recovered).toBe(true); + await expect + .poll( + async () => { + const health = await officeApi.listRoutingHealth(officeSeed.workspaceId); + const claude = health.health.find((h) => h.provider_id === "claude-acp"); + return !claude || claude.state === "healthy"; + }, + { timeout: 20_000, message: "claude-acp never recovered to healthy" }, + ) + .toBe(true); }); }); 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 e15984f03d..bf1eda8c97 100644 --- a/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts +++ b/apps/web/e2e/tests/office/onboarding-task-launch.spec.ts @@ -90,6 +90,10 @@ 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)); } diff --git a/apps/web/e2e/tests/office/realtime-dashboard.spec.ts b/apps/web/e2e/tests/office/realtime-dashboard.spec.ts index c0f3d17fe6..530a276310 100644 --- a/apps/web/e2e/tests/office/realtime-dashboard.spec.ts +++ b/apps/web/e2e/tests/office/realtime-dashboard.spec.ts @@ -58,14 +58,17 @@ test.describe("Real-time dashboard updates", () => { // Wait for the page + initial fetches to fully settle. The dashboard // SSR + client hydration can fire late requests; give it a generous // window so we only measure fetches caused by the cross-ws event below. - let settledAt = 0; let priorFetchCount = -1; - while (settledAt < 8000) { - await testPage.waitForTimeout(1000); - settledAt += 1000; - if (fetchTimes.length === priorFetchCount) break; - priorFetchCount = fetchTimes.length; - } + await expect + .poll( + () => { + const settled = fetchTimes.length === priorFetchCount; + priorFetchCount = fetchTimes.length; + return settled; + }, + { timeout: 8_000, intervals: [1_000], message: "dashboard fetches never settled" }, + ) + .toBe(true); const baselineCount = fetchTimes.length; // Create a task in the OTHER workspace via API (fires office.task.created @@ -74,7 +77,10 @@ test.describe("Real-time dashboard updates", () => { workflow_id: otherWf.id, }); - // Wait long enough for any WS event to arrive and would-be refetch to fire. + // 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); // No additional dashboard fetches should have occurred after the event. diff --git a/apps/web/e2e/tests/office/task-chat.spec.ts b/apps/web/e2e/tests/office/task-chat.spec.ts index 3d0d298535..234f9510a4 100644 --- a/apps/web/e2e/tests/office/task-chat.spec.ts +++ b/apps/web/e2e/tests/office/task-chat.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 { waitForOfficeTaskSessionLive } from "../../helpers/office-launch"; import { AppSidebarPage } from "../../pages/app-sidebar-page"; /** @@ -30,7 +31,7 @@ const test = base.extend<{ testPage: Page }, IssueChatFixtures>({ ], chatSeed: [ - async ({ officeApi, seedData }, use) => { + async ({ officeApi, apiClient, seedData }, use) => { const result = (await officeApi.completeOnboarding({ workspaceName: "Chat Test Workspace", taskPrefix: "CT", @@ -45,28 +46,8 @@ const test = base.extend<{ testPage: Page }, IssueChatFixtures>({ throw new Error("completeOnboarding did not return a taskId"); } - // Wait for the agent to leave the pre-launch states. Task state - // surfaces from the API in canonical lowercase form - // (`in_progress`, `in_review`, `done`, …); legacy SCREAMING_SNAKE_CASE - // values are accepted defensively. - const launched = new Set([ - "in_progress", - "in_review", - "done", - "completed", - "waiting_for_input", - "review", - ]); - const deadline = Date.now() + 25_000; - while (Date.now() < deadline) { - const issue = await officeApi.getTask(result.taskId); - const raw = issue as Record; - const inner = (raw.task as Record) ?? raw; - const state = ((inner.state as string) ?? (inner.status as string) ?? "").toLowerCase(); - if (state === "failed") throw new Error("Task entered FAILED state"); - if (launched.has(state)) break; - await new Promise((r) => setTimeout(r, 500)); - } + // Wait for the agent runtime to come up before any test navigates. + await waitForOfficeTaskSessionLive(apiClient, result.taskId); await use({ workspaceId: result.workspaceId, @@ -97,16 +78,21 @@ test.describe("Office issue chat", () => { // via maybeAsync (goroutine) and needs the DB fallback for streaming // agents where Data.Text is empty. let agentComment: Record | undefined; - const deadline = Date.now() + 15_000; - while (Date.now() < deadline) { - const res = await officeApi.listTaskComments(chatSeed.taskId); - const comments = (res as { comments?: Record[] }).comments ?? []; - agentComment = comments.find( - (c) => (c.authorType as string) === "agent" || (c.author_type as string) === "agent", - ); - if (agentComment) break; - await new Promise((r) => setTimeout(r, 500)); - } + // `agentComment` is captured on each poll so the assertions below read the + // last observed value. + await expect + .poll( + async () => { + const res = await officeApi.listTaskComments(chatSeed.taskId); + const comments = (res as { comments?: Record[] }).comments ?? []; + agentComment = comments.find( + (c) => (c.authorType as string) === "agent" || (c.author_type as string) === "agent", + ); + return Boolean(agentComment); + }, + { timeout: 15_000, message: "no agent comment appeared" }, + ) + .toBe(true); expect(agentComment, "agent response must be auto-bridged as a task comment").toBeDefined(); expect((agentComment!.body as string).length).toBeGreaterThan(0);