mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
## Thinking Path > - Paperclip runs agent tasks and preserves their managed agent files. > - Heartbeat must prepare a private copy, collect stopped-provider edits, and retain ownership during warm-session handoff. > - That lifecycle is spread across preparation, adapter callbacks, and cleanup. > - A per-run controller can own the copy and receipt while the executor retains teardown ordering. > - This PR moves that coordination into a dedicated module. > - Live getters preserve updated state and partially acquired resources. ## Linked Issues or Issue Description **What existing behavior does this improve?** Maintenance of heartbeat instruction preparation. This is the second and final PR in this batch, originally stacked on #15980. That dependency is now merged; this PR is rebased onto its squash merge with an identical patch and identical touched-file contents. Related-PR search found no duplicate extraction. **Current behavior** After #15980, `heartbeat.ts` has 4,215 lines and 173,657 bytes. It coordinates instruction-copy preparation, save receipts, warm handoffs, and deferred cleanup inline. **Proposed behavior** `heartbeat/instruction-working-copy.ts` owns the per-run lifecycle in 233 lines. `heartbeat.ts` becomes 4,055 lines and 162,520 bytes. The executor retains the outer release order, unavailable-copy reporting, and environment/service cleanup policy. **Reason and benefit** The copy, save receipt, and warm ownership are readable together. The remaining executor names preparation and dispatch phases without obscuring teardown decisions. **Breaking changes** None intended. All 140 named public heartbeat exports remain unchanged. ## What Changed - Extract copy preparation, save/checkpoint callbacks, warm reservation/adoption, prompt guidance, and the deferred-release closure. - Read the executor's current run row through a callback; expose current copies, receipts, and acquired cleanup handles through live getters. - Keep cleanup handles available after preparation failure, and preserve release-before-retry on invalidated warm reuse. - Add four public-path characterization tests with real disposable PostgreSQL and files, plus four direct boundary tests. - Add isolated browser acceptance with a real process that edits instructions, completes its task, and reads the saved bytes on its next task. - Document the boundary and opt-in acceptance command. ## Verification - **Before extraction:** four new public-path tests pass on the original implementation, as do 108 existing instruction-copy/directory/native-cleanup tests. The two-task browser journey also passes on the baseline server. - **After extraction:** all 112 baseline tests pass; all four new boundary tests pass; 12 selected native-session ownership/retirement cases pass (the other 670 cases were outside this focused selection). - **Browser acceptance:** the fresh extracted-code server passes the full two-task journey (25.9 seconds): real child-process edits, saved/unchanged receipts, canonical bytes through the agent-file API, next-task restore, completed task UI, and copy cleanup. - `pnpm -r typecheck`, final server typecheck, and `pnpm build` pass. Module-boundary and public-export checks pass. - Normalized syntax comparison confirms all moved bodies and the reconstructed executor match the base, accounting explicitly for live run getters and delegation. - Final rebased head: `a08872c97e587a21ffc0fc6b78f34b81e484e7b0`. Combined entry-point and boundary regressions pass again (12/12); fresh Greptile review is 5/5 with no findings. All 55 current-head checks pass (two configured skips), including the hosted full-suite tests, build, typecheck, and all eight browser shards. There are no open review threads. ## Risks - Acquisition can succeed before preparation throws. Boundary tests verify that both the reservation and a transferred copy remain visible to the cleanup owner. - Callbacks can replace copy/save state during execution. Public tests verify saved receipts; direct tests verify the current run row, warm checkpoint receipt, and permission-root preservation. - Missing edit authorization and cleanup failure must remain nonfatal. Characterization tests revoke actual membership after dispatch admission and inject a cleanup error while preserving the saved result. - The browser uses a test-launcher-only instruction-capable wrapper around the real process adapter. Production adapter capabilities are unchanged. No live provider calls are made; native warm ownership is covered by focused module and native-session tests rather than a live provider browser run. ## Model Used OpenAI Codex, based on GPT-6, assisted with reasoning, code edits, tool execution, and verification. The exact deployment model ID and context-window size are not exposed in this session. ## 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>
48 lines
3.2 KiB
TypeScript
48 lines
3.2 KiB
TypeScript
import { test, expect } from "@playwright/test";
|
|
import { mkdtemp, rm, stat } from "node:fs/promises";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
|
|
const base = process.env.INSTRUCTION_COPY_TEST_URL;
|
|
test.skip(!base, "Launch this suite's disposable instruction-capable test-drive; see README.md");
|
|
|
|
test("saves stopped-process edits, shows completed tasks, and restores saved bytes on the next task", async ({ page }, info) => {
|
|
expect(new URL(base!).hostname).toBe("127.0.0.1");
|
|
const api = async (route: string, method = "GET", body?: unknown) => {
|
|
const response = await page.request.fetch(`${base}/api${route}`, { method, data: body });
|
|
expect(response.ok(), await response.text()).toBe(true);
|
|
return response.json();
|
|
};
|
|
expect((await api("/health")).deploymentMode).toBe("local_trusted");
|
|
const [company] = await api("/companies");
|
|
expect(company.name).toBe("Instruction Lifecycle QA");
|
|
const cwd = await mkdtemp(path.join(os.tmpdir(), "instruction-acceptance-"));
|
|
try {
|
|
const agent = await api(`/companies/${company.id}/agents`, "POST", {
|
|
name: `Instruction worker ${path.basename(cwd)}`, role: "engineer", adapterType: "process",
|
|
adapterConfig: { command: process.execPath, args: [path.resolve(import.meta.dirname, "worker.mjs")], cwd },
|
|
instructionsBundle: { entryFile: "AGENTS.md", files: { "AGENTS.md": "# Original\n" } },
|
|
runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: true } },
|
|
});
|
|
for (const turn of [0, 1]) {
|
|
const issue = await api(`/companies/${company.id}/issues`, "POST", { title: `Persist instructions turn ${turn}`, status: "todo", assigneeAgentId: agent.id });
|
|
await page.goto(`${base}/${company.issuePrefix}/issues/${issue.identifier}`);
|
|
const runs = async () => (await api(`/companies/${company.id}/heartbeat-runs`)).filter((run: { contextSnapshot: { issueId?: string } }) => run.contextSnapshot?.issueId === issue.id);
|
|
await expect.poll(async () => (await runs()).some((run: { status: string }) => run.status === "succeeded"), { timeout: 45_000 }).toBe(true);
|
|
const history = await runs();
|
|
expect(history).toHaveLength(1);
|
|
const run = await api(`/heartbeat-runs/${history[0].id}`);
|
|
expect(run.resultJson.instructionSave).toMatchObject({ state: turn === 0 ? "saved" : "unchanged", contract: "agent_files", entryFile: "AGENTS.md" });
|
|
const outputLine = run.stdoutExcerpt.split("\n").find((line: string) => line.startsWith('{"before":'));
|
|
expect(outputLine).toBeTruthy();
|
|
const output = JSON.parse(outputLine!);
|
|
expect(output.before).toBe(turn === 0 ? "# Original\n" : "# Saved by the real worker\n☃\n");
|
|
expect((await api(`/agents/${agent.id}/instructions-bundle/file?path=AGENTS.md`)).content).toBe("# Saved by the real worker\n☃\n");
|
|
await expect.poll(async () => stat(output.instructionPath).then(() => true, () => false)).toBe(false);
|
|
await page.reload();
|
|
await expect(page.getByRole("button", { name: /^Change status \(current: Done/ }).first()).toBeVisible();
|
|
await page.screenshot({ path: info.outputPath(`completed-turn-${turn}.png`), fullPage: true });
|
|
}
|
|
} finally { await rm(cwd, { recursive: true, force: true }); }
|
|
});
|