mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 21:01:54 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A board user can stop or interrupt a native run and send a message for the next turn > - When the stop leaves sandbox files unsaved, automatic recovery saves them, re-stops the sandbox, and the saved message continues > - For agents whose sandbox is destroyed after each turn (an ephemeral lease), recovery then queues the destroy. That step removed the lease's stop receipt > - The continuation gate waits for a stop receipt, so the message waited for the destroy and its cleanup retry, about 5 to 8 minutes > - Agents with retained sandboxes continued within seconds, so the delay looked agent specific > - This pull request keeps the verified stop on the cleanup lease and lets the continuation and the next remote writer accept it > - The benefit is that a redirect, Retry or interrupt continues right after its files are saved, for every lease policy ## Linked Issues or Issue Description Refs #19206 **What happened?** On hosted staging, a Stop or `interrupt: true` comment on a native run left a follow-up turn that started 5 to 8 minutes later for some agents, and 25 to 40 seconds later for others. The slow agents use ephemeral sandbox leases. In each slow case, the stopped run's lease was released by `pending_cleanup_retry` about 5 minutes after recovery, and the saved message started one second later. The fast agents use retained leases, which recovery releases at once. **Expected behavior** The saved message continues right after recovery saves the files and the sandbox is stopped. The destroy of a stopped sandbox is resource cleanup and must not hold the next turn. **Steps to reproduce** 1. Use an agent whose sandbox lease is ephemeral (destroy after each turn). 2. Interrupt or stop its native run so that recovery must save the sandbox files. 3. Post a message. See the follow-up start only after the sandbox destroy finishes. **Paperclip version or commit** Hosted, at `56517dda`. ## What Changed - `server/src/services/workspace-recovery.ts`: `scheduleRecoveredWorkspaceCleanup` keeps the verified stop receipt as `workspaceRecoveryCleanup.stopReceipt` when it queues the destroy. - `server/src/services/remote-execution-termination.ts`: new `hasRecoveredCleanupStopReceipt(lease)`. It is true only when the lease has recovery cleanup for its own run, was released, and the carried receipt matches the lease, run, provider and provider lease id. - `server/src/services/explicit-native-continuation.ts`: the `remote_cleanup` gate accepts that receipt. - `server/src/services/execution-blocker.ts`: `hasPendingRunCleanup` ignores a lease with that valid receipt, so a saved native interrupt send (and Retry or fresh-workspace checks) does not wait for the destroy. Other cleanup holds are unchanged. - `server/src/services/remote-workspaces.ts`: the next remote writer accepts that receipt for the ended writer's lease. - `server/src/services/execution-blocker.ts` (from #19206 review, not in the merged head): pre-turn evidence uses the exact startup lifecycle event list, so turn-scoped events such as `workspace.diff.recorded` count as provider work. - `doc/execution-semantics.md`: describes the rule. ## Verification - Red before the fix, green after: `cd server && npx vitest run src/services/workspace-recovery.test.ts -t "admits the interrupt redirect"`. An interrupted native run with recovered files and a queued destroy blocked the user message with `remote_cleanup` before the fix. After the fix the message is admitted. A lease without the stop receipt, or with a receipt for another run, still blocks with `remote_cleanup`. The test runs the same input as a plain message and as a saved native interrupt send (`queuedCommentInterruptId`); `hasPendingRunCleanup` was true before the fix and blocked the interrupt send with `cleanup_pending`. - `cd server && npx vitest run src/services/remote-workspace-reservation.test.ts -t "ended writer"`: the next writer reserves the workspace from an ended writer whose recovered lease awaits destroy; a missing or foreign receipt still blocks. Removing the new branch fails the positive case. - Updated `saves the authoritative remote workspace after native failed ...`: Retry now continues before the destroy, and the cleanup lease carries the stop receipt for its run and lease. - `workspace-recovery.test.ts -t "provider work cannot be excluded"` includes `workspace.diff.recorded` as provider work. - Related suites pass: `workspace-recovery.test.ts`, `explicit-native-continuation.test.ts`, `remote-workspace-reservation.test.ts`, `remote-workspaces.integration.test.ts`, `remote-workspace-lifecycle.test.ts`, `task-retry*.test.ts`, `native-cancelled-workspace-preservation.test.ts`, `workspace-recovery-source.test.ts`. `cd server && npx tsc --noEmit` passes. - Hosted evidence (read only): slow cases have ephemeral leases released by `pending_cleanup_retry` about 5 minutes after `issue.workspace_recovered`, with settlement one second later. Fast cases have retained leases released by recovery. ## Risks - Low to medium. The next turn can start while the old stopped sandbox is still being destroyed. The receipt proves that the sandbox was stopped after the files were saved, and an ephemeral successor uses a new sandbox. A destroy failure stays with the existing cleanup retry. - Leases without the carried receipt behave as before. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, with tool use and code execution in Claude Code. ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
212 lines
14 KiB
TypeScript
212 lines
14 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { mkdtemp, rm } from "node:fs/promises";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
import express from "express";
|
|
import request from "supertest";
|
|
import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest";
|
|
import { eq } from "drizzle-orm";
|
|
import { agents, companies, companyMemberships, createDb, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS, environments,
|
|
environmentLeases, executionWorkspaces, heartbeatRuns, issues, projects, projectWorkspaces } from "@paperclipai/db";
|
|
import { readRemoteWorkspaceAuthority } from "@paperclipai/shared";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
|
import { createLocalDiskStorageProvider } from "../storage/local-disk-provider.js";
|
|
import { createStorageService } from "../storage/service.js";
|
|
import { instanceSettingsService } from "./instance-settings.js";
|
|
import { issueService } from "./issues.js";
|
|
import { reserveRemoteWorkspace } from "./remote-workspaces.js";
|
|
import { remoteTerminationReceipt } from "./remote-execution-termination.js";
|
|
import { executionWorkspaceService } from "./execution-workspaces.js";
|
|
import { accessService } from "./access.js";
|
|
import { taskWorkspaceRoutes } from "../routes/task-workspaces.js";
|
|
import { errorHandler } from "../middleware/error-handler.js";
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
|
|
describe.skipIf(!support.supported)("remote workspace reservation authority", () => {
|
|
let database: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
let home: string;
|
|
|
|
beforeAll(async () => {
|
|
home = await mkdtemp(join(tmpdir(), "paperclip-remote-reservation-"));
|
|
database = await startEmbeddedPostgresTestDatabase("paperclip-remote-reservation-");
|
|
db = createDb(database.connectionString);
|
|
await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: true });
|
|
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
|
afterEach(() => vi.unstubAllEnvs());
|
|
afterAll(async () => {
|
|
await db?.$client.end();
|
|
await database?.cleanup();
|
|
if (home) await rm(home, { recursive: true, force: true });
|
|
});
|
|
|
|
async function fixture(mode = "isolated_workspace") {
|
|
vi.stubEnv("PAPERCLIP_HOME", home);
|
|
vi.stubEnv("DATABASE_URL", database.connectionString);
|
|
vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY", "62".repeat(32));
|
|
const [company] = await db.insert(companies).values({ name: "Reservation proof", issuePrefix: `R${randomUUID().slice(0, 6)}` }).returning();
|
|
const [agent] = await db.insert(agents).values({ companyId: company.id, name: "Worker", adapterType: "paperclip_runner" }).returning();
|
|
const [project] = await db.insert(projects).values({ companyId: company.id, name: "Source project" }).returning();
|
|
const [source] = await db.insert(projectWorkspaces).values({ companyId: company.id, projectId: project.id,
|
|
name: "Task files", sourceType: "empty" }).returning();
|
|
const [environment] = await db.insert(environments).values({ name: `Reservation ${randomUUID()}`, driver: "sandbox", config: {} }).returning();
|
|
const [parent] = await db.insert(issues).values({ companyId: company.id, projectId: project.id,
|
|
projectWorkspaceId: source.id, title: "Coordinator", status: "done", assigneeAgentId: agent.id }).returning();
|
|
const snapshotId = randomUUID();
|
|
const authority = { schema: "paperclip.remote-workspace.v1" as const, epoch: 3, snapshotId,
|
|
source: { kind: "empty" as const }, location: null, writerRunId: null, dirty: false };
|
|
const [workspace] = await db.insert(executionWorkspaces).values({ companyId: company.id, sourceIssueId: parent.id,
|
|
projectId: project.id, projectWorkspaceId: source.id, name: "Coordinator files", mode,
|
|
strategyType: "project_primary", providerType: "cloud_sandbox", cwd: `/workspaces/${randomUUID()}`,
|
|
metadata: { remoteWorkspace: authority } }).returning();
|
|
await db.update(issues).set({ executionWorkspaceId: workspace.id, executionWorkspacePreference: "reuse_existing",
|
|
executionWorkspaceSettings: { mode } }).where(eq(issues.id, parent.id));
|
|
const child = await issueService(db).create(company.id, { parentId: parent.id, title: "Continue the saved work", assigneeAgentId: agent.id });
|
|
expect(child.executionWorkspaceId).toBe(workspace.id);
|
|
const [run] = await db.insert(heartbeatRuns).values({ companyId: company.id, agentId: agent.id,
|
|
issueId: child.id, status: "running", runtimeMode: "legacy" }).returning();
|
|
const storage = { ...createStorageService(createLocalDiskStorageProvider(join(home, company.id))), provider: "s3" as const };
|
|
const input = { db, storage, companyId: company.id, issueId: child.id, runId: run.id, agentId: agent.id,
|
|
environmentId: environment.id, projectId: project.id, projectWorkspaceId: source.id,
|
|
existingWorkspaceId: workspace.id, source: authority.source };
|
|
const [boundWorkspace] = await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, workspace.id));
|
|
return { input, parent, child, workspace: boundWorkspace!, authority, run, agent, company };
|
|
}
|
|
const current = async (id: string) => (await db.select().from(executionWorkspaces).where(eq(executionWorkspaces.id, id)))[0]!;
|
|
|
|
it.each(["isolated_workspace", "operator_branch", "shared_workspace"])("reserves an inherited %s using the persisted task binding", async mode => {
|
|
const f = await fixture(mode);
|
|
const reserved = await reserveRemoteWorkspace(f.input);
|
|
expect(reserved.id).toBe(f.workspace.id);
|
|
expect(reserved.sourceIssueId).toBe(f.parent.id);
|
|
expect(readRemoteWorkspaceAuthority(reserved.metadata)).toEqual({ ...f.authority, epoch: 4, writerRunId: f.run.id });
|
|
const replay = await reserveRemoteWorkspace(f.input);
|
|
expect(readRemoteWorkspaceAuthority(replay.metadata)).toEqual(readRemoteWorkspaceAuthority(reserved.metadata));
|
|
});
|
|
|
|
it("rejects a workspace that is not bound to the requesting task", async () => {
|
|
const f = await fixture();
|
|
await db.update(issues).set({ executionWorkspaceId: null }).where(eq(issues.id, f.child.id));
|
|
await expect(reserveRemoteWorkspace(f.input)).rejects.toThrow("Task workspace binding is unavailable");
|
|
expect(await current(f.workspace.id)).toEqual(f.workspace);
|
|
});
|
|
|
|
it("uses an explicitly selected root without treating source assignment grants as filesystem permissions", async () => {
|
|
const f = await fixture();
|
|
await db.insert(companyMemberships).values({ companyId: f.company.id, principalType: "agent",
|
|
principalId: f.agent.id, status: "active", membershipRole: "member" });
|
|
await db.update(projects).set({ executionWorkspacePolicy: {
|
|
authorizationPolicy: { assignmentPolicy: { mode: "protected" } },
|
|
} }).where(eq(projects.id, f.workspace.projectId!));
|
|
await db.update(issues).set({ parentId: null, projectId: null, projectWorkspaceId: null,
|
|
executionWorkspaceId: null, workspaceBindingRevision: 0, status: "in_progress",
|
|
executionRunId: f.run.id, checkoutRunId: f.run.id }).where(eq(issues.id, f.child.id));
|
|
await db.update(heartbeatRuns).set({ contextSnapshot: { issueId: f.child.id } }).where(eq(heartbeatRuns.id, f.run.id));
|
|
const actor = { type: "agent" as const, source: "agent_jwt" as const,
|
|
companyId: f.company.id, agentId: f.agent.id, runId: f.run.id };
|
|
expect(await accessService(db).decide({ actor, action: "project:read", resource: {
|
|
type: "project", companyId: f.company.id, projectId: f.workspace.projectId!,
|
|
} })).toMatchObject({ allowed: true });
|
|
expect(await accessService(db).decide({ actor, action: "project:write_workspace", resource: {
|
|
type: "issue", companyId: f.company.id, projectId: f.workspace.projectId!,
|
|
issueId: f.parent.id, assigneeAgentId: f.agent.id,
|
|
} })).toMatchObject({ allowed: false });
|
|
const app = express();
|
|
app.use(express.json());
|
|
app.use((req, _res, next) => { req.actor = actor; next(); });
|
|
app.use("/api", taskWorkspaceRoutes(db));
|
|
app.use(errorHandler);
|
|
const response = await request(app).put(`/api/issues/${f.child.id}/workspace`).send({
|
|
selection: { kind: "existing", workspaceId: f.workspace.id },
|
|
expectedBindingRevision: 0, requestKey: randomUUID(),
|
|
});
|
|
expect(response.status, JSON.stringify(response.body)).toBe(200);
|
|
await db.update(heartbeatRuns).set({ status: "succeeded", finishedAt: new Date() }).where(eq(heartbeatRuns.id, f.run.id));
|
|
const [nextRun] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id,
|
|
issueId: f.child.id, status: "running", runtimeMode: "legacy", contextSnapshot: { issueId: f.child.id } }).returning();
|
|
const nextActor = { ...actor, runId: nextRun.id };
|
|
expect(await executionWorkspaceService(db).applyPendingTaskWorkspaceSelection({ companyId: f.company.id,
|
|
issueId: f.child.id, runId: nextRun.id, actor: nextActor })).toBe(true);
|
|
await expect(reserveRemoteWorkspace({ ...f.input, runId: nextRun.id })).resolves.toMatchObject({
|
|
id: f.workspace.id, sourceIssueId: f.parent.id,
|
|
});
|
|
});
|
|
|
|
it("keeps the original task's privacy boundary on an inherited workspace", async () => {
|
|
const f = await fixture();
|
|
await db.update(issues).set({ visibility: "private", privacyRootIssueId: f.parent.id,
|
|
assigneeAgentId: null }).where(eq(issues.id, f.parent.id));
|
|
await expect(reserveRemoteWorkspace(f.input)).rejects.toMatchObject({ status: 403 });
|
|
expect(await current(f.workspace.id)).toEqual(f.workspace);
|
|
});
|
|
|
|
it("keeps a live writer fenced until it releases the inherited workspace", async () => {
|
|
const f = await fixture();
|
|
const [writer] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id,
|
|
issueId: f.parent.id, status: "running" }).returning();
|
|
const held = { ...f.authority, writerRunId: writer.id, dirty: true };
|
|
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: held } }).where(eq(executionWorkspaces.id, f.workspace.id));
|
|
await expect(reserveRemoteWorkspace(f.input)).rejects.toThrow("unfinished writer");
|
|
expect(readRemoteWorkspaceAuthority((await current(f.workspace.id)).metadata)).toEqual(held);
|
|
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: f.authority } }).where(eq(executionWorkspaces.id, f.workspace.id));
|
|
await expect(reserveRemoteWorkspace(f.input)).resolves.toMatchObject({ id: f.workspace.id });
|
|
});
|
|
|
|
it.each(["verified-stop", "no-stop-receipt", "foreign-receipt"] as const)
|
|
("hands over from an ended writer whose recovered sandbox awaits destroy (%s)", async kind => {
|
|
const f = await fixture();
|
|
const [writer] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id,
|
|
issueId: f.parent.id, status: "cancelled", finishedAt: new Date() }).returning();
|
|
const providerLeaseId = randomUUID();
|
|
const [lease] = await db.insert(environmentLeases).values({ companyId: f.company.id, environmentId: f.input.environmentId,
|
|
heartbeatRunId: writer.id, issueId: f.parent.id, provider: "daytona", providerLeaseId,
|
|
status: "pending_cleanup", cleanupStatus: "failed", leasePolicy: "ephemeral", releasedAt: new Date() }).returning();
|
|
const receipt = remoteTerminationReceipt(lease, { providerLeaseId, state: "stopped" })!;
|
|
const stopReceipt = kind === "verified-stop" ? receipt : kind === "foreign-receipt" ? { ...receipt, runId: randomUUID() } : undefined;
|
|
await db.update(environmentLeases).set({ metadata: { workspaceRecoveryCleanup: { runId: writer.id, stopReceipt } } })
|
|
.where(eq(environmentLeases.id, lease.id));
|
|
// Recovery saved the files, so the writer is clean but not yet destroyed.
|
|
const ended = { ...f.authority, writerRunId: writer.id, dirty: false };
|
|
await db.update(executionWorkspaces).set({ metadata: { remoteWorkspace: ended } }).where(eq(executionWorkspaces.id, f.workspace.id));
|
|
if (kind === "verified-stop") {
|
|
await expect(reserveRemoteWorkspace(f.input)).resolves.toMatchObject({ id: f.workspace.id });
|
|
expect(readRemoteWorkspaceAuthority((await current(f.workspace.id)).metadata)).toMatchObject({ writerRunId: f.run.id, epoch: 4 });
|
|
} else {
|
|
await expect(reserveRemoteWorkspace(f.input)).rejects.toThrow();
|
|
expect(readRemoteWorkspaceAuthority((await current(f.workspace.id)).metadata)).toEqual(ended);
|
|
}
|
|
});
|
|
|
|
it("grants only one writer when two bound children reserve concurrently", async () => {
|
|
const f = await fixture();
|
|
const sibling = await issueService(db).create(f.company.id, { parentId: f.parent.id,
|
|
title: "Another child", assigneeAgentId: f.agent.id });
|
|
const [run] = await db.insert(heartbeatRuns).values({ companyId: f.company.id, agentId: f.agent.id,
|
|
issueId: sibling.id, status: "running" }).returning();
|
|
const inputs = [f.input, { ...f.input, issueId: sibling.id, runId: run.id }];
|
|
const results = await Promise.allSettled(inputs.map(reserveRemoteWorkspace));
|
|
expect(results.filter(result => result.status === "fulfilled")).toHaveLength(1);
|
|
const rejected = results.find(result => result.status === "rejected") as PromiseRejectedResult;
|
|
expect(rejected.reason).toMatchObject({ details: { code: "remote_workspace_writer_pending" } });
|
|
const winner = inputs[results.findIndex(result => result.status === "fulfilled")];
|
|
expect(readRemoteWorkspaceAuthority((await current(f.workspace.id)).metadata)).toEqual({
|
|
...f.authority, epoch: 4, writerRunId: winner.runId,
|
|
});
|
|
});
|
|
|
|
it.each(["archived", "cleanup_failed"])("keeps a %s workspace closed", async status => {
|
|
const f = await fixture();
|
|
await db.update(executionWorkspaces).set({ status }).where(eq(executionWorkspaces.id, f.workspace.id));
|
|
await expect(reserveRemoteWorkspace(f.input)).rejects.toThrow("Workspace ownership changed.");
|
|
expect(readRemoteWorkspaceAuthority((await current(f.workspace.id)).metadata)).toEqual(f.authority);
|
|
});
|
|
|
|
it("rejects a foreign company workspace without changing either binding", async () => {
|
|
const f = await fixture(), other = await fixture();
|
|
await expect(reserveRemoteWorkspace({ ...f.input, existingWorkspaceId: other.workspace.id })).rejects.toThrow("remote_workspace_missing");
|
|
expect(await current(f.workspace.id)).toEqual(f.workspace);
|
|
expect(await current(other.workspace.id)).toEqual(other.workspace);
|
|
});
|
|
});
|