Stabilize task lifecycle browser fixtures (#15878)

## Thinking Path > - Paperclip manages agents and their tasks. > - Browser and runner tests check task recovery and user actions. > - Some fixtures act before a prior request or run has settled. > - Those races can report a product failure before the intended assertion runs. > - This PR binds each check to the exact request, run, or geometry snapshot it needs. > - The tests retain their existing budgets and negative assertions. ## Linked Issues or Issue Description Refs: #15813, #15804. The draft stack is master → [fixture prerequisite #15878](https://github.com/paperclipai/paperclip/pull/15878) → [runtime prerequisite #15880](https://github.com/paperclipai/paperclip/pull/15880) → [Boat core #15813](https://github.com/paperclipai/paperclip/pull/15813) → [UI #15804](https://github.com/paperclipai/paperclip/pull/15804). GitHub confirms 5, 63, 97, and 75 changed files respectively, each below the 100-file limit. Existing commits and PR URLs remain intact. [Historical evidence](https://github.com/paperclipai/paperclip/tree/5909bfac9fa41405beb3fe8a65253dc28ba52835/doc/plans/assets/2026-10-10-boat) remains pinned to its original commit; current proof files stay with the UI PR. This is the test-only prerequisite extracted from the experimental Boat work. It contains five existing test-file changes. It does not add the Boat adapter or change task admission. ## What Changed - Wait for the exact signoff fixture run and each replacement run to succeed before applying the next stage decision. - Wait for the inbox Retry response for the exact failed run before navigating away. - Compare task planning time with the first run that actually started, instead of assuming the first list item started. - Read the touch target, container, and scroll position in one browser evaluation. Keep strict containment and native gesture checks. - Bind the native recovery fixture to its disposable fake Codex executable so installed provider binaries cannot override it. ## Verification Draft branch: `codex/boat-fixture-prerequisite`, commit `e318850909c0777565b43644a4d8114c6006da60`. Base: master `e733bddc45e00efb1819fffa8677f7b0d07802c5`. Five changed files. The isolated fixture-plus-runtime stack passed 24 browser cases. Six legacy recovery cases could not reach their assertions because the fixture requires embedded PostgreSQL metadata. A normal embedded PostgreSQL attempt then failed during cluster setup on this host. These are not six passing cases. No test seam, timeout increase, or product-database fixture was used. The fixture-only commit has not yet had a separate complete local browser run. All six legacy cases subsequently passed on this exact fixture commit in PR CI browser shard 8 (20/20 total, job114470999277). The original local setup failures remain recorded. The full-stack typecheck/build results belong to the assembled fixture-plus-runtime tree and are not claimed as independent qualification of this smaller commit. Review the exact run/request filters and the unchanged rejection assertions. Run the four changed Playwright specs and the native-session-resume suite through the repository's normal fixture setup. At 13:03:24 UTC, this exact head had 53 successful checks and two skips, with no pending checks or failures. All eight browser shards passed. Greptile is 5/5 on the exact head with no unresolved threads. [Exact-head legacy browser proof](https://github.com/paperclipai/paperclip/blob/9eacca2c5321bb63f061eef4bb775b6951d5846d/doc/plans/assets/2026-10-10-boat/evidence/prerequisite-legacy-browser-ci-proof.json). Feature acceptance stays in the downstream report. Deployment29 verified `05f3725af838b5a074707df1d6ed1e193bdfe49e` at 12:23:23.944 UTC. Native Claude cold execution passed at 12:39:57.782 after 14m56s of first-use preparation. Warm continuation passed at 12:42:09.943 with the same owner, boot and session identities. Normal retirement and archive followed. The ordinary saved post-idle turn then failed: its primary attempt reported the closed `LINK_COUNT` module fence at `turn_ownership_verified` (3,711ms), and same-run recovery settled failed at 12:52:09 with identity/authority errors. Natural archive completed at 12:52:21. Numeric link counts and their direction are not yet proven. A narrower failure-only diagnostic and a bounded read-only probe are under review; neither establishes a fix for this live failure. Prior staging27/28 failures remain preserved. Local complete lifecycle passes and staging Codex/computer/HMR passes remain separate source-bound evidence. No session, quarantine, provider stop or migration history was reset. These live observations do not qualify this prerequisite by themselves. ## Risks - Waiting for the wrong run could hide a task race. The fixtures match task, agent, and replacement-run identity before proceeding. - A geometry helper could weaken touch coverage. This change preserves containment and gesture assertions and only makes their inputs one snapshot. - The six legacy browser cases passed in exact-head CI; their local PostgreSQL setup limitation remains recorded. No live Boat acceptance result follows from this test-only change. Documentation impact: no product command or behavior changes. This description records fixture scope and validation limits. ## Model Used OpenAI Codex (GPT-6 family) assisted with code, test execution, and review. The host does not expose the exact primary model ID or context window. The work used tools for source inspection and local execution. Prior architecture work used `gpt-6-astra`; an independent design review used `gpt-6.1-sol`. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal issue id or instance-derived details - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
2026-10-18 05:23:39 +02:00 · 2026-10-12 07:29:15 -05:00
1 parent b189f586d2
commit 88b7bbb5e2
@@ -743,6 +743,13 @@ const recoveryFakeCodex = resolve(
      runnerDiagnostics.push(chunk.slice(-4_096));
      if (runnerDiagnostics.length > 32) runnerDiagnostics.shift();
    };
+    // The production resolver prefers installed Codex over PATH. Explicitly
+    // bind this disposable fixture so it can never invoke a real provider.
+    const codexCommandResolver = await vi.importActual<{ resolveCodexCommand: () => string }>(
+      "../../../../packages/paperclip-runner/src/drivers/codex/codex-command.js",
+    );
+    const resolveFixtureCodex = vi.spyOn(codexCommandResolver, "resolveCodexCommand")
+      .mockReturnValue(join(bin, "codex"));
    process.env.PAPERCLIP_RUNNER_STATE_DIR = stateBase;
    try {
      await Promise.all(
@@ -1227,6 +1234,7 @@ const recoveryFakeCodex = resolve(
        Promise.resolve().then(() => session.close({ reason: "Recovery fixture cleanup" })),
      ));
      const closeFailures = closed.filter((result) => result.status === "rejected");
+      resolveFixtureCodex.mockRestore();
      runnerPrpWebSocketInternals.resetForTests();
      server.closeAllConnections();
      await new Promise<void>((done) => server.close(() => done()));
@@ -124,12 +124,19 @@ test("split handoff commits relevant plans before execution and never creates ch
      const runs = await json(
        await request.get(`/api/companies/${f.company.id}/heartbeat-runs`),
      );
-      const run = runs.find(
+      const taskRuns = runs.filter(
        (run: any) => run.contextSnapshot?.issueId === task.id,
      );
-      expect(run).toBeTruthy();
+      expect(taskRuns.length).toBeGreaterThan(0);
+      const startedAt = taskRuns
+        .filter((run: any) => run.startedAt != null)
+        .map((run: any) => Date.parse(run.startedAt));
+      // A newer queued/cancelled run need not have started. Preserve the
+      // invariant against the first actual execution, not list ordering.
+      expect(startedAt.length).toBeGreaterThan(0);
+      expect(startedAt.every(Number.isFinite)).toBe(true);
      expect(Date.parse(plan.updatedAt)).toBeLessThanOrEqual(
-        Date.parse(run.startedAt),
+        Math.min(...startedAt),
      );
    }
    expect(
@@ -99,9 +99,21 @@ for (const action of ["task_retry", "thread_retry", "inbox_retry", "message", "q
      } else if (action === "message") {
        await page.getByRole("textbox", { name: "editable markdown" }).fill("Please continue the pending follow-up.");
        await page.getByRole("button", { name: "Send", exact: true }).click();
+      } else if (action === "inbox_retry") {
+        // Navigation can cancel the mutation before its request is sent.
+        const [response] = await Promise.all([
+          page.waitForResponse((response) => {
+            const request = response.request();
+            return request.method() === "POST"
+              && new URL(response.url()).pathname.endsWith(`/agents/${agent.id}/wakeup`)
+              && request.postDataJSON()?.failedRunId === sourceRunId;
+          }),
+          page.getByRole("button", { name: "Retry", exact: true }).click(),
+        ]);
+        expect(response.ok(), `${response.status()} ${await response.text()}`).toBe(true);
+        await page.goto(taskUrl);
      } else {
        await page.getByRole("button", { name: action === "thread_retry" ? "Try again" : "Retry", exact: true }).click();
-        if (action === "inbox_retry") await page.goto(taskUrl);
      }
      await expect(page.getByText("Answered the pending follow-up once.", { exact: false })).toBeVisible({ timeout: 45_000 });
      await expect(page.getByRole("status", { name: "Task recovery" })).toHaveCount(0);
@@ -21,13 +21,23 @@ async function swipeToLastOption(page: Page, picker: Locator, list = picker.getB
  }).toBeLessThanOrEqual(page.viewportSize()!.height);
  expect((await picker.boundingBox())!.y).toBeGreaterThanOrEqual(0);
  const session = await page.context().newCDPSession(page);
+  const targetHandle = await last.elementHandle();
+  expect(targetHandle).not.toBeNull();
  try {
    expect(await list.evaluate((element) => element.scrollHeight > element.clientHeight)).toBe(true);
    for (let attempt = 0; attempt < 20; attempt += 1) {
-      const bounds = (await list.boundingBox())!;
-      const target = (await last.boundingBox())!;
+      // Momentum can finish between protocol round trips. Observe geometry and
+      // scrollTop together so a stale target cannot trigger a swipe at the end.
+      const { bounds, target, before } = await list.evaluate((element, option) => {
+        const container = element.getBoundingClientRect();
+        const item = option!.getBoundingClientRect();
+        return {
+          bounds: { x: container.x, y: container.y, width: container.width, height: container.height },
+          target: { y: item.y, height: item.height },
+          before: element.scrollTop,
+        };
+      }, targetHandle);
      if (target.y >= bounds.y && target.y + target.height <= bounds.y + bounds.height) break;
-      const before = await list.evaluate((element) => element.scrollTop);
      const x = bounds.x + bounds.width / 2;
      const y = bounds.y + bounds.height - 20;
      const distance = Math.min(180, bounds.height - 40);
@@ -49,6 +59,7 @@ async function swipeToLastOption(page: Page, picker: Locator, list = picker.getB
    expect(target.y).toBeGreaterThanOrEqual(bounds.y);
    expect(target.y + target.height).toBeLessThanOrEqual(bounds.y + bounds.height + 1);
  } finally {
+    await targetHandle?.dispose();
    await session.detach();
  }
  return last;
@@ -56,6 +67,25 @@ async function swipeToLastOption(page: Page, picker: Locator, list = picker.getB

 test.use({ viewport: { width: 390, height: 844 }, hasTouch: true, isMobile: true });

+test("touch scroll observation stays valid when momentum finishes between browser reads", async ({ page, browserName }) => {
+  test.skip(browserName !== "chromium", "Native touch drags use Chromium's input protocol.");
+  await page.setContent(`<div role="dialog" style="position:absolute;top:20px;left:20px;width:300px">
+    <div role="listbox" style="height:150px;overflow-y:auto;touch-action:pan-y">
+      ${Array.from({ length: 12 }, (_, index) => `<div role="option" style="height:40px">Option ${index}</div>`).join("")}
+    </div></div>`);
+  const picker = page.getByRole("dialog"), list = picker.getByRole("listbox"), last = list.getByRole("option").last();
+  const boundingBox = last.boundingBox.bind(last);
+  // Reproduce the observed compositor interleaving: a returned target rectangle
+  // is stale by the next scrollTop read because inertia reached the list end.
+  last.boundingBox = async (options) => {
+    const beforeMomentum = await boundingBox(options);
+    await list.evaluate((element) => { element.scrollTop = element.scrollHeight - element.clientHeight; });
+    return beforeMomentum;
+  };
+  await swipeToLastOption(page, picker, list, last);
+  await expect(picker).toBeVisible();
+});
+
 test("new-task assignee, model, and project sheets scroll by touch and retain the selected values", async ({ page, request, browserName }, testInfo) => {
  test.skip(browserName !== "chromium", "Native touch drags use Chromium's input protocol.");
  const company = await json(await request.post("/api/companies", {
@@ -270,6 +270,29 @@ async function getIssueRunLockState(board: APIRequestContext, issueId: string):
  };
 }

+async function waitForFixtureRun(
+  board: APIRequestContext,
+  agentId: string,
+  issueId: string,
+  runId: string,
+) {
+  // Negative authorization cases deliberately invoke a non-participant. Leave
+  // their API rejection intact instead of waiting for a run they cannot own.
+  if ((await getIssueRunLockState(board, issueId)).assigneeAgentId !== agentId) return;
+  const waitBudgetMs = wakeupWaitBudgetMs();
+  const deadline = Date.now() + waitBudgetMs;
+  let status: string | null = null;
+  do {
+    const run = await fetchVerifiedRun(board, runId, agentId, issueId);
+    expect(run, "Fixture run must belong to the acting agent and issue").not.toBeNull();
+    status = run!.status;
+    if (status === "succeeded") return;
+    expect(["queued", "running"], `Fixture run ${runId} ended as ${status}`).toContain(status);
+    await new Promise((resolve) => setTimeout(resolve, WAKEUP_POLL_INTERVAL_MS));
+  } while (Date.now() < deadline);
+  throw new Error(`Fixture run ${runId} did not succeed within ${waitBudgetMs}ms (last status: ${status})`);
+}
+
 async function retryAgentPatchWithCurrentLockOnConflict(
  board: APIRequestContext,
  agent: AgentAuth,
@@ -286,6 +309,9 @@ async function retryAgentPatchWithCurrentLockOnConflict(
    if (issueRunLock.assigneeAgentId !== agent.agentId) return res;

    const lockedRunId = issueRunLock.checkoutRunId ?? issueRunLock.executionRunId ?? fallbackRunId;
+    // A new lock can belong to a newly promoted fixture run, not the completed
+    // run we first invoked. Let that exact verified run settle before PATCH.
+    await waitForFixtureRun(board, agent.agentId, issueId, lockedRunId);
    res = await agent.request.patch(`${BASE_URL}/api/issues/${issueId}`, {
      headers: { "X-Paperclip-Run-Id": lockedRunId },
      data: patchData,
@@ -325,6 +351,9 @@ async function agentPatch(
  }: { maxAttempts?: number; backoffMs?: number; maxBackoffMs?: number } = {},
 ) {
  const runId = await invokeHeartbeat(board, agent.agentId, issueId);
+  // The fixture process only prints "done". Let its startup and cleanup settle
+  // before a simulated decision reassigns the issue and cancels that process.
+  await waitForFixtureRun(board, agent.agentId, issueId, runId);
  const patchWith = (patchRunId: string) =>
    agent.request.patch(`${BASE_URL}/api/issues/${issueId}`, {
      headers: { "X-Paperclip-Run-Id": patchRunId },
@@ -339,6 +368,7 @@ async function agentPatch(
    // run-lock race — leave it for the caller to assert on.
    if (issueRunLock.assigneeAgentId !== agent.agentId) break;
    const retryRunId = issueRunLock.checkoutRunId ?? issueRunLock.executionRunId ?? runId;
+    await waitForFixtureRun(board, agent.agentId, issueId, retryRunId);
    res = await patchWith(retryRunId);
  }
  return res;
@@ -353,6 +383,7 @@ async function agentCheckoutAndPatch(
  patchData: Record<string, unknown>,
 ) {
  const runId = await invokeHeartbeat(board, agent.agentId, issueId);
+  await waitForFixtureRun(board, agent.agentId, issueId, runId);
  const directPatchRes = await agent.request.patch(`${BASE_URL}/api/issues/${issueId}`, {
    headers: { "X-Paperclip-Run-Id": runId },
    data: patchData,