From 846336e5a0d3003e1b938e8c983aa5a65bda81ff Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Mon, 21 Sep 2026 16:18:06 -0500 Subject: [PATCH] test: harden agent chat setup, interruptions and restart evals (#13762) ## Thinking Path > - Paperclip lets people manage agents through ongoing conversations. > - Chat users can change instructions while a provider is already working. > - Existing chat evals wait for each turn to settle before the next message. > - They cannot prove delivery during active work or the saved effect of a correction. > - Existing fixtures also enable Agent Chat through the API rather than the settings UI. > - This PR adds bounded browser workflows and checks their persisted outcomes. ## Linked Issues or Issue Description Refs #13741, #13752, #13750. **What happened?** The chat suites cover planning, delegation, status, and recovery. They lack active-turn follow-ups and the experimental settings lifecycle. A sequential conversation can pass even if messages sent during work are lost. **Expected behavior** A follow-up submitted during a provider turn survives and affects the final reply. A changed launch day appears in the saved plan. Disabling Agent Chat rejects new messages while preserving history; re-enabling resumes the same conversation. **Steps to reproduce** Run the explicit `agent-chat-stories` suite. It selects three local cases for each native Claude and Codex profile. An ordinary provider command waits for a fixture brief file so the browser can send the follow-up at an observed active-run boundary. ## What Changed - Add six opt-in Product E2E cells for settings, active follow-ups, and plan corrections. - Drive experimental settings through the UI and verify disabled sends are rejected by the public API. - Use a bounded file wait in the actual isolated agent workspace, with provider-written readiness and an undisclosed brief reference. - Grade persisted user messages, final replies, native run outcomes, and exact saved plan fields. - Accept active-turn steering or one queued successor; reject lost input, duplicate input, and stale outputs. - Allow one steered run or two sequential runs throughout the shared harness, while preserving exact counts for other cases. - Require a single marker-bearing response attributed to the final provider run. - Unload the development browser client before restarting the server, avoiding reconnect/navigation races without weakening the post-restart memory check. - Add browser regressions for restart isolation and asynchronously saved settings switches. - Document prepared-agent setup, native onboarding limits, and the separate API-tool rollout gate. ## Verification - Eval TypeScript check passed. - Eval support suite: 436 tests passed in 39 files. - New oracle calibration: six tests passed, including plausible invalid outcomes. - Browser support regressions: seven tests passed; the restart regression was observed failing before the fix. - Catalog discovery selects exactly six local native cases and leaves default paid selection unchanged. - [Consolidated existing native chat report](https://d1p6rlowie26tp.cloudfront.net/runner-e2e/campaigns/gha-35643286055-1/): master `b82661b56`, 33/34 passed, all cleanup passed. The failure was a browser navigation timeout across restart; the page request returned 200 and the chat rendered. - [Nine targeted restart/replay cells](https://d1p6rlowie26tp.cloudfront.net/runner-e2e/campaigns/gha-35645850088-1/) passed on `1fe2fe275`, including the original failure, across native Claude/Codex and local/Daytona; all cleanup passed. - [Initial six-story campaign](https://github.com/paperclipai/paperclip/actions/runs/35644832817) retained all six failures: asynchronous switch assertions, unavailable fixture paths, and rich-text escaping in raw command comparisons. The corrected fixtures preserve the same behavioral assertions. - [Six-story campaign v2](https://d1p6rlowie26tp.cloudfront.net/runner-e2e/campaigns/gha-35646270035-1/) on `8232773a0`: 4/6 passed (both settings cases and both Claude interruptions). Codex could not see the host-temp fixture outside its workspace; this failed before follow-up delivery was exercised. - [Four affected interruption cases](https://d1p6rlowie26tp.cloudfront.net/runner-e2e/campaigns/gha-35647760635-1/) all passed, including cleanup, on definition v3 / `ad6ac0545`. Files live inside the actual agent workspace and the observed run workspace is verified. Both providers saved Friday in the real plan with the undisclosed brief reference; follow-ups persisted while the original run was active. Together with both unchanged settings cases from v2, all six new scenario variants have passing live evidence. - Final head `ad6ac05456646c09d3452e320279457625353948`: 54 successful checks, two intentional skips, zero pending/failing checks; mergeable and clean. Fresh Greptile 5/5, zero unresolved findings. - Full typecheck, tests, build, and browser CI passed remotely. One earlier head encountered a signoff-policy browser timing failure; the final head passed that shard. - Local pnpm wrapper could not fetch its version/signature metadata in the restricted environment; local eval checks used the installed Node executables. Repo-wide validation was completed by GitHub Actions. ## Risks These are eval-only changes. The file wait is a timing fixture in the isolated agent workspace, not a production runner hook. Native Codex host-filesystem isolation stays unchanged. It has a two-minute limit and is released in `finally`. The prepared-agent settings case is not full native onboarding: the wizard currently offers legacy adapters. The disabled-entry assertion uses full document navigation, which clears the prior React Query cache; preserved history is checked through the public API and re-enabled chat. No production prompt, rollout default, adapter behavior, or credential policy changes. Active-task reassignment and worker-crash recovery remain outside these new cases. ## Model Used OpenAI Codex, GPT-6, with repository tools and code execution. The exact deployment model ID and context window are not exposed in this task. ## 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 - [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 --- doc/evals.md | 6 + tests/runner-e2e/FIXTURES.md | 12 ++ tests/runner-e2e/README.md | 31 +++- tests/runner-e2e/catalog.test.ts | 6 +- tests/runner-e2e/catalog.ts | 12 +- tests/runner-e2e/chat-cases.ts | 5 + tests/runner-e2e/chat-flow.ts | 19 ++- tests/runner-e2e/chat-restart.spec.ts | 33 ++++ tests/runner-e2e/chat-restart.ts | 7 + tests/runner-e2e/chat-stories.test.ts | 73 ++++++++ tests/runner-e2e/chat-stories.ts | 160 ++++++++++++++++++ tests/runner-e2e/playwright-support.config.ts | 2 +- tests/runner-e2e/run-count.test.ts | 11 ++ tests/runner-e2e/run-count.ts | 6 + tests/runner-e2e/runner.spec.ts | 12 +- tests/runner-e2e/settings-toggle.spec.ts | 11 ++ tests/runner-e2e/settings-toggle.ts | 8 + tests/runner-e2e/types.ts | 2 + 18 files changed, 398 insertions(+), 18 deletions(-) create mode 100644 tests/runner-e2e/chat-restart.spec.ts create mode 100644 tests/runner-e2e/chat-restart.ts create mode 100644 tests/runner-e2e/chat-stories.test.ts create mode 100644 tests/runner-e2e/chat-stories.ts create mode 100644 tests/runner-e2e/run-count.test.ts create mode 100644 tests/runner-e2e/run-count.ts create mode 100644 tests/runner-e2e/settings-toggle.spec.ts create mode 100644 tests/runner-e2e/settings-toggle.ts diff --git a/doc/evals.md b/doc/evals.md index 842422bf1c7..45a196b3461 100644 --- a/doc/evals.md +++ b/doc/evals.md @@ -66,6 +66,12 @@ Daytona paths. Its [fixture contract](../tests/runner-e2e/README.md) distinguish startup cancellation from active response cancellation and HTTP send replay from ambiguous provider action recovery. Select it explicitly; `--all` excludes it. +The explicit-only `agent-chat-stories` suite covers the experimental settings +lifecycle for a configured native agent and follow-ups during active work. Its +fixture-driven file wait and persisted-plan oracle are documented in the +[Product E2E guide](../tests/runner-e2e/README.md). It does not qualify the native +onboarding wizard or change the native API-tool rollout defaults. + ## Validation ladder Start with credential-free checks and a catalog listing. For Product E2E: diff --git a/tests/runner-e2e/FIXTURES.md b/tests/runner-e2e/FIXTURES.md index 9b1acf83510..27ebc6362b7 100644 --- a/tests/runner-e2e/FIXTURES.md +++ b/tests/runner-e2e/FIXTURES.md @@ -195,3 +195,15 @@ The lost-acknowledgement probe may interrupt only the fixture browser's own comment request after the real server has committed it. Retain its request ID and replay that same request through the public API after restarting the server. Never fabricate tool results or repair task state after a failed assertion. + +`chat-stories.ts` seeds an ordinary file wait in the isolated agent's actual +home workspace; native Codex intentionally cannot see arbitrary host temp files. +The observed run workspace must match the fixture location. This is a deterministic interruption +boundary. The real provider command writes the readiness file and waits at most +two minutes. The harness must persist the next browser message while the same +run is active before supplying the brief. Always release the wait in `finally`. +Save boundary observations independently of the final outcome. The final answer +must recover a brief reference absent from both prompts; the revision oracle +also reads the actual conversation plan. Fixture setup never enables native API +tools for this suite. Do not describe its prepared-agent settings case as a +production onboarding qualification. diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index c8006b22e25..b85da4dd96c 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -83,7 +83,7 @@ pnpm test:e2e:runner -- --suite daytona-warm-continuity pnpm test:e2e:runner -- --all ``` -The catalog contains eight suites, including the explicit-only everyday suite. `core-compatibility` (**Core Runner +The catalog contains nine suites, including the explicit-only suites. `core-compatibility` (**Core Runner Compatibility**) is seven major runner profiles × local/Daytona × three workflows: 42 cells. Its cases are: @@ -176,6 +176,9 @@ fails the case instead of silently testing another phase. Restart continuity requires the agent to recall a phrase after the server restarts. The final prompt does not reveal that phrase. A generic successful reply after restart cannot pass this check. +The browser leaves the old development client before the server stops, then +opens the canonical chat route and waits for the composer. This avoids racing +Vite's automatic reconnect navigation against the test's explicit navigation. The blocker query requests a JSON status snapshot. It must name the current recorded blocker and report zero active runs independently of the task's blocked @@ -188,6 +191,29 @@ requires the original comment, task, plan, and single consuming run. This proves HTTP request idempotency across restart, not replay safety for an ambiguous provider tool response. Existing native tool-receipt tests cover that boundary. +`agent-chat-stories` adds six explicit-only local cells across native Codex and +Claude. `enable-disable-resume` uses the Experimental settings UI to enable +Agent Chat, starts a conversation, disables new messages, verifies the public +write endpoint rejects a send without creating work, and re-enables the same +conversation with its remembered context. The company, credential, and native +agent are fixture-provisioned. This qualifies the experimental-settings path, +not native first-run onboarding: the current production wizard offers legacy +adapters, and native API tools remain an independent opt-in. + +`followup-while-running` and `revise-while-running` send a second browser message +while the provider runs a bounded command waiting for a fixture brief file. +The command publishes its own readiness file; the harness verifies the original +run is still active after the follow-up is saved, then supplies the brief. +The final reply must contain the previously undisclosed brief reference and the +new request's marker. The revision case also checks the saved plan uses Friday +instead of the original Monday. The oracle permits either steering the active +run or one queued successor, but rejects missing/duplicate comments, failed or +unfinished runs, stale plan contents, and unintended tasks/projects. This does +not qualify active-task reassignment or worker-crash recovery. +The maximum run count remains the cost estimate; the shared harness honors the +one-run minimum only for these two interruption cases. Exactly one reply may +consume the follow-up marker, and it must be attributed to the final provider run. + ```sh pnpm test:e2e:runner -- --list --suite agent-chat-hardening pnpm test:e2e:runner -- --id agent-chat-hardening.runner-codex.local.stop-startup-new-resume @@ -224,7 +250,8 @@ Missing provider credentials fail paid preflight and are not passing coverage. The default `--all` selection is 171 cells (148 local and 23 Daytona) and 371 expected paid agent turns. The explicit-only everyday suite adds 38 catalog cells -and chat hardening adds 18. Both are excluded from `--all`. The full catalog has 227 cells. +and chat hardening adds 18; chat stories adds six. All three are excluded from +`--all`. The full catalog has 233 cells. Follow-up steps remain ordered within their cell; all other cells are independent. Narrow selectors are strongly recommended while developing fixtures. diff --git a/tests/runner-e2e/catalog.test.ts b/tests/runner-e2e/catalog.test.ts index 027ab20201d..1d5e571af9f 100644 --- a/tests/runner-e2e/catalog.test.ts +++ b/tests/runner-e2e/catalog.test.ts @@ -71,10 +71,10 @@ describe("runner E2E catalog", () => { expect(localIntegrityTasks).toHaveLength(2); expect(openRouterBreadthTasks).toHaveLength(3); expect(runnerSuites.map((suite) => suite.expectedMatrixSize)).toEqual([ - 23, 38, 52, 28, 18, 42, 14, 10, 2, + 23, 38, 52, 28, 18, 6, 42, 14, 10, 2, ]); - expect(validateRunnerCatalog()).toHaveLength(227); - expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(227); + expect(validateRunnerCatalog()).toHaveLength(233); + expect(new Set(runnerMatrix.map((entry) => entry.id)).size).toBe(233); expect( runnerMatrix.filter((entry) => entry.suite.id === "core-compatibility"), ).toHaveLength(42); diff --git a/tests/runner-e2e/catalog.ts b/tests/runner-e2e/catalog.ts index 11833435c62..4877acddcd8 100644 --- a/tests/runner-e2e/catalog.ts +++ b/tests/runner-e2e/catalog.ts @@ -2,7 +2,7 @@ import { continuationTasks } from "./continuation-cases.js"; import { everydayTasks, productionStoryProfile } from "./everyday-cases.js"; import { firstTaskTasks } from "./first-task-cases.js"; -import { chatTasks, chatHardeningTasks } from "./chat-cases.js"; +import { chatTasks, chatHardeningTasks, chatStoryTasks } from "./chat-cases.js"; import { createHash } from "node:crypto"; import { createAgentSchema } from "../../packages/shared/src/validators/agent.js"; import { createEnvironmentSchema } from "../../packages/shared/src/validators/environment.js"; @@ -959,6 +959,15 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [ ["stop-startup-new-resume", "hire-delegate-reuse", "blocked-status-review"].map(task => `agent-chat-hardening.${profile}.daytona.${task}`)), definitionMetadata: { version: 5, permissions: "production-defaults", instructions: "production", grading: "durable-state-and-source-evidence", scheduling: "explicit-only", restartMemory: "required-after-restart", statusEvidence: "structured-current-blocker-and-active-run-count", readOnlyState: "public-mutation-contract-and-relations", hiringReference: "neutral-document-reference-line" }, }, + { + id: "agent-chat-stories", label: "Agent Chat Setup and Interruptions", manualOnly: true, + description: "Experimental settings lifecycle and user follow-ups during active native work.", + groups: ["chat", "native"], + profiles: runnerProfiles.filter(profile => ["runner-codex", "runner-acpx-claude"].includes(profile.id)) + .map(profile => productionStoryProfile(defaultPermissionProfile(profile))), + environments: [localEnvironment], tasks: chatStoryTasks, expectedMatrixSize: 6, + definitionMetadata: { version: 3, setup: "configured-native-agent", permissions: "production-defaults", interruptionBoundary: "provider-file-wait-in-agent-workspace", grading: "persisted-comments-and-plan-run-attributed", scheduling: "explicit-only" }, + }, ...(process.env.PAPERCLIP_RUNNER_E2E_CONNECTION_REVIEWS === "1" ? [connectionReviewSuite] : []), { id: "core-compatibility", @@ -1033,6 +1042,7 @@ export function suiteDefinitionHash(suite: RunnerSuiteFixture) { id: task.id, flow: task.flow, expectedRunCount: task.expectedRunCount, + ...(task.minimumExpectedRunCount === undefined ? {} : { minimumExpectedRunCount: task.minimumExpectedRunCount }), restartServerBeforeQuestionAnswer: task.restartServerBeforeQuestionAnswer ?? false, })), diff --git a/tests/runner-e2e/chat-cases.ts b/tests/runner-e2e/chat-cases.ts index c0e59620e08..575ba99c921 100644 --- a/tests/runner-e2e/chat-cases.ts +++ b/tests/runner-e2e/chat-cases.ts @@ -47,6 +47,11 @@ function buildChatTasks(definitions: readonly (readonly [string, string, number] } export const chatTasks = buildChatTasks(CHAT_CASES); export const chatHardeningTasks = buildChatTasks(HARDENING_CASES); +export const chatStoryTasks = buildChatTasks([ + ["enable-disable-resume", "Enable Agent Chat, pause access, and resume preserved history", 2], + ["followup-while-running", "Deliver a follow-up while a provider turn is running", 2], + ["revise-while-running", "Change instructions during active work and save the updated plan", 2], +]).map(task => ({ ...task, ...(task.id === "enable-disable-resume" ? {} : { minimumExpectedRunCount: 1 }) })); export function chatNeedsApiTools(suiteId: string, caseId: string): boolean { return suiteId === "agent-chat-hardening" && ["hire-delegate-reuse", "blocked-status-review"].includes(caseId); diff --git a/tests/runner-e2e/chat-flow.ts b/tests/runner-e2e/chat-flow.ts index 99fd8dfa606..6a1e54b16ce 100644 --- a/tests/runner-e2e/chat-flow.ts +++ b/tests/runner-e2e/chat-flow.ts @@ -9,6 +9,8 @@ import type { MatrixExecution } from "./types.js"; import { isBlockedUnstartedWake } from "./non-execution-wake.js"; import { chatMarker } from "./chat-cases.js"; import { assertChatRememberedAfterRestart, assertChatStartupStopped, isChatStopReady, runChatHardeningFlow } from "./chat-hardening.js"; +import { enableChatThroughSettings, runChatInterruption, runChatSettingsLifecycle } from "./chat-stories.js"; +import { matchesRunCount, minimumRunCount } from "./run-count.js"; // Public API observations only: this driver never fabricates provider results or writes DB state. export interface ChatIssue { @@ -306,6 +308,7 @@ export async function runChatFlow(input: ChatFlowInput) { const draftMarker = chatMarker("DRAFT", nonce); const caseId = execution.task.id; const stopCase = ["stop-new-resume", "stop-startup-new-resume"].includes(caseId); + const interruptionCase = ["followup-while-running", "revise-while-running"].includes(caseId); let issue: ChatIssue; let runs: ChatRun[] = []; const settings = await api.get>( @@ -368,7 +371,8 @@ export async function runChatFlow(input: ChatFlowInput) { }; const noTasks = async () => expect(await tasks()).toHaveLength(0); try { - await api.patch("/api/instance/settings/experimental", { + if (caseId === "enable-disable-resume") await enableChatThroughSettings(input); + else await api.patch("/api/instance/settings/experimental", { enableAgentChat: true, enableClassicTaskInterface: false, }); @@ -380,7 +384,12 @@ export async function runChatFlow(input: ChatFlowInput) { expect(await api.get(chatPath)).toBeNull(); expect(await allRuns()).toHaveLength(0); - if ( + if (caseId === "enable-disable-resume") { + await runChatSettingsLifecycle({ input, marker, issue: () => issue!, idle, allRuns, comments }); + } else if (interruptionCase) { + await runChatInterruption({ input, marker, issue: () => issue!, idle, allRuns, comments, + refreshIssue: async () => { issue = await api.get(chatPath); input.observe(issue, await allRuns()); } }); + } else if ( ["continuity-restart", "new-session", "stop-new-resume", "stop-startup-new-resume"].includes(caseId) ) { const secret = chatMarker("OLDCONTEXT", nonce); @@ -911,10 +920,8 @@ export async function runChatFlow(input: ChatFlowInput) { projects, }); } - await idle(execution.task.expectedRunCount); - expect(runs.filter((run) => !isResetRun(run))).toHaveLength( - execution.task.expectedRunCount, - ); + await idle(minimumRunCount(execution.task)); + expect(matchesRunCount(execution.task, runs.filter((run) => !isResetRun(run)).length)).toBe(true); for (const run of runs.filter((run) => !isResetRun(run))) { expect(run.runtimeMode).toBe(execution.profile.expectedRuntimeMode); expect(run.status).toBe( diff --git a/tests/runner-e2e/chat-restart.spec.ts b/tests/runner-e2e/chat-restart.spec.ts new file mode 100644 index 00000000000..ad021128946 --- /dev/null +++ b/tests/runner-e2e/chat-restart.spec.ts @@ -0,0 +1,33 @@ +import { createServer } from "node:http"; +import { expect, test } from "@playwright/test"; +import { restartChatServer } from "./chat-restart.js"; + +test("unloads the old browser client before server restart and opens the new document", async ({ page }) => { + let generation = 1; + const server = createServer((_req, res) => { + res.writeHead(200, { "content-type": "text/html" }); + res.end(``); + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (!address || typeof address === "string") throw new Error("Missing listener"); + const url = `http://127.0.0.1:${address.port}`; + try { + await page.goto(url); + await expect(page.getByTestId("composer")).toHaveValue("server generation 1"); + await restartChatServer(page, async () => { + expect(page.url(), "The old client must be gone before it can auto-reload").toBe("about:blank"); + server.closeAllConnections(); + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + generation += 1; + await new Promise(resolve => server.listen(address.port, "127.0.0.1", resolve)); + }); + await page.goto(url, { waitUntil: "commit" }); + await expect(page.getByTestId("composer")).toHaveValue("server generation 2"); + } finally { + if (server.listening) { + server.closeAllConnections(); + await new Promise((resolve, reject) => server.close(error => error ? reject(error) : resolve())); + } + } +}); diff --git a/tests/runner-e2e/chat-restart.ts b/tests/runner-e2e/chat-restart.ts new file mode 100644 index 00000000000..ef488ab3bd6 --- /dev/null +++ b/tests/runner-e2e/chat-restart.ts @@ -0,0 +1,7 @@ +import type { Page } from "@playwright/test"; + +/** Restart the test server without letting the old Vite client race navigation. */ +export async function restartChatServer(page: Page, restart: () => Promise) { + await page.goto("about:blank", { waitUntil: "commit", timeout: 30_000 }); + await restart(); +} diff --git a/tests/runner-e2e/chat-stories.test.ts b/tests/runner-e2e/chat-stories.test.ts new file mode 100644 index 00000000000..589831ce2fc --- /dev/null +++ b/tests/runner-e2e/chat-stories.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { assertInterruptedChat, prepareChatBrief } from "./chat-stories.js"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { runnerMatrix } from "./catalog.js"; +import { buildRunnerE2EProcessEnvironment } from "./harness-env.js"; + +const run = { id: "run", companyId: "company", agentId: "agent", status: "succeeded", runtimeMode: "native", contextSnapshot: { issueId: "chat" } }; +const valid = { + first: "Read the brief", followup: "Change the launch day", reference: "BRIEF123", marker: "UPDATED123", issueId: "chat", + boundaryRun: { ...run, status: "running" }, activeAtFollowup: { ...run, status: "running" }, + comments: [{ id: "first", body: "Read the brief" }, { id: "followup", body: "Change the launch day" }, + { id: "answer", authorAgentId: "agent", createdByRunId: "run", body: "BRIEF123 UPDATED123" }], + runs: [run], revisedPlan: JSON.stringify({ launchDay: "Friday", reference: "BRIEF123", revision: "UPDATED123" }), +}; + +describe("active chat follow-up oracle", () => { + it("creates the missing fixture directory and waits for the host-supplied brief", async () => { + const root = await mkdtemp(path.join(tmpdir(), "chat-brief-test-")); + try { + const { gate, ready, scriptPath } = await prepareChatBrief(path.join(root, "new workspace"), "fixture"); + const command = promisify(execFile)(process.execPath, [scriptPath], { timeout: 5_000 }); + try { + await expect.poll(() => readFile(ready, "utf8").catch(() => "")).toBe("waiting"); + } finally { + await writeFile(gate, "BRIEF fixture result"); + expect((await command).stdout.trim()).toBe("BRIEF fixture result"); + } + } finally { + await rm(root, { recursive: true, force: true }); + } + }); + it("accepts either steering or one queued successor, with the saved correction", () => { + expect(() => assertInterruptedChat(valid)).not.toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [run, { ...run, id: "successor" }], comments: [...valid.comments.slice(0, 2), { ...valid.comments[2]!, createdByRunId: "successor" }] })).not.toThrow(); + }); + it("rejects follow-ups sent after the active boundary", () => { + expect(() => assertInterruptedChat({ ...valid, boundaryRun: run })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, activeAtFollowup: run })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, activeAtFollowup: { ...valid.activeAtFollowup, id: "other" } })).toThrow(); + }); + it("rejects lost or duplicated input, missing file evidence, and stale plans", () => { + expect(() => assertInterruptedChat({ ...valid, comments: [...valid.comments, { ...valid.comments[2]!, id: "duplicate-reply" }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, comments: [...valid.comments.slice(0, 2), { ...valid.comments[2]!, createdByRunId: "unrelated" }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, comments: valid.comments.filter(c => c.id !== "followup") })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, comments: [...valid.comments, { id: "duplicate", body: valid.followup }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, comments: [...valid.comments.slice(0, 2), { id: "answer", authorAgentId: "agent", body: "UPDATED123" }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, revisedPlan: valid.revisedPlan.replace("Friday", "Monday") })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, revisedPlan: "The updated plan is saved." })).toThrow(); + }); + it("rejects failed, unfinished, duplicated, or unrelated execution", () => { + for (const status of ["running", "queued", "failed", "cancelled"]) + expect(() => assertInterruptedChat({ ...valid, runs: [{ ...run, status }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [run, run, run] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [run, run] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [{ ...run, id: "other" }] })).toThrow(); + expect(() => assertInterruptedChat({ ...valid, runs: [{ ...run, contextSnapshot: { issueId: "other" } }] })).toThrow(); + }); + it("keeps setup and interruption stories native, local, opt-in, and outside API-tool overrides", () => { + const cells = runnerMatrix.filter(cell => cell.suite.id === "agent-chat-stories"); + expect(cells).toHaveLength(6); + for (const cell of cells) { + expect(cell.suite.manualOnly).toBe(true); + expect(cell.profile.generation).toBe("native"); + expect(cell.environment.id).toBe("local"); + expect(buildRunnerE2EProcessEnvironment({}, [cell]).PAPERCLIP_RUNNER_API_TOOLS_ENABLED).toBeUndefined(); + } + }); +}); diff --git a/tests/runner-e2e/chat-stories.ts b/tests/runner-e2e/chat-stories.ts new file mode 100644 index 00000000000..f28bfc8a66f --- /dev/null +++ b/tests/runner-e2e/chat-stories.ts @@ -0,0 +1,160 @@ +import { expect } from "@playwright/test"; +import { mkdir, readFile, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import { sendChatMessage, type ChatFlowInput, type ChatIssue, type ChatRun } from "./chat-flow.js"; +import { setSettingsToggle } from "./settings-toggle.js"; +import { resolveDefaultAgentWorkspaceDir } from "../../server/src/home-paths.js"; + +type Comment = { id: string; body: string; authorAgentId?: string; createdByRunId?: string }; +type Context = { + input: ChatFlowInput; marker: string; issue(): ChatIssue; + idle(count: number): Promise; allRuns(): Promise; comments(): Promise; +}; + +/** Seed only the disabled setting; the browser performs the user's opt-in. */ +export async function enableChatThroughSettings(input: ChatFlowInput) { + await input.api.patch("/api/instance/settings/experimental", { enableAgentChat: false, enableClassicTaskInterface: false }); + await setChatEnabled(input, true); +} + +async function setChatEnabled(input: ChatFlowInput, enabled: boolean) { + await input.page.goto(`/${input.fixtures.company.issuePrefix}/company/settings/instance/experimental`, { waitUntil: "domcontentloaded" }); + const toggle = input.page.getByRole("switch", { name: "Toggle agent chat experimental setting" }); + await setSettingsToggle(toggle, enabled); + await expect.poll(async () => (await input.api.get<{ enableAgentChat: boolean }>("/api/instance/settings/experimental")).enableAgentChat).toBe(enabled); +} + +export function assertInterruptedChat(input: { + first: string; followup: string; reference: string; marker: string; issueId: string; + boundaryRun: ChatRun; activeAtFollowup: ChatRun; comments: Comment[]; runs: ChatRun[]; + revisedPlan?: string; +}) { + expect(input.boundaryRun.status).toBe("running"); + expect(input.activeAtFollowup).toMatchObject({ id: input.boundaryRun.id, status: "running" }); + for (const message of [input.first, input.followup]) { + expect(input.comments.filter(comment => !comment.authorAgentId && comment.body === message)).toHaveLength(1); + } + const replies = input.comments.filter(comment => comment.authorAgentId); + const final = replies.at(-1)!; + expect(final).toBeTruthy(); + expect(final.body).toContain(input.reference); + expect(final.body).toContain(input.marker); + expect(replies.filter(reply => reply.body.includes(input.marker))).toHaveLength(1); + expect(input.runs.length).toBeGreaterThanOrEqual(1); + expect(input.runs.length).toBeLessThanOrEqual(2); + expect(input.runs.every(run => run.status === "succeeded" && run.runtimeMode === "native")).toBe(true); + expect(new Set(input.runs.map(run => run.id)).size).toBe(input.runs.length); + expect(input.runs.every(run => run.agentId === input.boundaryRun.agentId && run.contextSnapshot?.issueId === input.issueId)).toBe(true); + expect(final.authorAgentId).toBe(input.boundaryRun.agentId); + const responseRun = input.runs.length === 1 + ? input.boundaryRun + : input.runs.find(run => run.id !== input.boundaryRun.id)!; + expect(final.createdByRunId).toBe(responseRun.id); + expect(input.runs.some(run => run.id === input.boundaryRun.id)).toBe(true); + if (input.revisedPlan !== undefined) { + const body = input.revisedPlan.trim().replace(/^```(?:json)?\s*/, "").replace(/\s*```$/, ""); + expect(JSON.parse(body)).toEqual({ launchDay: "Friday", reference: input.reference, revision: input.marker }); + } +} + +export async function prepareChatBrief(workspacePath: string, nonce: string) { + await mkdir(workspacePath, { recursive: true }); + const gate = path.join(workspacePath, `chat-brief-${nonce}.txt`); + const ready = `${gate}.waiting`; + const scriptPath = path.join(workspacePath, `wait-for-brief-${nonce}.cjs`); + const script = `const fs=require("node:fs");fs.writeFileSync(${JSON.stringify(ready)},"waiting");const end=Date.now()+120000;const timer=setInterval(()=>{if(fs.existsSync(${JSON.stringify(gate)})){console.log(fs.readFileSync(${JSON.stringify(gate)},"utf8"));clearInterval(timer)}else if(Date.now()>end){clearInterval(timer);process.exitCode=1}},100);`; + await writeFile(scriptPath, script, "utf8"); + return { gate, ready, scriptPath }; +} + +export async function runChatInterruption(context: Context & { refreshIssue(): Promise }) { + const { input, marker, idle, allRuns, comments } = context; + // Unprojected host /tmp files are intentionally hidden from native Codex. + // These projectless chats use the normal agent-home workspace, not the + // harness's separate project fixture directory. Keep the asset inside it. + const agentWorkspace = resolveDefaultAgentWorkspaceDir(input.fixtures.agent.id); + const relative = path.relative(path.dirname(input.workspacePath), agentWorkspace); + if (relative.startsWith("..") || path.isAbsolute(relative)) throw new Error("Chat fixture workspace escaped the isolated instance"); + const { gate, ready, scriptPath } = await prepareChatBrief(agentWorkspace, input.nonce); + const reference = `BRIEF${randomUUID().replaceAll("-", "")}`; + // Real provider tool execution waits on an ordinary fixture file. No runner + // hooks, provider results, database records, or task outcomes are fabricated. + // Keep JavaScript in a seeded fixture file: rich-text input serializes raw + // operators as Markdown escapes, which should not alter a timing fixture. + const command = `node ${scriptPath}`; + const first = `Our launch is planned for Monday. First run this bounded command to wait for the brief reference file I am supplying: ${command}\nAfter it returns, acknowledge the reference from the file here. This is discussion only; do not create projects or tasks.`; + const revise = input.execution.task.id === "revise-while-running"; + const followup = revise + ? `Change the launch day to Friday. Save the plan on this conversation as JSON with exactly launchDay, reference (from the brief file), and revision ("${marker}"). Then reply here with the brief reference and ${marker}. Do not create projects or execution tasks.` + : `After reading the brief, reply here with its reference and ${marker}. Keep this in the current conversation; do not create projects or tasks.`; + let boundaryRun: ChatRun | undefined; + let activeAtFollowup: ChatRun | undefined; + try { + await sendChatMessage(input.page, first); + await expect.poll(async () => { + if (await readFile(ready, "utf8").catch(() => "") !== "waiting") return false; + boundaryRun = (await allRuns()).find(run => run.status === "running"); + return Boolean(boundaryRun); + }, { timeout: 120_000 }).toBe(true); + expect(boundaryRun!.contextSnapshot?.paperclipWorkspace).toMatchObject({ cwd: agentWorkspace }); + await context.refreshIssue(); + await sendChatMessage(input.page, followup); + await expect.poll(async () => (await comments()).filter(comment => !comment.authorAgentId && comment.body === followup).length, + { timeout: 30_000 }).toBe(1); + activeAtFollowup = await input.api.get(`/api/heartbeat-runs/${boundaryRun!.id}`); + await input.evidence("chat-interruption-boundary.json", { first, followup, boundaryRun, activeAtFollowup, comments: await comments() }); + expect(activeAtFollowup.status, "Follow-up must persist while the first provider run is active").toBe("running"); + await writeFile(gate, reference, "utf8"); + await expect.poll(async () => (await comments()).filter(comment => comment.authorAgentId).at(-1)?.body ?? "", { timeout: 240_000 }).toContain(marker); + await idle(1); // Providers may queue a new run or steer the current run. + const plan = revise ? await input.api.get<{ body: string }>(`/api/issues/${context.issue().id}/documents/plan`) : undefined; + const evidence = { first, followup, reference, marker, issueId: context.issue().id, boundaryRun: boundaryRun!, activeAtFollowup, + comments: await comments(), runs: await allRuns(), ...(plan ? { revisedPlan: plan.body } : {}) }; + await input.evidence("chat-interruption.json", evidence); + assertInterruptedChat(evidence); + expect(await input.api.get(`/api/companies/${input.fixtures.company.id}/issues`)).toEqual([]); + expect(await input.api.get(`/api/companies/${input.fixtures.company.id}/projects`)).toEqual([]); + } finally { + // Always release a provider that is still waiting, including failed UI runs. + await writeFile(gate, reference, "utf8"); + const chatPath = `/api/companies/${input.fixtures.company.id}/chats/${input.fixtures.agent.id}`; + const current = await input.api.get(chatPath).catch(() => null); + await input.evidence("chat-interruption-state.json", { + issue: current, + runs: await allRuns().catch(error => ({ readError: String(error) })), + comments: current ? await input.api.get(`/api/issues/${current.id}/comments?order=asc`).catch(error => ({ readError: String(error) })) : [], + plan: current && revise ? await input.api.get(`/api/issues/${current.id}/documents/plan`).catch(error => ({ readError: String(error) })) : null, + }); + } +} + +export async function runChatSettingsLifecycle(context: Context) { + const { input, idle, comments, allRuns, marker } = context; + const remembered = `REMEMBER${marker}`; + const route = `/${input.fixtures.company.issuePrefix}/chats/${input.fixtures.agent.id}`; + await sendChatMessage(input.page, `Remember ${remembered} in this conversation. Acknowledge only; no tasks or projects.`); + await idle(1); + const before = { issue: context.issue(), comments: await comments(), runs: await allRuns() }; + await setChatEnabled(input, false); + // Full document navigation clears the client query cache. Unlike SPA + // navigation, this verifies the disabled entry page without cached history. + await input.page.goto(route, { waitUntil: "domcontentloaded" }); + await expect(input.page.getByText(/Agent Chat is disabled/)).toBeVisible(); + const blocked = await input.api.request.post(`/api/issues/${before.issue.id}/comments`, { data: { body: `DISABLED${marker}` } }); + expect(blocked.status()).toBe(404); + expect(await comments()).toEqual(before.comments); + expect((await allRuns()).map(run => ({ id: run.id, status: run.status }))) + .toEqual(before.runs.map(run => ({ id: run.id, status: run.status }))); + await setChatEnabled(input, true); + await input.page.goto(route, { waitUntil: "domcontentloaded" }); + await sendChatMessage(input.page, `What reference did I ask you to remember? Reply with it and ${marker}. No new work.`); + await idle(2); + expect(context.issue().id).toBe(before.issue.id); + expect(context.issue().conversationSessionGeneration).toBe(before.issue.conversationSessionGeneration); + const reply = (await comments()).filter(comment => comment.authorAgentId).at(-1)?.body ?? ""; + expect(reply).toContain(remembered); + expect(reply).toContain(marker); + expect(await input.api.get(`/api/companies/${input.fixtures.company.id}/issues`)).toEqual([]); + await input.evidence("chat-settings-lifecycle.json", { before, after: { issue: context.issue(), comments: await comments(), runs: await allRuns() } }); +} diff --git a/tests/runner-e2e/playwright-support.config.ts b/tests/runner-e2e/playwright-support.config.ts index 517dcfa332c..53e6b3a47c2 100644 --- a/tests/runner-e2e/playwright-support.config.ts +++ b/tests/runner-e2e/playwright-support.config.ts @@ -3,7 +3,7 @@ import { defineConfig } from "@playwright/test"; /** Browser-only harness regressions: no Paperclip instance or provider credentials. */ export default defineConfig({ testDir: ".", - testMatch: ["screenshot-readiness.spec.ts", "lost-send.spec.ts"], + testMatch: ["screenshot-readiness.spec.ts", "lost-send.spec.ts", "chat-restart.spec.ts", "settings-toggle.spec.ts"], workers: 1, retries: 0, timeout: 10_000, diff --git a/tests/runner-e2e/run-count.test.ts b/tests/runner-e2e/run-count.test.ts new file mode 100644 index 00000000000..ad893b3b03c --- /dev/null +++ b/tests/runner-e2e/run-count.test.ts @@ -0,0 +1,11 @@ +import { expect, it } from "vitest"; +import { matchesRunCount, minimumRunCount } from "./run-count.js"; + +it("keeps ordinary cases exact and bounds either one steered run or two sequential runs", () => { + const exact = { expectedRunCount: 2 }; + const ranged = { ...exact, minimumExpectedRunCount: 1 }; + expect(minimumRunCount(exact)).toBe(2); + expect(minimumRunCount(ranged)).toBe(1); + expect([0, 1, 2, 3].map(n => matchesRunCount(exact, n))).toEqual([false, false, true, false]); + expect([0, 1, 2, 3].map(n => matchesRunCount(ranged, n))).toEqual([false, true, true, false]); +}); diff --git a/tests/runner-e2e/run-count.ts b/tests/runner-e2e/run-count.ts new file mode 100644 index 00000000000..221ce39cb71 --- /dev/null +++ b/tests/runner-e2e/run-count.ts @@ -0,0 +1,6 @@ +import type { RunnerTaskFixture } from "./types.js"; + +type RunCountContract = Pick; +export const minimumRunCount = (task: RunCountContract) => task.minimumExpectedRunCount ?? task.expectedRunCount; +export const matchesRunCount = (task: RunCountContract, count: number) => + count >= minimumRunCount(task) && count <= task.expectedRunCount; diff --git a/tests/runner-e2e/runner.spec.ts b/tests/runner-e2e/runner.spec.ts index 73ce46984c0..e5008fd746e 100644 --- a/tests/runner-e2e/runner.spec.ts +++ b/tests/runner-e2e/runner.spec.ts @@ -4,6 +4,8 @@ import { createTaskThroughUi, submitTaskReply } from "./user-actions.js"; import { runFirstTaskFlow, setupFirstTaskFixtures } from "./first-task-flow.js"; import { runChatFlow } from "./chat-flow.js"; +import { restartChatServer } from "./chat-restart.js"; +import { matchesRunCount, minimumRunCount } from "./run-count.js"; import { createHash, randomBytes } from "node:crypto"; import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import path from "node:path"; @@ -832,7 +834,7 @@ for (const execution of executions) { } else if (execution.task.flow === "agent_chat") { const chat = await runChatFlow({ page, api, fixtures, execution, nonce, workspacePath, - restart: () => restartIsolatedPaperclipServer({ api, requestId: `chat-${nonce}`, deadlineAt: startedAtMs + deadlineMs }), + restart: () => restartChatServer(page, () => restartIsolatedPaperclipServer({ api, requestId: `chat-${nonce}`, deadlineAt: startedAtMs + deadlineMs })), observe: (chatIssue, chatRuns) => { issue = chatIssue; selectedRuns = chatRuns; }, capture: captureScreenshot, evidence: (name, data) => writeSanitizedJson(snapshotsDir, name, data, secrets), @@ -1548,7 +1550,7 @@ for (const execution of executions) { load: loadTaskState, accept: ({ currentIssue, taskRuns }) => currentIssue.status === execution.task.expectedTerminalState.issue && - taskRuns.length >= execution.task.expectedRunCount && + taskRuns.length >= minimumRunCount(execution.task) && taskRuns.every((run) => TERMINAL_RUN_STATUSES.has(run.status)), reject: ({ taskRuns }) => definitiveRunFailure(taskRuns), }); @@ -1565,7 +1567,7 @@ for (const execution of executions) { deadlineAt: Math.min(deadlineAt, Date.now() + 30_000), load: loadTaskState, accept: ({ taskRuns, comments }) => { - if (taskRuns.length !== execution.task.expectedRunCount) { + if (!matchesRunCount(execution.task, taskRuns.length)) { return false; } const finalRun = sortRunsChronologically(taskRuns).at(-1); @@ -1587,7 +1589,7 @@ for (const execution of executions) { expect(pending.actionRequests).toHaveLength(0); await writeSanitizedJson(snapshotsDir, "connection-review.json", { source: "local MCP fixture", connectionId: reviewProvider.connectionId, providerCalls: reviewProvider.invocationCount(), decision: execution.task.toolReviewDecision, issueId: issue.id }, secrets); } - if (selectedRuns.length !== execution.task.expectedRunCount) { + if (!matchesRunCount(execution.task, selectedRuns.length)) { const runLogs = await Promise.all( selectedRuns.map(async (candidate) => ({ runId: candidate.id, @@ -1608,7 +1610,7 @@ for (const execution of executions) { secrets, ); throw new Error( - `Expected exactly ${execution.task.expectedRunCount} task heartbeat run(s); observed ${selectedRuns.length}`, + `Expected ${minimumRunCount(execution.task)}..${execution.task.expectedRunCount} task heartbeat run(s); observed ${selectedRuns.length}`, ); } // The company run-list endpoint intentionally returns only a compact, diff --git a/tests/runner-e2e/settings-toggle.spec.ts b/tests/runner-e2e/settings-toggle.spec.ts new file mode 100644 index 00000000000..315dbe9ea49 --- /dev/null +++ b/tests/runner-e2e/settings-toggle.spec.ts @@ -0,0 +1,11 @@ +import { expect, test } from "@playwright/test"; +import { setSettingsToggle } from "./settings-toggle.js"; + +test("waits for a controlled switch and does not toggle an already-saved value", async ({ page }) => { + await page.setContent(``); + const toggle = page.getByRole("switch"); + await setSettingsToggle(toggle, true); + await setSettingsToggle(toggle, true); + await expect(toggle).toHaveAttribute("aria-checked", "true"); + await setSettingsToggle(toggle, false); +}); diff --git a/tests/runner-e2e/settings-toggle.ts b/tests/runner-e2e/settings-toggle.ts new file mode 100644 index 00000000000..59591b460a5 --- /dev/null +++ b/tests/runner-e2e/settings-toggle.ts @@ -0,0 +1,8 @@ +import { expect, type Locator } from "@playwright/test"; + +/** Controlled settings switches update after their asynchronous save handler. */ +export async function setSettingsToggle(toggle: Locator, enabled: boolean) { + await expect(toggle).toBeVisible(); + if (await toggle.getAttribute("aria-checked") !== String(enabled)) await toggle.click(); + await expect(toggle).toHaveAttribute("aria-checked", String(enabled)); +} diff --git a/tests/runner-e2e/types.ts b/tests/runner-e2e/types.ts index 795260ce2d9..81a4f22ef0f 100644 --- a/tests/runner-e2e/types.ts +++ b/tests/runner-e2e/types.ts @@ -125,6 +125,8 @@ export interface RunnerTaskFixture { workMode: RunnerTaskWorkMode; flow: RunnerTaskFlow; expectedRunCount: number; + /** Optional lower bound; expectedRunCount remains the maximum/cost estimate. */ + minimumExpectedRunCount?: number; attemptTimeoutMs: Readonly>; expectedTerminalState: { issue: "done" | "in_review" | "blocked";