test(e2e): replace unconditional sleeps in office specs with reactive polls (#2640)

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
João Salavisa
2026-08-13 21:38:27 +01:00
committed by GitHub
parent 73eac7ff18
commit efd0310e6c
11 changed files with 175 additions and 136 deletions
+52
View File
@@ -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<void> {
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}`);
});
}
@@ -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<string, unknown>;
const inner = (raw.task as Record<string, unknown>) ?? 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,
@@ -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();
});
+13 -10
View File
@@ -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 = {
@@ -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);
});
@@ -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 }>;
@@ -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,
@@ -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);
});
});
@@ -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));
}
@@ -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.
+19 -33
View File
@@ -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<string, unknown>;
const inner = (raw.task as Record<string, unknown>) ?? 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<string, unknown> | undefined;
const deadline = Date.now() + 15_000;
while (Date.now() < deadline) {
const res = await officeApi.listTaskComments(chatSeed.taskId);
const comments = (res as { comments?: Record<string, unknown>[] }).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<string, unknown>[] }).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);