mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
## Thinking Path > - Paperclip manages AI agents and their tasks. > - A task can outlive a provider process or a server restart. > - Legacy recovery treated unknown tool outcomes as a permanent execution hold. > - That hold could also reject a later user message. > - A conversation turn can use prior history without replaying prior tool calls. > - This pull request lets supported conversation adapters continue within the existing retry budget. > - Users can send a new message after automatic attempts stop. ## Linked Issues or Issue Description **What happened?** A server restart could interrupt a local ACP run and leave its task behind a permanent recovery hold. A later user message could be cancelled before the provider answered. The immediate recovery path could also create a successor outside the durable failure counter. **Expected behavior** Continue with a bounded new conversation turn. Preserve a compatible provider session or use full task context when it is unavailable. Do not replay recorded tools. When automatic attempts stop, allow a new user request through the normal execution gates. **Steps to reproduce** 1. Start a task with a local conversation adapter. 2. Restart the server while the provider is working. 3. Let the previous run become interrupted. 4. Send a follow-up message and observe the recovery hold on the old behavior. Related work: Refs #13075 for durable task recovery. Refs #12946 for retry-limit and checkout-lock handling. This change routes conversation recovery through the existing bounded scheduler. ## What Changed - Mark supported local conversation failures for continuation. Keep native-runner and non-conversation recovery rules. - Carry an interruption notice into the next turn. Retain stopped ACP session history even when a write outcome is unknown. - Clear unavailable ACP sessions so the next bounded attempt can use full task context. - Route immediate failure recovery through the same durable scheduler as process-loss recovery. Release only the predecessor checkout when its retry takes ownership. - Retire obsolete conversation holds using immutable run evidence, in bounded batches with an activity record. Preserve outcome evidence and do not wake historical tasks. - Block actual admission and Resume while a predecessor process or environment lease is still active. Keep the original interruption notice after a rejected wake. Preserve the upstream blocked-wake waiting contract: bounded retry planning can happen during cleanup, while deferred messages and execution remain gated. - Add subprocess and database regression tests. Update the execution contract. - Add the current thread-status field to the native recovery provider fixture so its damaged-journal test reaches the intended boundary. Tolerate an already-exited fixture process during test cleanup while still asserting both processes terminate. ## Verification - Workspace typecheck passed: `pnpm -r typecheck`. - Build passed: `pnpm build`. - Module boundaries passed: `pnpm check:module-boundaries`. - Focused tests passed: 293 recovery/session/dispatch tests, 66 retry and response-gate tests, and 37 native-session tests. Some suites overlap. - Tests cover interrupted writes, missing sessions, concurrent retries, restart persistence, pending questions and approvals, execution gates, and historical holds. - Built the Rust test executables with `pnpm --filter @paperclipai/paperclip-runner build:rust` for native-runner verification. - Full Vitest coverage verified locally using the repository’s general and serialized shards, with focused reruns for failures and files not reached after a shard stopped. The ownership-gate regression is fixed and the complete affected server shard passes (1,390 tests). Local parallel runs also hit temporary-directory, resource, and timing failures; those suites pass with canonical temporary paths and sequential reruns. No test timeouts were increased. - Final merged-branch regression run: 577 tests pass across process recovery, retry scheduling, liveness, durable chat, wake-queue application/adapter, dispatch, continuation, native sessions, and task chat. Earlier focused verification also passed 19 native control tests. Token gates and whitespace validation pass. - Browser verification passed all three ACP Stop/continue/pause scenarios, including a rerun after merging the upstream waiting behavior: `PAPERCLIP_E2E_PORT=3397 pnpm test:e2e tests/e2e/acp-stop-continuation.spec.ts`. The interrupted-write case verifies that follow-up completes without a repeated write. - Final-head [CI run 34625037394](https://github.com/paperclipai/paperclip/actions/runs/34625037394) passed on `06ac4bd9d150f8b209a96e5fd609c696958794a0`: all 31 reported checks are green, including server/workspace suites, all browser shards, native runner verification, build, typecheck, release dry run, and aggregate gates. The two conditional Storybook checks were skipped. Greptile reviewed this exact commit at 5/5; all review threads are resolved. ## Risks - A new model turn can choose to repeat an action. Paperclip does not replay recorded tool calls and does not certify unknown action outcomes. - Conversation adapters now stop after their retry budget instead of requiring action reconciliation. Explicit Stop, pause, dependency, approval, budget, and ownership gates remain in force. - No schema migration or dependency changes. Historical holds are folded without changing task status or waking work. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository tools, code execution, and test execution. The session does not expose a more specific model build ID or context-window size. ## 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 <noreply@paperclip.ing>
94 lines
6.3 KiB
TypeScript
94 lines
6.3 KiB
TypeScript
import { mkdtemp, readFile, rm } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { test, expect, type APIResponse } from "@playwright/test";
|
|
|
|
async function json(response: APIResponse) {
|
|
const body = await response.text();
|
|
expect(response.ok(), `${response.url()}: ${response.status()} ${body}`).toBe(true);
|
|
return JSON.parse(body);
|
|
}
|
|
|
|
for (const { unfinishedWrite, pause } of [{ unfinishedWrite: false, pause: false }, { unfinishedWrite: true, pause: false }, { unfinishedWrite: false, pause: true }]) {
|
|
test(`embedded ACP Stop: ${unfinishedWrite ? "unknown action continues without replaying the write" : pause ? "composer pause requires Resume before continuation" : "go continues the same session with queued input"}`, async ({ page, request }) => {
|
|
test.setTimeout(120_000);
|
|
const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-stop-browser-"));
|
|
const company = await json(await request.post("/api/companies", { data: { name: `ACP Stop ${Date.now()}` } }));
|
|
const originalSettings = await json(await request.get("/api/instance/settings/experimental"));
|
|
try {
|
|
await json(await request.patch("/api/instance/settings/experimental", { data: { enableClassicTaskInterface: false } }));
|
|
const owner = await json(await request.post(`/api/companies/${company.id}/agents`, { data: {
|
|
name: "ACP Stop fixture", role: "engineer", adapterType: "claude_local",
|
|
adapterConfig: { engine: "acp", cwd: root, stateDir: path.join(root, "state"),
|
|
agentCommand: `${JSON.stringify(process.execPath)} ${JSON.stringify(path.resolve("scripts/mcp-fixtures/servers/acp-stop-agent.mjs"))}`,
|
|
env: { PAPERCLIP_STOP_FIXTURE_ROOT: root, PAPERCLIP_STOP_FIXTURE_FINISH_TASK: "1", ...(unfinishedWrite ? { PAPERCLIP_STOP_FIXTURE_TOOL: "write" } : {}) },
|
|
}, runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: true } },
|
|
} }));
|
|
const issue = await json(await request.post(`/api/companies/${company.id}/issues`, { data: {
|
|
title: "ACP Stop continuation", status: "backlog", assigneeAgentId: owner.id,
|
|
} }));
|
|
await json(await request.patch(`/api/issues/${issue.id}`, { data: { status: "todo" } }));
|
|
await expect.poll(async () => (await readFile(path.join(root, "prompts"), "utf8").catch(() => "")).trim().split("\n").filter(Boolean).length, { timeout: 45_000 }).toBe(1);
|
|
const [active] = await json(await request.get(`/api/issues/${issue.id}/live-runs`));
|
|
expect(active).toBeTruthy();
|
|
await page.goto(`/${company.issuePrefix}/issues/${issue.identifier}`);
|
|
const editor = page.getByRole("textbox", { name: "editable markdown" });
|
|
await editor.fill("List my recent Drive files.");
|
|
await page.getByRole("button", { name: "Send", exact: true }).click();
|
|
await expect.poll(async () => JSON.stringify(await json(await request.get(`/api/issues/${issue.id}/queued-comments`))))
|
|
.toContain("List my recent Drive files.");
|
|
|
|
// Run-level Stop leaves the task unpaused; composer Stop additionally pauses the task.
|
|
let stopped;
|
|
if (pause) {
|
|
await page.getByRole("button", { name: "Stop", exact: true }).click();
|
|
} else {
|
|
await page.getByRole("button", { name: "Interrupt", exact: true }).click();
|
|
}
|
|
await expect.poll(async () => {
|
|
stopped = await json(await request.get(`/api/heartbeat-runs/${active.id}`));
|
|
return stopped.resultJson?.executionCancellation?.state;
|
|
}, { timeout: 30_000 }).toBe("acknowledged");
|
|
expect(stopped.status).toBe("cancelled");
|
|
expect(stopped.resultJson.executionCancellation.state).toBe("acknowledged");
|
|
const writesAtStop = unfinishedWrite ? await readFile(path.join(root, "writes"), "utf8") : null;
|
|
await page.reload();
|
|
if (pause) {
|
|
await expect(page.getByTestId("paused-composer-takeover")).toBeVisible();
|
|
await expect(editor).toHaveCount(0);
|
|
await expect(page.getByRole("button", { name: "Send", exact: true })).toHaveCount(0);
|
|
const rejected = await request.post(`/api/issues/${issue.id}/comments`, { data: { body: "go" } });
|
|
expect(rejected.status()).toBe(409);
|
|
expect((await readFile(path.join(root, "prompts"), "utf8")).trim().split("\n")).toHaveLength(1);
|
|
expect(await readFile(path.join(root, "completed"), "utf8").catch(() => "")).toBe("");
|
|
await page.getByRole("button", { name: "Resume task", exact: true }).click();
|
|
const dialog = page.getByRole("dialog");
|
|
await dialog.getByRole("checkbox").check();
|
|
await dialog.getByRole("button", { name: "Resume work", exact: true }).click();
|
|
} else {
|
|
await editor.fill("go");
|
|
await page.getByRole("button", { name: "Send", exact: true }).click();
|
|
}
|
|
await expect(page.getByText("Answered the pending follow-up once.", { exact: false })).toBeVisible({ timeout: 30_000 });
|
|
await expect.poll(async () => (await json(await request.get(`/api/issues/${issue.id}/live-runs`))).length).toBe(0);
|
|
const prompts = (await readFile(path.join(root, "prompts"), "utf8")).trim().split("\n").map(line => JSON.parse(line));
|
|
expect(prompts).toHaveLength(2);
|
|
expect(new Set(prompts.map(prompt => prompt.sessionId)).size).toBe(1);
|
|
// Resume delivers the queued follow-up in the same provider session.
|
|
const continuationPrompts = pause ? prompts.slice(1) : [prompts.at(-1)];
|
|
expect(JSON.stringify(continuationPrompts)).toContain("List my recent Drive files.");
|
|
if (!pause) expect(JSON.stringify(continuationPrompts)).toContain("go");
|
|
expect(await readFile(path.join(root, "completed"), "utf8")).toBe("follow-up\n");
|
|
const completedIssue = await json(await request.get(`/api/issues/${issue.id}`));
|
|
expect(completedIssue.executionBlocker).toBeNull();
|
|
expect(completedIssue.status).toBe("done");
|
|
if (unfinishedWrite) expect(await readFile(path.join(root, "writes"), "utf8")).toBe(writesAtStop);
|
|
await expect(page.getByRole("dialog")).toHaveCount(0);
|
|
} finally {
|
|
await request.patch(`/api/companies/${company.id}`, { data: { status: "archived" } });
|
|
await request.patch("/api/instance/settings/experimental", { data: { enableClassicTaskInterface: originalSettings.enableClassicTaskInterface } });
|
|
await rm(root, { recursive: true, force: true });
|
|
}
|
|
});
|
|
}
|