From b1a71044afd0fea976c2e30bf68b623c70b8a211 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:40:58 -0600 Subject: [PATCH] fix(evals): observe chat handoff replies and restarted task pages (#19073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The Product regression TestOps plan runs 20 Product E2E cells through `runner-full-stack-e2e.yml` > - The full 20-case TestOps launch on `master` 4740d251 passed 13 of 20 > - Four failures came from the eval's observation, not from the product > - Three Agent Chat handoff cases raced the product's completed-handoff reply turn, and the run-count contract treated that reply as an unexpected run > - One continuation case waited for `DOMContentLoaded` after a server restart, which the dev server can leave pending while the page is interactive > - This pull request makes the eval observe those two behaviors explicitly > - The benefit is that these cases measure the product again, and the handoff cases now also prove that the result returns to the chat ## Linked Issues or Issue Description Refs #14408, #19053 **What happened?** TestOps launch https://paperclip.testops.cloud/launch/2694 (GitHub run https://github.com/paperclipai/paperclip/actions/runs/41915965118) failed these cells: - `agent-chat` `multi-repository`, `clarify-reuse`, `plan-handoff`: "Declared total provider-run bounds". Each final run ledger has exactly one extra run. That run is the source chat's `chat_task_completed` turn, with `chatCompletionUpdates` naming the finished task. - `continuation` `completed-action-resume`: `page.goto: Timeout 30000ms exceeded` waiting for `domcontentloaded` right after the deliberate server restart. The server log shows the page loaded and called the API during that wait. **Expected behavior** A completed handoff returns to the source chat (#14408). The eval must observe that reply, require it, and count it. After a restart, the eval must wait for the rendered task, not for a browser load event. **Steps to reproduce** Run the cells listed above on `master` with the Product regression TestOps lane. ## What Changed - `chat-flow.ts`: after the delegated task is `done`, the handoff branch waits for the source chat's completion-reply turn (`chat_task_completed`, correlated by `chatCompletionUpdates[].id`). It requires exactly one such turn, `succeeded`, and exactly one agent comment created by that turn. - `chat-cases.ts`: the declared run counts include that one reply turn: `multi-repository` 2 → 3, `clarify-reuse` 3 → 4, `plan-handoff` 4 → 5. The minimum and maximum stay equal, so a missing reply or an extra run still fails. - `catalog.test.ts`: the catalog run-count total changes from 385 to 397 for these three cases across their profiles. - `continuation-flow.ts`: `open()` uses `waitUntil: "commit"` with a 60 s navigation limit, then requires the rendered task thread (`waitForTaskChatRendered`). This matches the existing restart handling in `chat-flow.ts`. ## Verification - `npx vitest run --config tests/runner-e2e/vitest.config.ts tests/runner-e2e/chat-flow.test.ts tests/runner-e2e/continuation.test.ts tests/runner-e2e/catalog.test.ts tests/runner-e2e/kimi-qualification.test.ts tests/runner-e2e/non-execution-wake.test.ts tests/runner-e2e/product-regression-testops-catalog.test.ts tests/runner-e2e/lifecycle-live.test.ts`: all pass. - `tsc -p tests/runner-e2e/tsconfig.json`: no errors in `tests/runner-e2e`. - The retained ledgers from run 41915965118 show the reply turn: `multi-repository` 3 runs, `clarify-reuse` 4 runs, `plan-handoff` 5 runs, each with one `chat_task_completed` run on the chat issue. - Live single-case TestOps runs against this branch: see the PR comments. ## Risks - Changed eval conditions: the three handoff cases now also require the completion reply. A product regression that drops the reply now fails these cases, as intended. - The continuation navigation allowance changes from 30 s (`domcontentloaded`) to 60 s (`commit`) plus a rendered-thread check. That is an observation deadline, not a product latency requirement. The rendered-thread check is stronger than the old load event. - Three other plan failures are not changed here. They are product failures: the agent creates `human_only` confirmations, and `resolve-from-comment` then refuses to record the user's explicit text approval (`interaction_human_only`). ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use and code execution. ## 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 Paperclip ticket id or instance-derived details - [x] 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 --- tests/runner-e2e/catalog.test.ts | 2 +- tests/runner-e2e/chat-cases.ts | 7 ++++--- tests/runner-e2e/chat-flow.ts | 29 ++++++++++++++++++++++++++- tests/runner-e2e/continuation-flow.ts | 7 +++++-- 4 files changed, 38 insertions(+), 7 deletions(-) diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 969b43626cc..6a37e2ab92f 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -357,7 +357,7 @@ describe("runner E2E catalog", () => { (total, execution) => total + execution.task.expectedRunCount, 0, ), - ).toBe(385); + ).toBe(397); expect( runnerTasks.find((task) => task.id === "plan-revise-accept") ?.attemptTimeoutMs, diff --git a/tests/runner-e2e/chat-cases.ts b/tests/runner-e2e/chat-cases.ts index 50d22658b4e..3684c003be2 100644 --- a/tests/runner-e2e/chat-cases.ts +++ b/tests/runner-e2e/chat-cases.ts @@ -15,9 +15,10 @@ export const CHAT_CASES = [ ["continuity-restart", "Conversation continuity across restart", 3], ["new-session", "Fresh context within preserved history", 2], ["stop-new-resume", "Stop, reset, and resume", 3], - ["plan-handoff", "Draft, revise, approve, and hand off a plan", 4], - ["clarify-reuse", "Clarify and reuse an existing project", 3], - ["multi-repository", "Create a project with multiple repository URLs", 2], + // Handoff cases include the source chat's one completion-reply turn. + ["plan-handoff", "Draft, revise, approve, and hand off a plan", 5], + ["clarify-reuse", "Clarify and reuse an existing project", 4], + ["multi-repository", "Create a project with multiple repository URLs", 3], ] as const; export type ChatCase = (typeof CHAT_CASES)[number][0]; const HARDENING_CASES = [ diff --git a/tests/runner-e2e/chat-flow.ts b/tests/runner-e2e/chat-flow.ts index a5a76a5a5ff..0a8808fb826 100644 --- a/tests/runner-e2e/chat-flow.ts +++ b/tests/runner-e2e/chat-flow.ts @@ -183,6 +183,12 @@ export async function readChatOutputDocument( export const isResetRun = (run: ChatRun) => run.contextSnapshot?.conversationReset === true || run.resultJson?.conversationReset === true; +/** The source chat's automatic turn that reports one completed handoff. */ +export const isChatCompletionReply = (run: ChatRun, chatIssueId: string, taskId: string) => + run.contextSnapshot?.issueId === chatIssueId && + run.contextSnapshot?.wakeReason === "chat_task_completed" && + Array.isArray(run.contextSnapshot?.chatCompletionUpdates) && + (run.contextSnapshot.chatCompletionUpdates as Array<{ id?: unknown }>).some((update) => update?.id === taskId); export function chatRunFailure( runs: ChatRun[], allowCancelled = false, @@ -944,7 +950,28 @@ export async function runChatFlow(input: ChatFlowInput) { accept: (state) => state.task.status === "done", reject: (state) => chatTaskCompletionFailure(state.task, state.runs), }); - runs = await allRuns(); + // A completed handoff returns to the source chat (#14408) as one + // correlated provider turn. Observe that delivery instead of racing it. + const completionReply = await pollUntil({ + label: `completion of ${child.id} is reported in the chat`, + deadlineAt: Date.now() + 240_000, + intervalMs: 1000, + load: async () => { + const rows = await allRuns(); + const reply = rows.filter((run) => isChatCompletionReply(run, issue!.id, child.id)); + return { runs: rows, reply, comments: await comments() }; + }, + // A native reply comment is presented after its run succeeds, so wait + // for the comment too; a failed reply turn settles immediately. + accept: (state) => state.reply.length > 0 && + state.reply.every((run) => ["succeeded", "failed", "cancelled", "timed_out"].includes(run.status)) && + (state.reply.some((run) => run.status !== "succeeded") || + state.comments.some((c) => c.authorAgentId === f.agent.id && state.reply.some((run) => run.id === c.createdByRunId))), + }); + expect(completionReply.reply.map((run) => run.status), "one completion reply turn").toEqual(["succeeded"]); + expect(completionReply.comments.filter((c) => c.authorAgentId === f.agent.id && + c.createdByRunId === completionReply.reply[0]!.id), "completion reply posted in the chat").toHaveLength(1); + runs = completionReply.runs; input.observe(issue!, runs); const plan = caseId === "plan-handoff" diff --git a/tests/runner-e2e/continuation-flow.ts b/tests/runner-e2e/continuation-flow.ts index df999ff2537..06a8b3f6f12 100644 --- a/tests/runner-e2e/continuation-flow.ts +++ b/tests/runner-e2e/continuation-flow.ts @@ -12,7 +12,7 @@ import { answerableRuntimeRunIds, isSingleClaudeQuestion } from "./runtime-quest import { expect, type Page } from "@playwright/test"; import path from "node:path"; import { continuationAnswerCommitted, continuationInitialReady, continuationCheckpointReady } from "./continuation-readiness.js"; -import { captureLoadedContinuation } from "./continuation-screenshot.js"; +import { captureLoadedContinuation, waitForTaskChatRendered } from "./continuation-screenshot.js"; import { seedContinuationContext } from "./continuation-workspace.js"; import { pollUntil, type RunnerApi } from "./api.js"; import { @@ -130,10 +130,13 @@ export async function runContinuationFlow(input: { }); } async function open() { + // A restarted dev server can leave DOMContentLoaded pending after the app + // is interactive (see chat-flow). Require the rendered task thread instead. await page.goto( `/${fixtures.company.issuePrefix}/issues/${issue!.identifier ?? issue!.id}`, - { waitUntil: "domcontentloaded" }, + { waitUntil: "commit", timeout: 60_000 }, ); + await waitForTaskChatRendered(page, undefined, 60_000); } async function snapshot(phase: ContinuationCheckpoint["phase"]) { const [tasks, summaries, comments, interactions, attachments, activity] =