mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 07:55:47 +01:00
## Thinking Path > - Paperclip manages agent execution and the resources that execution owns. > - A retained environment task can need a replacement before the next turn. > - Core records the creation attempt before it asks the provider to acquire it. > - A lost reply can leave that journal without the resource ID needed for cleanup. > - This change lets an opted-in provider recover the identity from that recorded attempt. > - Core still requires the existing exact-task termination proof before it releases ownership. ## Linked Issues or Issue Description Refs #18405 and #18658. This is the separate lost-reply follow-up from their recovery audit. **Draft status:** Both prior review findings are fixed in `a66520c8d4e06e0eec6ccb5446190ace0ca3003f`. Fresh standard Greptile is 5/5 with no unresolved threads. Current-head CI is green (53 successful checks/statuses including Snyk, three expected skips). Apex completed 5/5 on this exact head at 2026-11-03T00:54:59Z (request 6592407130), with no unresolved threads. The review cycle is complete; the PR remains draft for human merge review. **What happened?** If a replacement task's acquire reply is never received, the creation journal can reach manual reconciliation without a resource ID. Startup cleanup cannot complete, and ordinary Stop cannot release the run owner. **Expected behavior** When the selected provider supports creation inspection, Core should retain enough binding information to inspect the recorded attempt and continue verified cleanup. An inspected identity must not count as termination proof. **Steps to reproduce** 1. Use a retained task whose provider supports creation inspection. 2. Lose the replacement acquire reply before Core records its resource ID. 3. Restart cleanup and inspect the existing creation journal. 4. Confirm that the exact task is stopped and its termination receipt is saved before cancellation releases the owner. **Paperclip version or commit** Current head `a66520c8d4e06e0eec6ccb5446190ace0ca3003f`, based on master `3a7bb4dac62dbd7a34a55deb665c55df310bdf64`. Tested with isolated PostgreSQL fixtures. **Deployment mode** Local source tests with controlled provider responses. No deployment or live model request. ## What Changed - Save the selected inspection capability and prior opaque binding in the existing creation journal. Acquire through that same worker so a worker replacement cannot change the recorded contract. - Make failed opted-in creations due through the existing cleanup notification and saved retry deadline. - Add optional SDK observation field `creationLease`. Use it only to recover the requested creation's identity and binding, then follow the existing Stop, status, completion and receipt checks. - Keep old entries and providers without the recorded opt-in under their existing reconciliation policy. - Add actual PostgreSQL recovery, cancellation and worker-replacement regressions. Document the provider contract. ## Verification - Current correction: 53 distinct targeted cases pass (20 real-worker IPC, 12 task-recovery PostgreSQL and 21 generic-sandbox cases), with server typechecking. The IPC selection was rerun after the final late-observation ordering refinement; the other tested sources were unchanged. - An assembled live-child IPC and PostgreSQL case proves cleanup and cancellation release the owner despite the original worker never replying and remaining alive. Accepted host work, unrelated acquisitions and late observations remain accounted for. Removing the recovery handoff and restoring the old drain snapshot produce distinct assertion failures. - The unsupported-host control skips all 12 PostgreSQL cases without setup; removing the guard fails the controlled setup. These tests do not constitute live VM/provider qualification. - Initial head `5db6882` passed an exact reference-tree comparison against master plus the reviewed follow-up patch, 97 targeted cases and server typechecking. Those checks predate the final inspection correction. - Previous head `59f87fa` passed 32 targeted cases (11 task-recovery and 21 generic-sandbox cases) and server typechecking. Both `pending` and `needs_reconciliation` observations without `creationLease` preserve ownership and get a future retry deadline. Restoring the previous guard makes both new cases fail. - The reproduced failure was a wrong generic cleanup shape that could resolve its journal while leaving the owner held—not a demonstrated false ownership release. The correction reuses the existing missing-identity guard before the generic decoder and leaves generic sandbox behavior unchanged. - Earlier affected selection: 72 cases passed on the original follow-up base. This overlaps the final selection and is not an additional count of unique final-head tests. - Two isolated negative controls restore the missing task inspection and redirected-worker behaviors. Both fail at the expected recovery and dispatch assertions without changing production source files. - The PostgreSQL cases verify one cleanup lease, both late-result orders, recovery after the old journal deadline, ordinary legacy behavior, retained ownership on missing or mismatched identity, and cancellation acknowledgement after exact termination proof. - The tests use actual database and run-control services with controlled worker callbacks. They do not prove live IPC or production recovery timing. - A compatible provider is needed for the new recovery path. No normal successful-startup database or network call is added. No ordinary TTFT improvement is claimed; the repair removes an indefinite ownership hold for supported lost replies. No full local CI suite was run; GitHub runs the full regression gates. ## Risks The provider must recover the identity for the recorded creation operation and preserve its selected binding. `creationLease` does not authorize a new turn or prove that a process stopped. Older journal entries do not infer a new identity after an upgrade. Unavailable original machine bindings or credentials retain the existing uncertainty behavior; this change does not repair machine or credential loss. No schema migration, deployment or ROM qualification is included. ## Model Used OpenAI Codex, GPT-6 family. Exact model ID and context window are not exposed in this session. Tool-assisted implementation, targeted tests and independent subagent review. ## 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>
231 lines
15 KiB
TypeScript
231 lines
15 KiB
TypeScript
import { collectSecretRefPaths, isUuidSecretRef, parseSecretRefBindingObject, readConfigValueAtPath } from "./json-schema-secret-refs.js";
|
|
import { createHash, randomUUID } from "node:crypto";
|
|
import { and, eq, lte, or, sql } from "drizzle-orm";
|
|
import { companies, environments, sandboxCreationOperations, type Db } from "@paperclipai/db";
|
|
import { readEnvironmentCreationCleanupError, type PluginEnvironmentLease, type PluginEnvironmentLeaseObservation } from "@paperclipai/plugin-sdk";
|
|
import { assertEnvironmentCompanyBinding, environmentService } from "./environments.js";
|
|
import { ENVIRONMENT_CLEANUP_RETRY_MS, recoverableSandboxCreationCondition } from "./environment-cleanup-state.js";
|
|
import { logger } from "../middleware/logger.js";
|
|
import { notifyEnvironmentCleanup, writeEnvironmentCleanup } from "./environment-cleanup-notifications.js";
|
|
|
|
export type SandboxCreationOperation = typeof sandboxCreationOperations.$inferSelect;
|
|
|
|
export function recoverableTaskCreation(operation: Pick<SandboxCreationOperation, "metadata">): boolean {
|
|
const metadata = operation.metadata;
|
|
return metadata.runnerTaskLease === true && metadata.creationOperationVersion === 1 &&
|
|
typeof metadata.runnerTaskCreationContextId === "string" && metadata.runnerTaskCreationContextId.length > 0 &&
|
|
metadata.providerMetadata !== null && typeof metadata.providerMetadata === "object" && !Array.isArray(metadata.providerMetadata);
|
|
}
|
|
|
|
type CreationOwner = {
|
|
companyId: string;
|
|
environmentId: string;
|
|
executionWorkspaceId?: string | null;
|
|
issueId?: string | null;
|
|
heartbeatRunId?: string | null;
|
|
provider: string;
|
|
pluginId?: string;
|
|
driverKey: string;
|
|
config: Record<string, unknown>;
|
|
metadata: Record<string, unknown>;
|
|
timeoutMs?: number;
|
|
requestKey?: string;
|
|
configSchema?: Record<string, unknown> | null;
|
|
resolvedConfig?: Record<string, unknown>;
|
|
};
|
|
|
|
export async function createOwnedSandbox(
|
|
db: Db, owner: CreationOwner,
|
|
create: (identity: { id: string; runId: string }) => Promise<PluginEnvironmentLease>,
|
|
): Promise<PluginEnvironmentLease> {
|
|
const credentialHashes: Record<string, string> = {};
|
|
for (const path of collectSecretRefPaths(owner.configSchema)) {
|
|
const value = readConfigValueAtPath(owner.config, path);
|
|
if (value != null && value !== "" && !(typeof value === "string" && isUuidSecretRef(value)) && !parseSecretRefBindingObject(value)) {
|
|
throw new Error("Sandbox creation requires saved secret references");
|
|
}
|
|
if (value != null && value !== "") {
|
|
const resolved = readConfigValueAtPath(owner.resolvedConfig ?? {}, path);
|
|
if (typeof resolved !== "string") throw new Error("Sandbox creation credential identity is unavailable");
|
|
credentialHashes[path] = createHash("sha256").update(resolved).digest("hex");
|
|
}
|
|
}
|
|
const id = randomUUID();
|
|
const runId = owner.heartbeatRunId ?? randomUUID();
|
|
const operation = await db.transaction(async tx => {
|
|
await notifyEnvironmentCleanup(tx, { companyId: owner.companyId, runId: owner.heartbeatRunId ?? undefined });
|
|
await assertEnvironmentCompanyBinding(tx as unknown as Db, owner.environmentId, owner.companyId);
|
|
const [inserted] = await tx.insert(sandboxCreationOperations).values({
|
|
id, companyId: owner.companyId, environmentId: owner.environmentId, originalEnvironmentId: owner.environmentId,
|
|
runId, requestKey: owner.requestKey ?? "run", issueId: owner.issueId, executionWorkspaceId: owner.executionWorkspaceId,
|
|
provider: owner.provider, pluginId: owner.pluginId, driverKey: owner.driverKey,
|
|
config: owner.config, metadata: { ...owner.metadata, creationCredentialHashes: credentialHashes },
|
|
nextAttemptAt: new Date(Date.now() + Math.max(30 * 60_000, (owner.timeoutMs ?? 0) + 60_000)),
|
|
}).onConflictDoNothing().returning();
|
|
if (inserted) return inserted;
|
|
const [existing] = await tx.select().from(sandboxCreationOperations).where(and(
|
|
eq(sandboxCreationOperations.companyId, owner.companyId), eq(sandboxCreationOperations.originalEnvironmentId, owner.environmentId),
|
|
eq(sandboxCreationOperations.runId, runId), eq(sandboxCreationOperations.requestKey, owner.requestKey ?? "run"),
|
|
));
|
|
if (!existing) throw new Error("Sandbox creation ownership was not recorded");
|
|
return existing;
|
|
});
|
|
if (operation.id !== id) {
|
|
if (operation.state !== "attached" || !operation.leaseId) throw new Error("Sandbox creation requires reconciliation before another attempt");
|
|
const existing = await environmentService(db).getLeaseById(operation.leaseId);
|
|
if (!existing || existing.status !== "active" || !existing.providerLeaseId) throw new Error("Sandbox creation has already transferred ownership");
|
|
return { providerLeaseId: existing.providerLeaseId, expiresAt: existing.expiresAt?.toISOString(),
|
|
metadata: { ...existing.metadata, creationOperationId: operation.id } };
|
|
}
|
|
try {
|
|
const lease = await create({ id, runId });
|
|
return { ...lease, metadata: { ...lease.metadata, creationOperationId: id } };
|
|
} catch (error) {
|
|
if (recoverableTaskCreation(operation)) {
|
|
try {
|
|
await writeEnvironmentCleanup(db, tx => tx.update(sandboxCreationOperations).set({ nextAttemptAt: new Date(), updatedAt: new Date() })
|
|
.where(and(eq(sandboxCreationOperations.id, id), recoverableSandboxCreationCondition())),
|
|
{ companyId: operation.companyId, runId: operation.runId });
|
|
} catch {
|
|
logger.warn({ errorKind: "task_creation_recovery_wakeup_failed", operationId: id, companyId: operation.companyId },
|
|
"Task creation recovery retains its saved deadline");
|
|
}
|
|
}
|
|
throw error;
|
|
}
|
|
}
|
|
|
|
/** Keep an already returned resource identity even when task attachment fails.
|
|
* This enriches the existing owner; it never transfers or reopens ownership. */
|
|
export async function recordSandboxCreationResource(db: Db, input: {
|
|
id: string; companyId: string; environmentId: string; providerLeaseId: string;
|
|
providerMetadata?: Record<string, unknown>;
|
|
}): Promise<void> {
|
|
await writeEnvironmentCleanup(db, async tx => {
|
|
const [recorded] = await tx.update(sandboxCreationOperations).set({
|
|
nextAttemptAt: sql`case when ${sandboxCreationOperations.state} = 'needs_reconciliation' then clock_timestamp() else ${sandboxCreationOperations.nextAttemptAt} end`,
|
|
metadata: sql`coalesce(${sandboxCreationOperations.metadata}, '{}'::jsonb) || ${JSON.stringify({ creationProviderLeaseId: input.providerLeaseId, ...(input.providerMetadata ? { creationProviderMetadata: input.providerMetadata } : {}) })}::jsonb`,
|
|
updatedAt: new Date(),
|
|
}).where(and(eq(sandboxCreationOperations.id, input.id), eq(sandboxCreationOperations.companyId, input.companyId),
|
|
eq(sandboxCreationOperations.originalEnvironmentId, input.environmentId),
|
|
or(sql`${sandboxCreationOperations.metadata}->>'creationProviderLeaseId' is null`,
|
|
sql`${sandboxCreationOperations.metadata}->>'creationProviderLeaseId' = ${input.providerLeaseId}`),
|
|
)).returning({ id: sandboxCreationOperations.id });
|
|
if (!recorded) throw new Error("Sandbox creation resource identity changed");
|
|
}, { companyId: input.companyId });
|
|
}
|
|
|
|
export async function queueSandboxCreationCleanup(db: Db, input: {
|
|
id: string; companyId: string; environmentId: string; providerLeaseId: string;
|
|
providerMetadata?: Record<string, unknown>;
|
|
}): Promise<void> {
|
|
await db.transaction(async tx => {
|
|
await tx.select({ id: environments.id }).from(environments).where(eq(environments.id, input.environmentId)).for("update");
|
|
const [operation] = await tx.select().from(sandboxCreationOperations).where(and(eq(sandboxCreationOperations.id, input.id),
|
|
eq(sandboxCreationOperations.companyId, input.companyId), eq(sandboxCreationOperations.originalEnvironmentId, input.environmentId))).for("update");
|
|
if (!operation || (operation.metadata.creationProviderLeaseId && operation.metadata.creationProviderLeaseId !== input.providerLeaseId)) {
|
|
throw new Error("Sandbox creation cleanup ownership changed");
|
|
}
|
|
if (["attached", "cleanup_pending", "resolved"].includes(operation.state)) return;
|
|
const contextId = operation.metadata.runnerTaskLease === true && typeof operation.metadata.runnerTaskCreationContextId === "string"
|
|
? operation.metadata.runnerTaskCreationContextId : null;
|
|
if (operation.state !== "pending" && !(operation.state === "needs_reconciliation" && contextId &&
|
|
operation.metadata.creationProviderLeaseId === input.providerLeaseId)) throw new Error("Sandbox creation cleanup ownership changed");
|
|
await notifyEnvironmentCleanup(tx, { companyId: operation.companyId, runId: operation.runId });
|
|
const lease = await environmentService(tx as unknown as Db).insertPendingCleanupLease({
|
|
companyId: operation.companyId, environmentId: operation.originalEnvironmentId,
|
|
heartbeatRunId: operation.runId, issueId: operation.issueId, executionWorkspaceId: operation.executionWorkspaceId,
|
|
provider: operation.provider, providerLeaseId: input.providerLeaseId,
|
|
metadata: { ...operation.metadata, creationOperationId: operation.id, environmentId: operation.originalEnvironmentId,
|
|
providerMetadata: input.providerMetadata ?? operation.metadata.creationProviderMetadata ?? {}, runnerTaskRetiring: true,
|
|
...(contextId ? { runnerTaskStartupCleanup: { contextId, requiresPreparationStop: false } } : {}) },
|
|
failureReason: "interrupted_task_creation",
|
|
});
|
|
await tx.update(sandboxCreationOperations).set({ state: "cleanup_pending", leaseId: lease.id, updatedAt: new Date(),
|
|
metadata: { ...operation.metadata, creationProviderLeaseId: input.providerLeaseId,
|
|
creationProviderMetadata: input.providerMetadata ?? operation.metadata.creationProviderMetadata ?? {} },
|
|
}).where(eq(sandboxCreationOperations.id, operation.id));
|
|
});
|
|
}
|
|
|
|
export async function recoverSandboxCreations(db: Db,
|
|
inspect: (operation: SandboxCreationOperation) => Promise<PluginEnvironmentLeaseObservation | null>,
|
|
onTaskRecovered?: (operation: SandboxCreationOperation) => void | Promise<void>,
|
|
): Promise<void> {
|
|
const due = await db.select().from(sandboxCreationOperations).where(and(
|
|
recoverableSandboxCreationCondition(), lte(sandboxCreationOperations.nextAttemptAt, new Date()),
|
|
)).limit(100);
|
|
for (const candidate of due) {
|
|
try {
|
|
if (candidate.metadata.runnerTaskLease === true && typeof candidate.metadata.runnerTaskCreationContextId === "string" &&
|
|
typeof candidate.metadata.creationProviderLeaseId === "string") {
|
|
await queueSandboxCreationCleanup(db, { id: candidate.id, companyId: candidate.companyId,
|
|
environmentId: candidate.originalEnvironmentId, providerLeaseId: candidate.metadata.creationProviderLeaseId });
|
|
if (recoverableTaskCreation(candidate)) await onTaskRecovered?.(candidate);
|
|
continue;
|
|
}
|
|
const [existingCompany] = await db.select({ id: companies.id }).from(companies).where(eq(companies.id, candidate.companyId)).limit(1);
|
|
const observation = existingCompany ? await inspect(candidate) : null;
|
|
if (existingCompany && recoverableTaskCreation(candidate) && observation?.creationLease) {
|
|
const lease = observation.creationLease;
|
|
if (typeof lease.providerLeaseId !== "string" || lease.providerLeaseId.length === 0 || lease.providerLeaseId !== observation.providerLeaseId ||
|
|
!lease.metadata || typeof lease.metadata !== "object" || Array.isArray(lease.metadata)) {
|
|
throw new Error("Task creation recovery returned foreign ownership");
|
|
}
|
|
const identity = { id: candidate.id, companyId: candidate.companyId, environmentId: candidate.originalEnvironmentId,
|
|
providerLeaseId: lease.providerLeaseId, providerMetadata: lease.metadata };
|
|
await recordSandboxCreationResource(db, identity);
|
|
await queueSandboxCreationCleanup(db, identity);
|
|
await onTaskRecovered?.(candidate);
|
|
continue;
|
|
}
|
|
if (existingCompany && recoverableTaskCreation(candidate)) throw new Error("Task creation identity is not available yet");
|
|
const cleanup = observation?.creationCleanup ? readEnvironmentCreationCleanupError({ data: {
|
|
schema: "paperclip/environment-creation-cleanup/v1", cleanup: observation.creationCleanup,
|
|
} }) : null;
|
|
if (cleanup && (cleanup.companyId !== candidate.companyId || cleanup.environmentId !== candidate.originalEnvironmentId ||
|
|
cleanup.runId !== candidate.runId || cleanup.attemptId !== candidate.id || cleanup.providerLeaseId !== observation!.providerLeaseId)) {
|
|
throw new Error("Sandbox creation recovery returned foreign ownership");
|
|
}
|
|
await db.transaction(async tx => {
|
|
const [company] = await tx.select({ id: companies.id }).from(companies).where(eq(companies.id, candidate.companyId)).for("key share");
|
|
await tx.select({ id: environments.id }).from(environments).where(eq(environments.id, candidate.originalEnvironmentId)).for("update");
|
|
const [operation] = await tx.select().from(sandboxCreationOperations)
|
|
.where(eq(sandboxCreationOperations.id, candidate.id)).for("update");
|
|
if (!operation || operation.state !== "pending") return;
|
|
await notifyEnvironmentCleanup(tx, { companyId: operation.companyId });
|
|
if (!company || !cleanup) {
|
|
logger.warn({ errorKind: "sandbox_creation_needs_reconciliation", operationId: operation.id, companyId: operation.companyId },
|
|
"Sandbox creation could not be identified; operator reconciliation is required");
|
|
await tx.update(sandboxCreationOperations).set({ state: "needs_reconciliation", updatedAt: new Date(),
|
|
metadata: { ...operation.metadata, creationReconciliationReason: company ? "creation_identity_unavailable" : "company_deleted" },
|
|
})
|
|
.where(eq(sandboxCreationOperations.id, operation.id));
|
|
return;
|
|
}
|
|
const lease = await environmentService(tx as unknown as Db).insertPendingCleanupLease({
|
|
companyId: operation.companyId, environmentId: operation.originalEnvironmentId,
|
|
provider: operation.provider, providerLeaseId: cleanup.providerLeaseId,
|
|
metadata: { ...operation.metadata, creationOperationId: operation.id, failedCreateCleanup: cleanup },
|
|
failureReason: "interrupted_sandbox_creation",
|
|
});
|
|
await tx.update(sandboxCreationOperations).set({ state: "cleanup_pending", leaseId: lease.id, updatedAt: new Date() })
|
|
.where(eq(sandboxCreationOperations.id, operation.id));
|
|
});
|
|
} catch {
|
|
const retry = await db.transaction(async tx => {
|
|
const [company] = await tx.select({ id: companies.id }).from(companies).where(eq(companies.id, candidate.companyId)).for("key share");
|
|
await notifyEnvironmentCleanup(tx, { companyId: candidate.companyId });
|
|
await tx.update(sandboxCreationOperations).set(company
|
|
? { nextAttemptAt: new Date(Date.now() + ENVIRONMENT_CLEANUP_RETRY_MS), updatedAt: new Date() }
|
|
: { state: "needs_reconciliation", updatedAt: new Date(),
|
|
metadata: sql`coalesce(${sandboxCreationOperations.metadata}, '{}'::jsonb) || '{"creationReconciliationReason":"company_deleted"}'::jsonb` })
|
|
.where(and(eq(sandboxCreationOperations.id, candidate.id), recoverableSandboxCreationCondition()));
|
|
return Boolean(company);
|
|
});
|
|
logger.warn({ errorKind: retry ? "sandbox_creation_recovery_failed" : "sandbox_creation_needs_reconciliation", operationId: candidate.id, companyId: candidate.companyId },
|
|
retry ? "Sandbox creation recovery will retry at its saved deadline" : "Sandbox creation company was deleted; operator reconciliation is required");
|
|
}
|
|
}
|
|
}
|