Files
PaperClipAI/tests/e2e/instruction-working-copy/lifecycle.spec.ts
DottaandPaperclip ce08872dfb refactor(heartbeat): extract instruction working-copy lifecycle (#15984)
## 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>
2026-10-12 10:52:32 -05:00

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 }); }
});