test: stabilize session resume status assertion (#2572)

2026-08-12 19:13:05 +01:00
parent 8ca01e0f7e
commit d616a1fcda
5 changed files with 230 additions and 3 deletions
@@ -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));

@@ -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({
@@ -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.
@@ -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.
@@ -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