mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs must preserve files when Stop ends a turn before its result is saved. > - The cancellation path could skip file retention and leave a prepared remote workspace active. > - An older controller could also admit an extra run while an earlier lost-source hold was unresolved. > - Rejecting every later run then prevented recovery even after that extra run was stopped and its files were reconciled. > - This change retains cancelled workspaces and records the exact proof for a safely reconciled intervening cancellation. > - Recovery can proceed without changing cancelled history, replaying actions, or weakening execution gates. ## Linked Issues or Issue Description Refs: #16583, #16690, #16696. **What happened?** Stop could acknowledge a native turn without recording its unsaved file obligation. Reconstruction also rejected a later cancelled run after its exact environment and files had been reconciled. The task could remain inactive with an `in_progress` status, which the reconstruction endpoint rejected. **Expected behavior** Retain unsaved files after Stop. Permit a board operator to reconstruct from saved sources only after each intervening cancellation has a confirmed stop and a file-only repair record. Preserve the original failed export and the cancelled run. **Steps to reproduce** 1. Prepare a native workspace and cancel the turn before a structured result exists. 2. Inspect its file retention and remote lease state. 3. For a task with an older destroyed export, reconcile the later cancelled run's exact files and unperformed actions. 4. Request reconstruction from verified same-task attachments. ## What Changed - Pin prepared native files before cancellation settlement. Defer teardown if pinning fails. Use the existing stop-only cleanup worker. - Repair active warm and ephemeral sources, plus stopping and confirmed-stopped historical cancellations in bounded startup and periodic batches. Preserve the existing exact-source checks. Bound row-lock waits and advance past a busy source. - Accept only same-owner native run-only cancellations with no result or current coordinator, confirmed stopped leases, and board file-only repair with `not_performed` action outcomes. - Save hashes of each intervening repair and recheck them at queue claim and final dispatch. Reject changed evidence or another later run. - Expose saved-source reconstruction in the issue API for inactive `blocked` and `in_progress` tasks, and hide it when an execution or checkout owner exists. Permit the same inactive source at submission. Keep the source, owner, version, attachment, budget, pause and retry checks. - Update the shared evidence type, recovery runbook and capability maps. Add an optional cancelled-native no-action confirmation to file recovery, resetting evidence when its source changes. No migration, new endpoint, permission or dependency. ## Verification - The new fixture fails on the original validator with `workspace_reconstruction_stale` after valid cancelled-run file repair. - Focused source and Stop retention tests: 94 passed before the final contention case. The subsequent three-suite run passed 123 tests, including real PostgreSQL lock contention and public startup/API/final-dispatch paths. The final narrow-query rerun passed all 123 tests. Review correction: all 143 tests passed across four suites, including real lock contention through the teardown boundary and stopping/stopped allocations. - Six broader cancellation, executor, heartbeat and restart suites: 831 passed, 9 existing skips. - Final correction: all 145 backend tests and all 36 recovery UI tests passed. Real contention covers both warm reusable and ephemeral sources. Optional no-action confirmation defaults to uncertified, and changing source clears evidence. Workspace `pnpm -r typecheck` and token gates passed on these sources. - Maps, module boundaries and Git whitespace checks: passed. Full `pnpm build` passed on the final `4e8c0e73ac` revision as well as the prior reviewed heads. A full local run of the preceding fix completed with 21,724 passes, 109 skips and three failures in webhook body limits, a status-card timeout, and a port-allocation fixture. All three suites then passed unchanged in an isolated rerun (209 tests). The full local run remains recorded as failed; the final head's full CI passed (55 completed checks, no failures). A fresh full local run is also in progress on the frozen reviewed head. - Preserved failures: the first attempt lacked a built plugin SDK; after building prerequisites, the regression reached the expected validator rejection. Treatment tests caught the file-only action's deliberately null continuation owner. A real lock test then caught the database driver's wrapped timeout error. Both defects were corrected and retested. Independent review then found a teardown race after a failed retention write. A second review found active warm sources excluded by the historical policy filter; the regression reproduced both warm failures before correcting the query. Its real-row-lock fixture reproduced the failure; the fixed release guard and historical stopping/stopped cases pass. No checks were weakened. - A pre-deployment walkthrough caught the issue API still hiding reconstruction for an inactive `in_progress` task. The status/capability baseline reproduced three failures, including inappropriate capability flags for execution/checkout owners. The fix uses the same inactive-state requirements as submission; its positive fixture reads the assembled issue API before reconstructing. All 117 tests in those three recovery suites pass, and all 28 heartbeat-recovery tests pass separately. Full workspace typecheck passed again. Full build passed with the exact `4e8c0e73ac` stamp; latest-head independent review is 5/5 with no open findings. All 55 latest-head checks completed with no failures. - Live tenant acceptance remains pending. No successful reconstruction, export or warm-process proof is claimed by these local tests. ## Risks - Stop retains unfinished work, so storage remains until file recovery and explicit cleanup complete. - Reconstruction remains held for unknown or performed external actions, foreign owners, live or rebound environments, accepted results, changed evidence, and unrelated later runs. - The sweep examines at most five candidates per pass. It uses a fair cursor and bounded transaction waits. It performs no provider call and starts no new turn. - Intervening verification reads only the needed JSON fields. The source transaction still serializes recovery for the same task. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, tool use and code execution. The session does not expose an exact backend model 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>
105 lines
7.1 KiB
TypeScript
105 lines
7.1 KiB
TypeScript
// @vitest-environment jsdom
|
|
import { act } from "react";
|
|
import { createRoot, type Root } from "react-dom/client";
|
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
|
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import type { ExecutionBlocker, Issue } from "@paperclipai/shared";
|
|
import { WorkspaceFileRecovery } from "./WorkspaceFileRecovery";
|
|
const resolve = vi.hoisted(() => vi.fn());
|
|
vi.mock("../api/issues", () => ({ issuesApi: { resolveRecoveryAction: resolve } }));
|
|
const baseBlocker = { recoveryActionId: "held-source", runId: "original-run",
|
|
workspaceRepairRequired: true, canRecordWorkspaceRepair: true, canRetry: false, canContinue: false, nextAction: "Recover the original files." } as ExecutionBlocker;
|
|
|
|
describe.each(["legacy_execution_requires_reconciliation", "native_continuation_requires_reconciliation"] as const)("workspace file recovery: %s", cause => {
|
|
const blocker = { ...baseBlocker, cause };
|
|
let root: Root, container: HTMLDivElement, client: QueryClient;
|
|
const onRecorded = vi.fn();
|
|
beforeEach(() => {
|
|
resolve.mockReset().mockResolvedValue({}); onRecorded.mockReset();
|
|
container = document.createElement("div"); document.body.append(container); root = createRoot(container);
|
|
client = new QueryClient({ defaultOptions: { mutations: { retry: false } } });
|
|
});
|
|
afterEach(async () => { await act(async () => root.unmount()); client.clear(); container.remove(); });
|
|
async function mount(overrides: Partial<Parameters<typeof WorkspaceFileRecovery>[0]> = {}) {
|
|
await act(async () => root.render(<QueryClientProvider client={client}><WorkspaceFileRecovery
|
|
issueId="task" issueStatus="in_progress" blocker={blocker} canManage onRecorded={onRecorded} {...overrides} />
|
|
</QueryClientProvider>));
|
|
}
|
|
async function enterEvidence() {
|
|
const textarea = container.querySelector("textarea")!;
|
|
await act(async () => {
|
|
Object.getOwnPropertyDescriptor(HTMLTextAreaElement.prototype, "value")!.set!.call(textarea,
|
|
"Recovered and verified both repository patches in the linked backup artifact.");
|
|
textarea.dispatchEvent(new Event("input", { bubbles: true }));
|
|
});
|
|
}
|
|
async function confirm() { await act(async () => container.querySelector<HTMLButtonElement>('[role="checkbox"]')!.click()); }
|
|
async function submit() {
|
|
await act(async () => Array.from(container.querySelectorAll("button")).find(b => b.textContent === "Record file recovery")!.click());
|
|
await act(async () => { await new Promise(resolve => setTimeout(resolve, 10)); });
|
|
}
|
|
it("records an optional verified no-action outcome only for the selected cancelled native run", async () => {
|
|
await mount({ blocker: { ...blocker, runStatus: "cancelled" } });
|
|
await enterEvidence(); await confirm();
|
|
const label = Array.from(container.querySelectorAll("label"))
|
|
.find(node => node.textContent === "I verified that this run performed no external actions.");
|
|
if (cause === "native_continuation_requires_reconciliation") {
|
|
expect(label).toBeDefined();
|
|
const input = document.getElementById(label!.htmlFor)!;
|
|
await act(async () => input.click());
|
|
} else expect(label).toBeUndefined();
|
|
await submit();
|
|
expect(resolve).toHaveBeenCalledWith("task", expect.objectContaining({ executionReconciliation: expect.objectContaining({
|
|
actionOutcome: cause === "native_continuation_requires_reconciliation" ? "not_performed" : "mixed",
|
|
}) }));
|
|
});
|
|
it("leaves external action outcomes uncertified by default for cancelled runs", async () => {
|
|
await mount({ blocker: { ...blocker, runStatus: "cancelled" } });
|
|
await enterEvidence(); await confirm(); await submit();
|
|
expect(resolve).toHaveBeenCalledWith("task", expect.objectContaining({ executionReconciliation: expect.objectContaining({
|
|
actionOutcome: "mixed", outcomeEvidence: expect.stringContaining("does not replay the source run or certify its external action outcomes"),
|
|
}) }));
|
|
});
|
|
it.each(["runId", "recoveryActionId"] as const)("clears source evidence and confirmations when %s changes", async field => {
|
|
await mount({ blocker: { ...blocker, runStatus: "cancelled" } });
|
|
await enterEvidence(); await confirm();
|
|
const label = Array.from(container.querySelectorAll("label"))
|
|
.find(node => node.textContent === "I verified that this run performed no external actions.");
|
|
if (label) await act(async () => document.getElementById(label.htmlFor)!.click());
|
|
await mount({ blocker: { ...blocker, runStatus: "cancelled", [field]: "another-source" } });
|
|
expect(container.querySelector("textarea")?.value).toBe("");
|
|
expect(Array.from(container.querySelectorAll('[role="checkbox"]')).every(node => node.getAttribute("aria-checked") === "false")).toBe(true);
|
|
expect(Array.from(container.querySelectorAll("button")).find(node => node.textContent === "Record file recovery")?.disabled).toBe(true);
|
|
expect(resolve).not.toHaveBeenCalled();
|
|
});
|
|
it.each(["blocked", "in_progress", "in_review", "done", "cancelled"] as Issue["status"][])("records verified source files without changing the %s task", async issueStatus => {
|
|
await mount({ issueStatus });
|
|
const button = Array.from(container.querySelectorAll("button")).find(b => b.textContent === "Record file recovery")!;
|
|
expect(button.disabled).toBe(true); await enterEvidence(); expect(button.disabled).toBe(true);
|
|
await confirm(); expect(button.disabled).toBe(false); await submit();
|
|
expect(resolve).toHaveBeenCalledWith("task", expect.objectContaining({ actionId: "held-source", sourceIssueStatus: issueStatus,
|
|
outcome: "restored", executionReconciliation: expect.objectContaining({ runId: "original-run", providerStopped: true,
|
|
workspaceRepairEvidence: "Recovered and verified both repository patches in the linked backup artifact." }) }));
|
|
expect(onRecorded).toHaveBeenCalledOnce();
|
|
expect(container.querySelector('[role="status"]')?.textContent).toContain("task status is unchanged");
|
|
});
|
|
it("keeps the hold and entered evidence when the server rejects unverified shutdown", async () => {
|
|
resolve.mockRejectedValueOnce(new Error("The original workspace has not confirmed that it stopped."));
|
|
await mount(); await enterEvidence(); await confirm(); await submit();
|
|
expect(container.querySelector('[role="alert"]')?.textContent).toContain("not confirmed");
|
|
expect(container.querySelector("textarea")?.value).toContain("both repository patches");
|
|
expect(onRecorded).not.toHaveBeenCalled();
|
|
});
|
|
it("does not offer attestation without board runtime access", async () => {
|
|
await mount({ canManage: false }); expect(container.querySelector("button")).toBeNull();
|
|
expect(container.textContent).toContain("board member with runtime access"); expect(resolve).not.toHaveBeenCalled();
|
|
});
|
|
it.each([
|
|
{ workspaceRepairRequired: false }, { canRecordWorkspaceRepair: false }, { cause: "native_workspace_sync_out_retry_exhausted" },
|
|
{ recoveryActionId: null }, { runId: null },
|
|
])("does not expose file-only repair for other or unbound holds: %j", async patch => {
|
|
await mount({ blocker: { ...blocker, ...patch } }); expect(container.textContent).toBe("");
|
|
expect(resolve).not.toHaveBeenCalled();
|
|
});
|
|
});
|