mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
## Thinking Path > - Paperclip manages AI agents and the work assigned to them. > - A task can retain saved input after a failure during execution preparation. > - Retry must prove that the old attempt stopped and its files are saved before it admits a new turn. > - The host can write a skill announcement before it dispatches a provider. > - The historical preparation verifier requires an empty log and rejects this safe host output. > - This pull request verifies that exact small log against the saved skill selection and immutable log facts. > - Eligible saved input can continue once. Uncertain execution and current policy gates stay held. ## Linked Issues or Issue Description Refs: #19004, #19464, #19552. These repairs establish the existing preparation proof. They do not accept the host skill announcement described here. A search for the exact announcement found no duplicate repair. This is a bug fix within the existing recovery contract and does not duplicate a roadmap feature. **What happened?** A legacy preparation failed before provider dispatch with a host bootstrap certificate. Its remote lease stopped and its files were recovered. A newer preparation failed while that save was in progress. An accepted Retry then remained in execution recovery because its ancestor had one host skill log record. **Expected behavior** Admit one fresh turn for the existing saved user request after all execution, workspace and policy proofs pass. Preserve old outcomes and source records. **Steps to reproduce** 1. Create the real PostgreSQL fixture in `saved-bootstrap-continuation.test.ts`. It includes a recovered remote workspace, exact stop receipt, bootstrap ancestor, later failed preparation and saved user status queue. 2. Call `requestTaskRetry`, then call the real resume path concurrently. 3. On the baseline guard, the empty-log control dispatches. Adding only the host skill announcement leaves zero queued successors and the same execution-recovery wait. **Paperclip version or commit** Reproduced on master `65aee998a3`. The final branch rebases cleanly onto `c650f36007`. **Deployment mode** Source checkout with real embedded PostgreSQL and controlled log storage. The reported runtime is a server with remote sandbox workspaces. ## What Changed - Add a narrow verifier for one host skill-announcement record. It binds the complete bytes to the run log reference, digest, size, sequence, timestamp, output excerpt and recorded skill selection. Each announced skill must have a saved version; unrelated configured skills may also be selected. - Read at most 2,049 bytes with a two-second cancellation deadline. Missing, slow, changed, additional or provider output fails closed. - Use this proof only for historical bootstrap ancestors that already pass the no-provider, settlement, stop and saved-file checks. - Add real admission regressions for one successor, concurrent resume, duplicate Retry and delivery of the original saved comment. Add current-gate and storage-boundary negative cases. - Document the continuation contract. ## Verification - **RED:** On the unchanged baseline guard, the exact final fixture passes the empty-log control and fails the host-announcement case: zero successors instead of one. The saved comment and Retry remain deferred. - **GREEN:** 486/486 tests pass across `task-retry.test.ts`, `explicit-native-continuation.test.ts`, `saved-bootstrap-continuation.test.ts` and `verified-preparation-log.test.ts`. - The new cases cover provider-started evidence, provider events, a native coordinator, missing stop proof, missing skill selection, changed hash, unknown output, pending approval and operator Stop. - Read-boundary cases cover foreign references, changed bytes, truncation, extra records, malformed selection and cancellation. An unrelated recovery completes while another read is stalled. - After the rebase, the original 27 new cases passed again at head `be640292fe136232e3db0dc6938233f53b6ccb43`. The review fix adds three cases, and the full 486-test pass includes all 30 new cases. - The exact previously observed historical input also passes the production log verifier: one read, 240 bytes matching the stored size and SHA, the same timestamp and output sequence, and a recorded version for the announced skill. This uses captured bytes for transport and does not claim live hosted admission. - Full local server layered typecheck passes after building this checkout's Runner declarations and normal excluded-plugin SDK links. The typing correction also passes all 30 new cases and the actual captured historical log proof. The prior CI build/typecheck failure was TS7006 in the JSON parser; it is retained as failed evidence and corrected in this revision. - Module boundary and whitespace checks pass. Local verification selection requires broad CI because the execution contract changed; focused local suites are recorded above. [Required final-head CI](https://github.com/paperclipai/paperclip/actions/runs/42281590449) typecheck, build and tests pass at `c557cdc4dc57000cb478f9be0717675dcac7364e` (56 successful checks, no required check pending or failed). Greptile rates this final revision 5/5; the addressed thread is resolved. The advisory hosted Paperclip Review is paused with its request saved and has not supplied a final-head assessment. Its pending status is disclosed separately from required CI and Greptile. - Controlled fixtures exercise the admission and dispatch receipts. Hosted continuation still requires rollout and reassessment of the existing request. ## Risks This adds one bounded storage read to an active historical admission check. Failed or uncertain reads keep the existing hold. The host bootstrap certificate remains the primary no-provider proof. Log text alone cannot authorize continuation. No schema, retry budget, provider setting, account selection or scheduler changes. ## Model Used OpenAI GPT-6 family via Codex, with coding and tool use. The exact model ID and context window are unavailable in this session's metadata. ## 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>
85 lines
5.3 KiB
TypeScript
85 lines
5.3 KiB
TypeScript
import { createHash, randomUUID } from "node:crypto";
|
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { hasVerifiedPreparationLog } from "./verified-preparation-log.js";
|
|
|
|
const fixture = vi.hoisted(() => ({ read: vi.fn() }));
|
|
vi.mock("./run-log-store.js", () => ({ getRunLogStore: () => ({ read: fixture.read }) }));
|
|
|
|
function source() {
|
|
const companyId = randomUUID(), agentId = randomUUID(), id = randomUUID(), ts = new Date();
|
|
const chunk = "[paperclip] Enabled run-scoped skills from issue mentions: fixture/status-report\n";
|
|
const record = { ts: ts.toISOString(), attemptId: randomUUID(), stream: "stdout", chunk, seq: 1 };
|
|
const content = JSON.stringify(record) + "\n";
|
|
const run: Parameters<typeof hasVerifiedPreparationLog>[0] = { companyId, agentId, id,
|
|
contextSnapshot: { skillSelectionSnapshot: true, skillVersions: { "fixture/status-report": randomUUID() } },
|
|
logStore: "s3_segments", logRef: `${companyId}/${agentId}/${id}`, logBytes: Buffer.byteLength(content),
|
|
logSha256: createHash("sha256").update(content).digest("hex"), logCompressed: false,
|
|
stdoutExcerpt: chunk, stderrExcerpt: "", lastOutputAt: ts, lastOutputSeq: 1,
|
|
lastOutputStream: "stdout", lastOutputBytes: Buffer.byteLength(content) };
|
|
return { run, content, record };
|
|
}
|
|
|
|
describe("verified host preparation output", () => {
|
|
beforeEach(() => { fixture.read.mockReset(); });
|
|
it("verifies the complete canonical bytes against the persisted selection and output watermark", async () => {
|
|
const f = source();
|
|
fixture.read.mockResolvedValue({ content: f.content });
|
|
expect(await hasVerifiedPreparationLog(f.run)).toBe(true);
|
|
expect(fixture.read).toHaveBeenCalledTimes(1);
|
|
expect(fixture.read.mock.calls[0][1]).toMatchObject({ offset: 0 });
|
|
expect(fixture.read.mock.calls[0][1].limitBytes).toBeLessThan(4096);
|
|
expect(fixture.read.mock.calls[0][1].signal.aborted).toBe(true);
|
|
});
|
|
|
|
it.each(["versioned", "unversioned"])("verifies mentioned skills when the saved selection also has %s configured skills", async kind => {
|
|
const f = source();
|
|
f.run.contextSnapshot!.skillVersions = { "fixture/configured-skill": kind === "versioned" ? randomUUID() : null,
|
|
"fixture/status-report": randomUUID() };
|
|
fixture.read.mockResolvedValue({ content: f.content });
|
|
expect(await hasVerifiedPreparationLog(f.run)).toBe(true);
|
|
});
|
|
|
|
it.each(["missing_selection", "different_selection", "malformed_version", "foreign_log", "changed_hash",
|
|
"truncated", "extra_record", "provider_output", "changed_timestamp", "extra_provenance", "larger_log",
|
|
"other_stream", "changed_watermark", "missing_log"] as const)("keeps uncertain output held: %s", async state => {
|
|
const f = source();
|
|
let content = f.content;
|
|
if (state === "missing_selection") f.run.contextSnapshot = null;
|
|
if (state === "different_selection") f.run.contextSnapshot!.skillVersions = { "other/skill": randomUUID() };
|
|
if (state === "malformed_version") f.run.contextSnapshot!.skillVersions = { "fixture/status-report": "unbound" };
|
|
if (state === "foreign_log") f.run.logRef = `${randomUUID()}/${f.run.agentId}/${f.run.id}`;
|
|
if (state === "changed_hash") f.run.logSha256 = "b".repeat(64);
|
|
if (state === "larger_log") f.run.logBytes = 4096;
|
|
if (state === "other_stream") f.run.lastOutputStream = "stderr";
|
|
if (state === "changed_watermark") f.run.lastOutputSeq = 2;
|
|
if (state === "extra_record") content += JSON.stringify({ ...f.record, chunk: "Provider performed a tool action", seq: 2 }) + "\n";
|
|
if (state === "provider_output") { f.record.chunk = "Provider completed an action\n"; content = JSON.stringify(f.record) + "\n"; f.run.stdoutExcerpt = f.record.chunk; }
|
|
if (state === "changed_timestamp") content = JSON.stringify({ ...f.record, ts: new Date(0).toISOString() }) + "\n";
|
|
if (state === "extra_provenance") content = JSON.stringify({ ...f.record, sourceCursor: 1 }) + "\n";
|
|
if (["extra_record", "provider_output", "changed_timestamp", "extra_provenance"].includes(state)) {
|
|
f.run.logSha256 = createHash("sha256").update(content).digest("hex");
|
|
f.run.logBytes = f.run.lastOutputBytes = Buffer.byteLength(content);
|
|
}
|
|
if (state === "missing_log") fixture.read.mockRejectedValue(new Error("Object log not found"));
|
|
else fixture.read.mockResolvedValue({ content, ...(state === "truncated" ? { nextOffset: f.run.logBytes } : {}) });
|
|
expect(await hasVerifiedPreparationLog(f.run)).toBe(false);
|
|
});
|
|
|
|
it("cancels a slow read, fails closed, and leaves an unrelated recovery responsive", async () => {
|
|
const slow = source(), fast = source();
|
|
let signal: AbortSignal | undefined;
|
|
fixture.read.mockImplementation((handle, options) => {
|
|
if (handle.logRef === fast.run.logRef) return Promise.resolve({ content: fast.content });
|
|
signal = options.signal;
|
|
return new Promise((_, reject) => options.signal.addEventListener("abort", () => reject(new Error("aborted")), { once: true }));
|
|
});
|
|
const pending = hasVerifiedPreparationLog(slow.run);
|
|
const ready = hasVerifiedPreparationLog(fast.run);
|
|
expect(await Promise.race([pending.then(() => "slow"), ready.then(() => "fast")])).toBe("fast");
|
|
expect(await ready).toBe(true);
|
|
expect(await pending).toBe(false);
|
|
expect(signal?.aborted).toBe(true);
|
|
expect(fixture.read).toHaveBeenCalledTimes(2);
|
|
});
|
|
});
|