mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 04:25:52 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native agents keep remote processes and conversation history across warm turns. > - A completed turn can leave handoff authority only in controller memory. > - Losing that controller can block a follow-up even while its sandbox survives. > - This pull request verifies durable ownership and reconnects the exact prior process before handing it to the next run. > - Healthy follow-ups stay warm. Recovery preserves saved work without replaying a completed turn. ## Linked Issues or Issue Description **What happened?** A remote warm turn can finish without suspended backup authority. A replacement controller rejects that prior owner even when its exact process and completed checkpoint remain. A single unavailable identity probe can also stop an otherwise valid reattachment. **Expected behavior** Keep healthy follow-ups warm. Recover a completed session after proving that its controller stopped and that the successor holds current authority. Reuse compatible processes; otherwise save and resume the conversation. Retry unavailable identity reads within one shared deadline. **Steps to reproduce** Use `doc/testing-native-remote-continuity.md`. Its Linux cases execute real turns through PostgreSQL, TLS ingress, and the Rust runner. They kill a separate controller process, recover old checkpoint metadata, and change process configuration. The provider is deterministic. Related: #16432 and #16441 fix live warm-owner handoffs. #16341 adds recovery diagnostics. #14695 covers provider process retention. ## What Changed - Preserve warm process reuse between healthy remote turns. - Verify a committed prior result, stopped controller, matching checkpoint and sandbox, and current task/coordinator ownership before recovering an orphaned warm session. - Reconnect through the prior run route, then use the existing authenticated run attachment to change authority. - Persist a process-configuration digest. Close and save recovered sessions when old or changed configuration cannot support reuse. - Retry unavailable state, marker, and process reads within one 30-second deadline. Keep signals and definitive identity failures single-attempt. - Record closed recovery diagnostics, including unreadable or malformed saved state. - Retire the recovery-only ownership check after verified attachment, so later warm turns use their current authority. - Add real-process Linux regressions and refusal controls. - Expose the existing source-only file-recovery form for retained native holds, with the same board authority, evidence, and exact stop-receipt requirements as legacy holds. Recording recovered files does not change task status or replay a run. ## Verification - `pnpm -r typecheck` passed; the server check was repeated for the third-turn fix. - `pnpm build` passed after the final UI change. The UI typecheck, token gates, and map checks also pass. - Four Linux continuity cases passed: warm reuse, controller loss, legacy checkpoint metadata, and changed configuration. Each creates one conversation and starts exactly three turns, including a healthy follow-up after recovery. The fixture uses expired prior leases; the third turn keeps the runner and provider processes. - Greptile caught the retained recovery gate; the third-turn regression now covers it. An initial test omitted the heartbeat’s saved input and left a test process after failure; the fixture was corrected and its disposable container reset before the passing run. - The current focused executor/restart group passes all 749 tests, including the original failure classifications. - Native file-recovery and UI tests pass (52 tests), including blocked tasks and both native and legacy holds. - After merging current master and retaining its equivalent CI expression check, the CI policy tests pass (15 tests) and all four three-turn Linux continuity cases pass again on `dec418ae55ff21b4fb067132613658cce445f181`. - `pnpm test:run` completed its general server group with 20,572 passes, 103 skips, and three failures: the skill-audit child process and two HTTP test timeouts. The same three suites then passed in isolation (89 tests) without code changes. That local command stopped before later groups; the complete final-head CI, including general, serialized, Runner, build, and browser gates, is green. - Greptile reviewed the combined head `dec418ae55ff21b4fb067132613658cce445f181` at 5/5 with no actionable findings; both earlier findings are resolved. All final-head CI gates and the security scan passed. Staging deployment and retained-incident disposition remain deployment acceptance work. ## Risks - Recovery requires exact durable evidence. Live conflicting controllers, replacement sandboxes, Stop/pause, and uncertain effects must remain blocked. - After controller loss, tool-contract or process-configuration changes can restart the provider. Normal healthy follow-ups retain their processes. - The new idle-recovery path has Linux fake-provider evidence. Live Codex/OpenCode and retained quarantined-state recovery still need staging qualification. - No database schema, telemetry, or runner protocol changes. ## Model Used OpenAI GPT-6 through Codex, with reasoning, repository inspection, code execution, and browser tools. The session does not expose a more specific model version 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 (targeted checks and isolated reruns; full-run limitation recorded above) - [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 on `dec418ae55ff21b4fb067132613658cce445f181` - [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>
71 lines
4.8 KiB
TypeScript
71 lines
4.8 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.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();
|
|
});
|
|
});
|