mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
Preserve unbound legacy workspaces and contain failed final saves
Bundle the task disposition helper with the installed Paperclip skill. Retry transient read-only file transfers within a fixed deadline and retain sandbox RPC caller provenance for staging failure diagnosis. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
f4a4916216
commit
e60c6099c5
14 files changed
+419
-208
No files matched your search
@@ -2,14 +2,16 @@ import { spawn } from "node:child_process";
|
||||
import http from "node:http";
|
||||
import type { AddressInfo } from "node:net";
|
||||
import path from "node:path";
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
|
||||
// End-to-end coverage for scripts/paperclip-issue-update.sh: the helper must
|
||||
// End-to-end coverage for the installed runtime skill helper: the helper must
|
||||
// only exit 0 when the server confirms the write by echoing the update, must
|
||||
// classify failures (retry connection-level faults and 5xx, never retry a
|
||||
// definitive 4xx), and must stop at two attempts total to honor the shared
|
||||
// bounded-write-retry rule.
|
||||
const HELPER_PATH = path.resolve("scripts/paperclip-issue-update.sh");
|
||||
const HELPER_PATH = path.resolve("skills/paperclip/scripts/paperclip-issue-update.sh");
|
||||
|
||||
interface HelperResult {
|
||||
code: number | null;
|
||||
@@ -66,9 +68,10 @@ describe("paperclip issue update helper", () => {
|
||||
return { baseUrl: `http://127.0.0.1:${port}`, requests };
|
||||
}
|
||||
|
||||
function runHelper(apiUrl: string, args: string[]): Promise<HelperResult> {
|
||||
function runHelper(apiUrl: string, args: string[], helperPath = HELPER_PATH, cwd?: string): Promise<HelperResult> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const child = spawn("bash", [HELPER_PATH, ...args], {
|
||||
const child = spawn("bash", [helperPath, ...args], {
|
||||
cwd,
|
||||
env: {
|
||||
...process.env,
|
||||
PAPERCLIP_API_URL: apiUrl,
|
||||
@@ -109,6 +112,25 @@ describe("paperclip issue update helper", () => {
|
||||
expect(JSON.parse(requests[0]?.body ?? "{}")).toEqual({ status: "done", comment: "closing note" });
|
||||
});
|
||||
|
||||
it("works after installing only the skill into a sandbox home", async () => {
|
||||
const home = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-installed-skill-"));
|
||||
cleanupFns.push(() => fs.rm(home, { recursive: true, force: true }));
|
||||
const skillDir = path.join(home, ".codex", "skills", "paperclip");
|
||||
await fs.cp(path.resolve("skills/paperclip"), skillDir, { recursive: true });
|
||||
const { baseUrl, requests } = await startServer((_request, _attempt, res) => {
|
||||
res.writeHead(200, { "content-type": "application/json" });
|
||||
res.end(JSON.stringify({ id: "issue-1", status: "done" }));
|
||||
});
|
||||
const result = await runHelper(baseUrl, doneArgs, path.join(skillDir, "scripts", "paperclip-issue-update.sh"), home);
|
||||
expect(result.code).toBe(0);
|
||||
expect(JSON.parse(result.stdout)).toMatchObject({ id: "issue-1", status: "done" });
|
||||
expect(requests).toHaveLength(1);
|
||||
await expect(fs.access(path.join(home, "scripts"))).rejects.toThrow();
|
||||
const repositoryWrapper = await runHelper(baseUrl, doneArgs, path.resolve("scripts/paperclip-issue-update.sh"), home);
|
||||
expect(repositoryWrapper.code).toBe(0);
|
||||
expect(requests).toHaveLength(2);
|
||||
});
|
||||
|
||||
it("fails an empty 2xx body instead of treating it as success", async () => {
|
||||
const { baseUrl } = await startServer((_request, _attempt, res) => {
|
||||
res.writeHead(200, { "content-length": "0" });
|
||||
|
||||
@@ -249,6 +249,10 @@ describe("plugin-worker-manager stderr failure context", () => {
|
||||
await expect(pendingCall).rejects.toBeInstanceOf(JsonRpcCallError);
|
||||
await expect(pendingCall).rejects.toMatchObject({
|
||||
message: expect.stringContaining("terminated"),
|
||||
cause: expect.objectContaining({
|
||||
message: "Sandbox command requested here",
|
||||
stack: expect.stringContaining("plugin-worker-manager.test.ts"),
|
||||
}),
|
||||
});
|
||||
expect(unhandledRejection).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
|
||||
@@ -10,6 +10,7 @@ import { agents, assets, companyMemberships, issueAttachments, companies, create
|
||||
import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js";
|
||||
import { prepareSandboxWorkFolders } from "../services/sandbox-work-folders.js";
|
||||
import { bindWarmSandboxWorkspace } from "../services/sandbox-workspace-binding.js";
|
||||
import { findUnboundLegacyTaskWorkspace } from "../services/legacy-sandbox-workspace.js";
|
||||
import { retainUnsavedWorkFolderLease, workFolderSandboxKey } from "../services/work-folder-retention.js";
|
||||
import * as activityLog from "../services/activity-log.js";
|
||||
import { workFolderService } from "../services/work-folders.js";
|
||||
@@ -51,6 +52,47 @@ describe("shared sandbox work-folder lifecycle", () => {
|
||||
for (const run of active) await run.stop().catch(() => {});
|
||||
await database?.cleanup(); if (root) await fs.rm(root, { recursive: true, force: true });
|
||||
});
|
||||
it.each(["codex_local", "paperclip_runner"])("recovers an unbound pre-change %s workspace only for its recorded identity", async (adapterType) => {
|
||||
const task = randomUUID(), runId = randomUUID(), workspaceId = randomUUID(), leaseId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, responsibleUserId: "owner",
|
||||
status: "succeeded", contextSnapshot: { issueId: task } });
|
||||
await db.insert(issues).values({ id: task, companyId, projectId, title: "Unbound old task", assigneeAgentId: agentId });
|
||||
await db.insert(executionWorkspaces).values({ id: workspaceId, companyId, projectId, sourceIssueId: task,
|
||||
mode: "shared_workspace", strategyType: "project_primary", name: "Retained old workspace" });
|
||||
const metadata = { driver: "sandbox", agentId, reusableSandboxLease: { version: 1, companyId, agentId,
|
||||
environmentId, executionWorkspaceId: workspaceId, adapterType, provider: "daytona" } };
|
||||
await db.insert(environmentLeases).values({ id: leaseId, companyId, environmentId, issueId: task,
|
||||
executionWorkspaceId: workspaceId, heartbeatRunId: runId, status: "retained", leasePolicy: "reuse_by_environment",
|
||||
provider: "daytona", providerLeaseId: "original-sandbox", metadata });
|
||||
const input = { companyId, issueId: task, projectId, agentId, responsibleUserId: "owner", adapterType,
|
||||
executionWorkspaceId: null, executionWorkspacePreference: null,
|
||||
environment: { id: environmentId, driver: "sandbox" as const, config: { reuseLease: true } } };
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBe(workspaceId);
|
||||
for (const bad of [{ companyId: randomUUID() }, { issueId: randomUUID() }, { projectId: randomUUID() },
|
||||
{ agentId: randomUUID() }, { responsibleUserId: null }, { responsibleUserId: "another-user" },
|
||||
{ adapterType: "other-adapter" }, { executionWorkspaceId: randomUUID() },
|
||||
{ executionWorkspacePreference: "create_new" }, { environment: null },
|
||||
{ environment: { ...input.environment, id: randomUUID() } },
|
||||
{ environment: { ...input.environment, driver: "local" as const } },
|
||||
{ environment: { ...input.environment, config: { reuseLease: false } } }]) {
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, { ...input, ...bad })).toBeNull();
|
||||
}
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: randomUUID() } }).where(eq(heartbeatRuns.id, runId));
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull();
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: task } }).where(eq(heartbeatRuns.id, runId));
|
||||
await db.update(executionWorkspaces).set({ status: "archived" }).where(eq(executionWorkspaces.id, workspaceId));
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull();
|
||||
await db.update(executionWorkspaces).set({ status: "active" }).where(eq(executionWorkspaces.id, workspaceId));
|
||||
await db.update(environmentLeases).set({ metadata: { ...metadata,
|
||||
reusableSandboxLease: { ...metadata.reusableSandboxLease, executionWorkspaceId: randomUUID() } } }).where(eq(environmentLeases.id, leaseId));
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull();
|
||||
await db.update(environmentLeases).set({ metadata }).where(eq(environmentLeases.id, leaseId));
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBe(workspaceId);
|
||||
await db.insert(workFolderRuns).values({ runId, companyId, manifest: { version: 1, companyId, runId,
|
||||
taskId: task, projectId, agentId, responsibleUserId: "owner", leaseId, sandboxKey: leaseId,
|
||||
home: "/home/daytona", folders: { task: null, agent: null, user: null, project: null }, repositories: [] } });
|
||||
expect(await findUnboundLegacyTaskWorkspace(db, input)).toBeNull();
|
||||
});
|
||||
it("keeps the host's warm task binding without enabling user-configurable worktrees", async () => {
|
||||
const task = randomUUID(), runId = randomUUID(), workspaceId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: runId, companyId, agentId, status: "running" });
|
||||
|
||||
@@ -31,4 +31,17 @@ describe("shared work folder checkpoint cadence", () => {
|
||||
expect(onError).toHaveBeenCalledWith(error);
|
||||
await expect(sync.stop()).rejects.toThrow("Storage unavailable");
|
||||
});
|
||||
it("does not restart a failed final save during error teardown", async () => {
|
||||
vi.useFakeTimers();
|
||||
const checkpoint = vi.fn().mockRejectedValueOnce(new Error("socket hang up")).mockResolvedValue(undefined);
|
||||
const onError = vi.fn().mockResolvedValue(undefined);
|
||||
const sync = startWorkFolderCheckpointer({ checkpoint, onError });
|
||||
const firstStop = sync.stop();
|
||||
await expect(firstStop).rejects.toThrow("socket hang up");
|
||||
expect(sync.stop()).toBe(firstStop);
|
||||
await expect(sync.stop()).rejects.toThrow("socket hang up");
|
||||
await vi.advanceTimersByTimeAsync(360_000);
|
||||
expect(checkpoint).toHaveBeenCalledTimes(1);
|
||||
expect(onError).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { execFile } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
import { mkdtemp, mkdir, readFile, realpath, rm, symlink, writeFile } from "node:fs/promises";
|
||||
@@ -18,6 +18,39 @@ describe("sandbox work folder transport with real Node and Git", () => {
|
||||
roots.push(dir); return dir;
|
||||
}
|
||||
afterEach(async () => { for (const dir of roots.splice(0)) await rm(dir, { recursive: true, force: true }); });
|
||||
it("retries a lost read response without duplicating streamed bytes", async () => {
|
||||
const dir = await root();
|
||||
const body = Buffer.alloc(700_000, "x");
|
||||
await writeFile(path.join(dir, "file"), body);
|
||||
let lostResponse = false;
|
||||
const execute = vi.fn(async (input: Parameters<typeof localTestWorkFolderRunner.execute>[0]) => {
|
||||
const result = await localTestWorkFolderRunner.execute(input);
|
||||
const request = JSON.parse(Buffer.from(input.args!.at(-1)!, "base64").toString());
|
||||
if (request.operation === "read" && request.offset > 0 && !lostResponse) {
|
||||
lostResponse = true;
|
||||
throw Object.assign(new Error("socket hang up"), { code: "ECONNRESET" });
|
||||
}
|
||||
return result;
|
||||
});
|
||||
const chunks = [];
|
||||
for await (const chunk of workFolderTransport({ execute }).read(dir, "file", body.length)) chunks.push(chunk);
|
||||
expect(lostResponse).toBe(true);
|
||||
expect(Buffer.concat(chunks)).toEqual(body);
|
||||
const offsets = execute.mock.calls.map(([input]) => JSON.parse(Buffer.from(input.args!.at(-1)!, "base64").toString()).offset);
|
||||
expect(offsets.filter((offset) => offset === offsets[1])).toHaveLength(2);
|
||||
});
|
||||
it("bounds read retries and does not replay mutations or validation failures", async () => {
|
||||
const execute = vi.fn().mockRejectedValue(new Error("socket hang up"));
|
||||
const retrying = workFolderTransport({ execute });
|
||||
await expect(retrying.scan("/home/daytona/task")).rejects.toThrow("socket hang up");
|
||||
expect(execute).toHaveBeenCalledTimes(3);
|
||||
execute.mockClear();
|
||||
await expect(retrying.moveRoot("/old", "/new")).rejects.toThrow("socket hang up");
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
execute.mockReset().mockResolvedValue({ exitCode: 1, stdout: "", stderr: "symlink_not_allowed", timedOut: false });
|
||||
await expect(retrying.scan("/home/daytona/task")).rejects.toThrow("symlink_not_allowed");
|
||||
expect(execute).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
it("streams and atomically publishes files larger than a transfer chunk", async () => {
|
||||
const dir = await root();
|
||||
const staging = await root();
|
||||
|
||||
Reference in new issue
Block a user