From d616a1fcda5f4ae9df1e3008ca0fe6a2a5e6418b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Wed, 12 Aug 2026 19:13:05 +0100 Subject: [PATCH] test: stabilize session resume status assertion (#2572) --- .../components/github/pr-detail-panel.test.ts | 2 - .../e2e/tests/session/session-resume.spec.ts | 24 +++- .../session-resume-test-readiness/plan.md | 90 +++++++++++++++ .../task-01-session-resume-readiness.md | 107 ++++++++++++++++++ .../agent-resume-runtime-recovery/spec.md | 10 ++ 5 files changed, 230 insertions(+), 3 deletions(-) create mode 100644 docs/plans/session-resume-test-readiness/plan.md create mode 100644 docs/plans/session-resume-test-readiness/task-01-session-resume-readiness.md diff --git a/apps/web/components/github/pr-detail-panel.test.ts b/apps/web/components/github/pr-detail-panel.test.ts index c3fd6df125..8c17880e30 100644 --- a/apps/web/components/github/pr-detail-panel.test.ts +++ b/apps/web/components/github/pr-detail-panel.test.ts @@ -503,9 +503,7 @@ describe("PRDetailContent persistent request expiry", () => { feedbackMocks.value = makeFeedback({ reviews: [dismissedReview()] }); reviewMocks.requestReviewers.mockResolvedValue({ requested: true }); renderPRDetail(); - const timersBeforeRequest = vi.getTimerCount(); await act(async () => fireEvent.click(screen.getByTestId(RE_REQUEST_BUTTON))); - expect(vi.getTimerCount()).toBe(timersBeforeRequest + 1); act(() => vi.advanceTimersByTime(5 * 60 * 1000)); diff --git a/apps/web/e2e/tests/session/session-resume.spec.ts b/apps/web/e2e/tests/session/session-resume.spec.ts index b3ac41c70d..39dc4d7a08 100644 --- a/apps/web/e2e/tests/session/session-resume.spec.ts +++ b/apps/web/e2e/tests/session/session-resume.spec.ts @@ -4,6 +4,7 @@ import path from "node:path"; import { test, expect } from "../../fixtures/test-base"; import type { ApiClient } from "../../helpers/api-client"; +import { waitForSessionState } from "../../helpers/session"; import { KanbanPage } from "../../pages/kanban-page"; import { SessionPage } from "../../pages/session-page"; import type { Page } from "@playwright/test"; @@ -127,7 +128,7 @@ test.describe("Task status during resume", () => { // 1. Create task and start agent — after the turn completes the workflow // advances it from "Running" to "Turn Finished". - await apiClient.createTaskWithAgent( + const task = await apiClient.createTaskWithAgent( seedData.workspaceId, "Status Stable Task", seedData.agentProfileId, @@ -138,6 +139,8 @@ test.describe("Task status during resume", () => { repository_ids: [seedData.repositoryId], }, ); + const sessionId = task.session_id; + if (!sessionId) throw new Error("createTaskWithAgent did not return a session_id"); // 2. Navigate to the session and wait for the agent to finish its first turn const session = await openTaskSession(testPage, "Status Stable Task"); @@ -145,6 +148,13 @@ test.describe("Task status during resume", () => { timeout: 30_000, }); await session.waitForChatIdle({ timeout: 15_000 }); + await waitForSessionState(apiClient, { + taskId: task.id, + sessionId, + expectedState: "WAITING_FOR_INPUT", + message: "Initial session did not settle before the Turn Finished assertion", + timeout: 30_000, + }); // 3. Confirm the task moved to the "Turn Finished" section after the turn completed await expect(session.taskInSection("Status Stable Task", "Turn Finished")).toBeVisible({ @@ -172,6 +182,18 @@ test.describe("Task status during resume", () => { // 7. Wait for auto-resume to complete (agent relaunches and becomes idle) await session.waitForChatIdle({ timeout: 60_000 }); + // The idle composer can briefly survive the reload before auto-resume starts. + // The durable boot message proves that the resumed agent actually relaunched. + await expect(session.chat.getByText("Resumed agent Mock", { exact: false })).toBeVisible({ + timeout: 15_000, + }); + await waitForSessionState(apiClient, { + taskId: task.id, + sessionId, + expectedState: "WAITING_FOR_INPUT", + message: "Resumed session did not settle before the final Turn Finished assertion", + timeout: 30_000, + }); // 8. After resume completes, the task must still be in "Turn Finished" await expect(session.taskInSection("Status Stable Task", "Turn Finished")).toBeVisible({ diff --git a/docs/plans/session-resume-test-readiness/plan.md b/docs/plans/session-resume-test-readiness/plan.md new file mode 100644 index 0000000000..360a19e741 --- /dev/null +++ b/docs/plans/session-resume-test-readiness/plan.md @@ -0,0 +1,90 @@ +--- +spec: docs/specs/agent-resume-runtime-recovery/spec.md +created: 2026-08-12 +status: completed +--- + +# Implementation Plan: Session resume test readiness + +## Overview + +Harden the session-resume E2E regression by synchronizing the first finished-turn +assertion with the persisted session lifecycle instead of treating a visible +agent response or idle composer as lifecycle completion. Keep the backend-restart +and UI assertions intact so the test still verifies transcript persistence, +automatic resume, and the final Turn Finished bucket. + +## Root cause + +The mock response is persisted before the session reaches `WAITING_FOR_INPUT`. +`SessionPage.waitForChatIdle()` observes the composer and can return during that +interval, while the sidebar still classifies the task from the prior session +state. Under CI shard load, the existing 15-second locator wait expires before +the state publication reaches the sidebar. The CI error context showed the +response and Review step, but no finished-state icon on the sidebar row. + +## E2E test synchronization + +- In `apps/web/e2e/tests/session/session-resume.spec.ts`, retain the task + returned by `apiClient.createTaskWithAgent` so the test can query its exact + session. +- After the first response and `waitForChatIdle`, poll + `apiClient.listTaskSessions(task.id)` until the created session is + `WAITING_FOR_INPUT`. This is the lifecycle readiness condition used by the + sidebar classifier; do not add a fixed sleep or increase a locator timeout. +- Keep the finished-turn assertion as a UI assertion after the lifecycle poll. + Keep the backend restart, reload, transcript, no-Backlog/no-Running, and + post-resume UI assertions unchanged in intent. +- Before the final post-resume Turn Finished assertion, use the same exact + session-state readiness condition so the final UI check cannot race a stale + session snapshot. + +## Tests + +- **Scenario:** A completed mock turn persists its response before the session + lifecycle settles. + **File:** `apps/web/e2e/tests/session/session-resume.spec.ts`. + **How:** Create a task through the existing fixture API, poll the exact + session state to `WAITING_FOR_INPUT`, then assert the task row is in the + Turn Finished bucket. +- **Scenario:** A backend restart resumes a review-state task. + **File:** `apps/web/e2e/tests/session/session-resume.spec.ts`. + **How:** Reload after restart, assert the persisted transcript and immediate + review placement, wait for the durable `Resumed agent Mock` boot message to + prove automatic resume ran, poll the session back to `WAITING_FOR_INPUT`, and + assert the task remains Turn Finished without Backlog or Running placement. + +## Verification Results + +- `cd apps/web && pnpm e2e:run tests/session/session-resume.spec.ts -- --grep + "task stays in Turn Finished section after backend restart and agent resume" + --retries=0 --workers=1` — passed, 1 test in 8.3s after the review fix. +- `cd apps/web && pnpm e2e:docker --no-build -- --repeat-each=4 --workers=1 + --retries=0 tests/session/session-resume.spec.ts:121` — passed, 4 tests in + 31.2s after the review fix. +- `cd apps/web && pnpm run typecheck` — passed. +- `cd apps/web && pnpm exec prettier --check + e2e/tests/session/session-resume.spec.ts` — passed. +- `git diff --check` — passed. +- The managed E2E runner's generated artifacts were cleaned and no generated + files remain in the worktree. +- Review fixup: the post-restart assertion now requires the durable + `Resumed agent Mock` boot message before polling back to `WAITING_FOR_INPUT`. + +## Implementation Waves And Parallel Candidates + +Wave 1: + +- [x] [Task 01: Harden session-resume readiness](task-01-session-resume-readiness.md) + +The task is sequential. It edits one E2E spec and must be verified against the +managed production-build runner and the CI-style repeated isolated worker. + +## Risks and out of scope + +- The API poll must target the created task's session, not an arbitrary session + in the worker, or a parallel-session regression could be hidden. +- The test must continue to assert the UI. API state is only a readiness signal, + not a replacement for the user-visible assertion. +- No production lifecycle, sidebar classifier, timeout policy, or backend API + behavior changes are planned. diff --git a/docs/plans/session-resume-test-readiness/task-01-session-resume-readiness.md b/docs/plans/session-resume-test-readiness/task-01-session-resume-readiness.md new file mode 100644 index 0000000000..fc8bfd0b29 --- /dev/null +++ b/docs/plans/session-resume-test-readiness/task-01-session-resume-readiness.md @@ -0,0 +1,107 @@ +--- +id: "01-session-resume-readiness" +title: "Harden session-resume readiness" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/agent-resume-runtime-recovery/spec.md" +--- + +# Task 01: Harden session-resume readiness + +Replace the flaky lifecycle assumption in the existing session-resume E2E test +with a bounded poll of the created session's persisted state. Keep the UI +assertions that prove the task remains in the Turn Finished bucket across a +backend restart and automatic resume. + +## Acceptance + +- The test waits for the created session to reach `WAITING_FOR_INPUT` before + asserting the initial Turn Finished row, without fixed sleeps or larger + locator timeouts. +- The test still verifies transcript persistence, post-restart review state, + absence from Backlog and Running, and final Turn Finished placement through + the UI. +- The focused test passes with retries disabled under the managed E2E runner and + under four CI-style repetitions in one isolated worker. + +## Files likely touched + +- `apps/web/e2e/tests/session/session-resume.spec.ts` + +## Dependencies + +None. + +## Parallelism + +`sequential`. + +## Inputs + +- Spec scenario: completed-turn response persistence and backend restart resume. +- Plan section: **E2E test synchronization**. +- `apps/web/e2e/references/ui-state-and-cleanup.md`: agent output is not + lifecycle readiness; poll the exact session state before follow-up assertions. +- `apps/web/e2e/helpers/api-client.ts`: `createTaskWithAgent` and + `listTaskSessions` response shapes. + +## TDD sequence + +1. Preserve the CI failure evidence as the RED signal: the existing test can + render the response and idle composer while the sidebar row has no finished + icon. +2. Add the exact-session `WAITING_FOR_INPUT` readiness poll. +3. Require the durable `Resumed agent Mock` boot message before the post-restart + poll so the assertion proves automatic resume ran. +4. Run the focused test with retries disabled, then run the repeated isolated + worker command. +5. Re-run the final changed test after any formatting or selector adjustment. + +## Verification + +Run from `apps/web` after the workspace dependencies are installed: + +```bash +pnpm e2e:run tests/session/session-resume.spec.ts -- --grep 'task stays in Turn Finished section after backend restart and agent resume' --retries=0 --workers=1 +pnpm e2e:docker --no-build -- --repeat-each=4 --workers=1 --retries=0 tests/session/session-resume.spec.ts:121 +``` + +The managed runner must rebuild the production Go-served Vite assets before the +first command. Confirm each command discovers the intended test and leaves no +generated E2E artifacts in the diff. + +## Risks + +- A poll over `sessions[0]` could observe the wrong session. Match the created + session ID when it is available and fail clearly if it is missing. +- The post-restart `WAITING_FOR_INPUT` poll must follow the durable resumed-agent + boot message, or it could accept the pre-restart idle state. +- Polling only the API could hide a frontend hydration issue. Keep the existing + sidebar locator assertions as the user-visible proof. + +## Output contract + +Report the exact files changed, the bounded poll condition, RED evidence, both +verification commands and outcomes, generated-artifact cleanup, and any +remaining CI-shard risk. Update this task's `status` and `## Results`, then +synchronize the plan checkbox and `## Verification Results`. + +## Results + +- Added an exact-session poll for `WAITING_FOR_INPUT` before the initial + Turn Finished assertion and again after automatic resume. +- Added a durable `Resumed agent Mock` boot-message assertion before the + post-restart poll, so the final state check cannot accept the pre-restart + idle state. +- Preserved the transcript, restart, Backlog, Running, and final Turn Finished + UI assertions. +- The original CI failure showed the response and idle composer before the + sidebar received the settled lifecycle state. The new poll synchronizes the + UI assertion with that persisted state instead of adding a sleep or larger + locator timeout. +- Focused managed E2E run after review fix: passed, 1 test in 8.3s. +- Four-repeat single-worker E2E run after review fix: passed, 4 tests in 31.2s. +- Web typecheck, Prettier check, and `git diff --check`: passed. +- No generated E2E artifacts remain in the worktree. diff --git a/docs/specs/agent-resume-runtime-recovery/spec.md b/docs/specs/agent-resume-runtime-recovery/spec.md index 7859ab177a..923d1d6c01 100644 --- a/docs/specs/agent-resume-runtime-recovery/spec.md +++ b/docs/specs/agent-resume-runtime-recovery/spec.md @@ -43,6 +43,11 @@ without repairing it. - If request assembly, credential issuance, or launch fails after that early transition, Kandev restores the prior recoverable session state unless another terminal transition won the race. +- A completed turn remains represented by the task's review state while its + persisted response and session lifecycle state settle. After a backend + restart and automatic resume, the prior transcript remains visible and the + task returns to the Turn Finished review bucket once the session is again + `WAITING_FOR_INPUT`; it does not settle in Backlog or Running. - The explicit managed-runtime update path may invalidate only the deterministic `_npx` execution directory for the selected built-in package after an initial update failure, then retry once and run the normal ACP @@ -83,6 +88,11 @@ without repairing it. - **GIVEN** that request construction, credential issuance, or launch fails after the early `STARTING` transition, **WHEN** recovery handling completes, **THEN** the session is recoverable and no stale `STARTING` state remains. +- **GIVEN** a task whose completed-turn response is persisted before its + session lifecycle reaches its settled state, **WHEN** the backend restarts + and the task page reloads, **THEN** the prior transcript remains visible and, + after automatic resume settles at `WAITING_FOR_INPUT`, the task is shown in + the Turn Finished review bucket rather than Backlog or Running. - **GIVEN** an extracted managed npm runtime is corrupt, **WHEN** the first explicit update attempt fails, **THEN** only that package's deterministic execution directory is invalidated, the update runs once more, and success