mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 01:46:15 +02:00
fix: renew explicit continuation authorization for bounded retries (#14987)
Preserve exact user-authorized continuation receipts across bounded retries and retain the task claim through owned retry admission. Stop, reassignment, superseding input, and active cleanup continue to block unsafe continuation. Validation: exact-head Greptile 5/5, passing CI, no unresolved review threads, and a clean merge. Detailed verification and limitations are recorded in the pull request. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
7498705642
commit
2e67ea8dc7
11 files changed
+639
-16
No files matched your search
@@ -1,10 +1,12 @@
|
||||
import * as executionContinuation from "../services/execution-continuation.js";
|
||||
import * as environmentOrchestrator from "../services/environment-run-orchestrator.js";
|
||||
import { remoteTerminationReceipt } from "../services/remote-execution-termination.js";
|
||||
import { legacyDispositionFingerprint, LEGACY_DISPOSITION_REPAIR_INSTRUCTION } from "../services/recovery/legacy-continuation.js";
|
||||
import * as controllerLeases from "../services/legacy-controller-lease.js";
|
||||
import * as instructionWorkingCopies from "../services/agent-instruction-working-copies.js";
|
||||
import * as runEvents from "../services/heartbeat-run-events.js";
|
||||
import { instanceSettingsService } from "../services/instance-settings.js";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { createHash, randomUUID } from "node:crypto";
|
||||
import { terminalizeLegacyExecution } from "../services/legacy-execution-recovery.js";
|
||||
import { issueService } from "../services/issues.js";
|
||||
import { getExecutionBlocker } from "../services/execution-blocker.js";
|
||||
@@ -1489,6 +1491,207 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => {
|
||||
return { companyId, agentId, runId, wakeupRequestId, issueId };
|
||||
}
|
||||
|
||||
it.each(["timeout", "upstream", "cleanup_pending", "cleanup_pending_edited", "cleanup_pending_exhausted", "edited", "exhausted", "new_message", "no_claim", "reassigned", "superseded", "stopped"])(
|
||||
"settles explicit retry admission through real executor cleanup: %s", async scenario => {
|
||||
const { companyId, agentId, runId, issueId } = await seedQueuedIssueRunFixture();
|
||||
const sourceRunId = randomUUID(), commentId = randomUUID(), nextCommentId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: sourceRunId, companyId, agentId, status: "failed",
|
||||
createdAt: new Date("2020-01-01T00:00:00Z"), finishedAt: new Date("2020-01-01T00:00:01Z"),
|
||||
contextSnapshot: { issueId } });
|
||||
const [comment] = await db.insert(issueComments).values({ id: commentId, companyId, issueId,
|
||||
authorType: "user", authorUserId: "responsible-user", body: "Continue the task." }).returning();
|
||||
await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId,
|
||||
kind: "active_run_watchdog", cause: "legacy_execution_requires_reconciliation", fingerprint: sourceRunId,
|
||||
status: "resolved", outcome: "cancelled", nextAction: "User continued.", evidence: { explicitUserContinuation: {
|
||||
runId, previousRunId: sourceRunId, commentId, actorId: "responsible-user", recordedAt: new Date().toISOString(),
|
||||
commentUpdatedAt: comment!.updatedAt.toISOString(), commentBodyHash: createHash("sha256").update(comment!.body).digest("hex"),
|
||||
} } });
|
||||
await db.update(heartbeatRuns).set({ createdAt: new Date(), contextSnapshot: {
|
||||
issueId, taskId: issueId, wakeReason: "issue_commented", wakeCommentId: commentId,
|
||||
previousRunId: sourceRunId, explicitUserContinuation: { previousRunId: sourceRunId, commentId },
|
||||
} }).where(eq(heartbeatRuns.id, runId));
|
||||
const identity = { id: randomUUID(), companyId, heartbeatRunId: runId, provider: "daytona", providerLeaseId: "fixture-explicit-sandbox" };
|
||||
const supersedingRunId = randomUUID();
|
||||
let heartbeat: ReturnType<typeof heartbeatService>;
|
||||
let cancellation: ReturnType<typeof heartbeat.cancelRun> | undefined;
|
||||
let cleanupObserved = false;
|
||||
let cleanupPending = scenario.startsWith("cleanup_pending");
|
||||
const originalFactory = environmentOrchestrator.environmentRunOrchestrator;
|
||||
const release = vi.fn(async () => {
|
||||
const [lease] = await db.select().from(environmentLeases).where(eq(environmentLeases.id, identity.id));
|
||||
if (!lease) return { released: [], errors: [] };
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, runId))).toHaveLength(0);
|
||||
if (!cleanupObserved) {
|
||||
cleanupObserved = true;
|
||||
const denied = await db.select().from(heartbeatRunEvents).where(and(eq(heartbeatRunEvents.runId, runId),
|
||||
eq(heartbeatRunEvents.message, "The automatic retry could not revalidate the original user continuation.")));
|
||||
// Upstream failure first tries the direct scheduler, then the release
|
||||
// tail. Both must defer admission until the executor releases ownership.
|
||||
if (scenario === "upstream") expect(denied.length).toBeGreaterThanOrEqual(2);
|
||||
if (scenario === "timeout") expect(denied.length).toBeGreaterThanOrEqual(1);
|
||||
if (scenario === "new_message") {
|
||||
await db.insert(issueComments).values({ id: nextCommentId, companyId, issueId, authorType: "user",
|
||||
authorUserId: "responsible-user", body: "Use these new instructions." });
|
||||
await heartbeat.wakeup(agentId, { source: "automation", reason: "issue_commented",
|
||||
requestedByActorType: "user", requestedByActorId: "responsible-user",
|
||||
payload: { issueId, commentId: nextCommentId },
|
||||
contextSnapshot: { issueId, commentId: nextCommentId, wakeReason: "issue_commented" } });
|
||||
}
|
||||
if (scenario === "reassigned") {
|
||||
const [other] = await db.insert(agents).values({ companyId, name: "New assignee", role: "engineer",
|
||||
adapterType: "codex_local" }).returning();
|
||||
await db.update(issues).set({ assigneeAgentId: other!.id }).where(eq(issues.id, issueId));
|
||||
}
|
||||
if (scenario === "superseded") {
|
||||
await db.insert(heartbeatRuns).values({ id: supersedingRunId, companyId, agentId, status: "running",
|
||||
contextSnapshot: { issueId } });
|
||||
await db.update(issues).set({ executionRunId: supersedingRunId }).where(eq(issues.id, issueId));
|
||||
}
|
||||
}
|
||||
await db.update(environmentLeases).set({ status: "released", cleanupStatus: "success", releasedAt: new Date() })
|
||||
.where(and(eq(environmentLeases.companyId, companyId), eq(environmentLeases.heartbeatRunId, runId),
|
||||
eq(environmentLeases.provider, "local")));
|
||||
await db.update(environmentLeases).set(cleanupPending
|
||||
? { status: "pending_cleanup", cleanupStatus: "failed", releasedAt: new Date() }
|
||||
: { status: "released", cleanupStatus: "success", releasedAt: new Date(), metadata: {
|
||||
remoteExecutionTermination: remoteTerminationReceipt(identity, { providerLeaseId: identity.providerLeaseId, state: "destroyed" }),
|
||||
} }).where(eq(environmentLeases.id, identity.id));
|
||||
return { released: [], errors: [] };
|
||||
});
|
||||
const factory = vi.spyOn(environmentOrchestrator, "environmentRunOrchestrator").mockImplementation((...args) => ({
|
||||
...originalFactory(...args), releaseForRun: release,
|
||||
}));
|
||||
heartbeat = heartbeatService(db);
|
||||
mockAdapterExecute.mockImplementationOnce(async (input?: unknown) => {
|
||||
const invocation = input as { onCancellationReady: () => Promise<void>; onDispatch: () => void };
|
||||
await invocation.onCancellationReady();
|
||||
invocation.onDispatch();
|
||||
expect(adapterExecutionControls.has(runId)).toBe(true);
|
||||
await db.insert(environmentLeases).values({ ...identity, status: "active", leasePolicy: "ephemeral" });
|
||||
if (scenario === "edited") await db.update(issueComments).set({ body: "Changed after dispatch" }).where(eq(issueComments.id, commentId));
|
||||
if (scenario === "no_claim") await db.update(issues).set({ executionRunId: null }).where(eq(issues.id, issueId));
|
||||
if (scenario === "exhausted") await db.update(heartbeatRuns).set({ scheduledRetryAttempt: 2,
|
||||
scheduledRetryReason: "transient_failure" }).where(eq(heartbeatRuns.id, runId));
|
||||
if (scenario === "stopped") {
|
||||
cancellation = heartbeat.cancelRun(runId, "Operator Stop");
|
||||
void cancellation.catch(() => undefined);
|
||||
await waitForValue(async () => adapterExecutionControls.get(runId)?.controller.signal.aborted);
|
||||
}
|
||||
return { exitCode: 1, signal: scenario === "stopped" ? "SIGTERM" : null,
|
||||
timedOut: scenario !== "upstream" && scenario !== "stopped",
|
||||
errorMessage: scenario === "upstream" ? "Upstream temporarily unavailable" : "Execution timed out",
|
||||
errorCode: scenario === "upstream" ? "upstream_unavailable" : "timeout",
|
||||
resultJson: scenario === "upstream" ? { errorFamily: "transient_upstream" }
|
||||
: scenario === "stopped" ? { executionCancellation: { state: "acknowledged" } } : {},
|
||||
summary: "Stopped turn", provider: "test", model: "test-model" };
|
||||
});
|
||||
mockAdapterExecute.mockImplementation(async () => {
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, issueId));
|
||||
return { exitCode: 0, signal: null, timedOut: false, errorMessage: null,
|
||||
summary: "New message handled.", provider: "test", model: "test-model" };
|
||||
});
|
||||
try {
|
||||
await heartbeat.resumeQueuedRuns();
|
||||
await heartbeat.drainActiveRunExecutions();
|
||||
if (cancellation) expect((await cancellation)?.status).toBe("cancelled");
|
||||
expect(release).toHaveBeenCalled();
|
||||
expect(adapterExecutionControls.has(runId)).toBe(false);
|
||||
const children = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, runId));
|
||||
const receipts = await db.select().from(issueRecoveryActions).where(and(eq(issueRecoveryActions.sourceIssueId, issueId),
|
||||
eq(issueRecoveryActions.cause, "explicit_user_continuation_retry")));
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, issueId));
|
||||
if (scenario === "timeout" || scenario === "upstream") {
|
||||
expect(children).toHaveLength(1);
|
||||
expect(receipts).toHaveLength(1);
|
||||
expect(task!.executionRunId).toBe(children[0]!.id);
|
||||
await expect(executionContinuation.buildExecutionContinuation({ db, companyId, issueId, agentId,
|
||||
runId: children[0]!.id, context: children[0]!.contextSnapshot!, summary: null, exposeLowTrustRaw: false }))
|
||||
.resolves.toMatchObject({ interruptedRunId: runId });
|
||||
expect(await heartbeat.scheduleBoundedRetry(runId)).toMatchObject({ outcome: "scheduled", run: { id: children[0]!.id } });
|
||||
} else {
|
||||
expect(children).toHaveLength(0);
|
||||
expect(receipts).toHaveLength(0);
|
||||
expect(task!.executionRunId).toBe(cleanupPending ? runId : scenario === "superseded" ? supersedingRunId : null);
|
||||
}
|
||||
if (scenario === "new_message") expect(await db.select().from(heartbeatRuns).where(and(
|
||||
eq(heartbeatRuns.companyId, companyId), sql`${heartbeatRuns.contextSnapshot}->>'wakeCommentId' = ${nextCommentId}`)))
|
||||
.toHaveLength(1);
|
||||
if (cleanupPending) {
|
||||
if (scenario === "cleanup_pending") {
|
||||
const svc = issueService(db);
|
||||
const [other] = await db.insert(agents).values({ companyId, name: "Other worker", role: "engineer",
|
||||
adapterType: "codex_local" }).returning();
|
||||
const actorRunId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: actorRunId, companyId, agentId, status: "running", contextSnapshot: {} });
|
||||
// Neither a rejected checkout nor stale checkout adoption may erase
|
||||
// the exact pending retry claim, including mixed checkout owners.
|
||||
for (const checkoutRunId of [null, runId, sourceRunId]) {
|
||||
await db.update(issues).set({ checkoutRunId }).where(eq(issues.id, issueId));
|
||||
expect(await svc.clearExecutionRunIfTerminal(issueId)).toBe(false);
|
||||
expect(await svc.clearCheckoutRunIfTerminal(issueId)).toBe(false);
|
||||
await expect(svc.checkout(issueId, other!.id, ["in_progress"], actorRunId)).rejects.toMatchObject({ status: 409 });
|
||||
await expect(svc.assertCheckoutOwner(issueId, other!.id, actorRunId)).rejects.toMatchObject({ status: 409 });
|
||||
await expect(svc.checkout(issueId, agentId, ["in_progress"], actorRunId)).rejects.toMatchObject({ status: 409 });
|
||||
await expect(svc.assertCheckoutOwner(issueId, agentId, actorRunId)).rejects.toMatchObject({ status: 409 });
|
||||
expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.executionRunId).toBe(runId);
|
||||
}
|
||||
await db.update(issues).set({ checkoutRunId: null }).where(eq(issues.id, issueId));
|
||||
await db.update(heartbeatRuns).set({ status: "failed", finishedAt: new Date() }).where(eq(heartbeatRuns.id, actorRunId));
|
||||
}
|
||||
await heartbeat.sweepStaleIssueLocks();
|
||||
expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.executionRunId).toBe(runId);
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, runId))).toHaveLength(0);
|
||||
expect(await db.select().from(issueRecoveryActions).where(and(eq(issueRecoveryActions.sourceIssueId, issueId),
|
||||
eq(issueRecoveryActions.cause, "explicit_user_continuation_retry")))).toHaveLength(0);
|
||||
if (scenario === "cleanup_pending_edited") await db.update(issueComments).set({ body: "Changed while cleanup waited" })
|
||||
.where(eq(issueComments.id, commentId));
|
||||
if (scenario === "cleanup_pending_exhausted") await db.update(heartbeatRuns).set({ scheduledRetryAttempt: 2,
|
||||
scheduledRetryReason: "transient_failure" }).where(eq(heartbeatRuns.id, runId));
|
||||
cleanupPending = false;
|
||||
await heartbeat.releaseEnvironmentLeasesForRun({ runId, companyId, agentId, status: "timed_out" });
|
||||
await heartbeat.sweepStaleIssueLocks();
|
||||
await heartbeat.sweepStaleIssueLocks();
|
||||
const afterCleanup = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, runId));
|
||||
expect(afterCleanup).toHaveLength(scenario === "cleanup_pending" ? 1 : 0);
|
||||
expect(await db.select().from(issueRecoveryActions).where(and(eq(issueRecoveryActions.sourceIssueId, issueId),
|
||||
eq(issueRecoveryActions.cause, "explicit_user_continuation_retry")))).toHaveLength(afterCleanup.length);
|
||||
expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.executionRunId).toBe(afterCleanup[0]?.id ?? null);
|
||||
}
|
||||
} finally { factory.mockRestore(); }
|
||||
},
|
||||
);
|
||||
|
||||
it("does not reuse a completed explicit turn's receipt for a missing-comment follow-up", async () => {
|
||||
const { companyId, agentId, runId, issueId } = await seedQueuedIssueRunFixture();
|
||||
const sourceRunId = randomUUID(), commentId = randomUUID();
|
||||
await db.insert(heartbeatRuns).values({ id: sourceRunId, companyId, agentId, status: "failed",
|
||||
finishedAt: new Date(), contextSnapshot: { issueId } });
|
||||
await db.insert(issueComments).values({ id: commentId, companyId, issueId,
|
||||
authorType: "user", authorUserId: "responsible-user", body: "Continue the task." });
|
||||
await db.insert(issueRecoveryActions).values({ companyId, sourceIssueId: issueId,
|
||||
kind: "active_run_watchdog", cause: "legacy_execution_requires_reconciliation", fingerprint: sourceRunId,
|
||||
status: "resolved", outcome: "cancelled", nextAction: "User continued.",
|
||||
evidence: { explicitUserContinuation: { runId, previousRunId: sourceRunId, commentId,
|
||||
actorId: "responsible-user", recordedAt: new Date().toISOString() } } });
|
||||
await db.update(heartbeatRuns).set({ contextSnapshot: {
|
||||
issueId, taskId: issueId, wakeReason: "issue_assigned",
|
||||
explicitUserContinuation: { previousRunId: sourceRunId, commentId },
|
||||
} }).where(eq(heartbeatRuns.id, runId));
|
||||
mockAdapterExecute.mockImplementationOnce(async () => {
|
||||
await db.update(issues).set({ status: "done" }).where(eq(issues.id, issueId));
|
||||
return { exitCode: 0, signal: null, timedOut: false, errorMessage: null,
|
||||
summary: "Task completed.", provider: "test", model: "test-model" };
|
||||
});
|
||||
const heartbeat = heartbeatService(db);
|
||||
await heartbeat.resumeQueuedRuns();
|
||||
await waitForRunToSettle(heartbeat, runId);
|
||||
await heartbeat.waitForRunExecutionDrain(runId);
|
||||
expect(await heartbeat.getRun(runId)).toMatchObject({ status: "succeeded", issueCommentStatus: "not_applicable" });
|
||||
expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.retryOfRunId, runId))).toHaveLength(0);
|
||||
expect(await db.select().from(agentWakeupRequests).where(and(eq(agentWakeupRequests.companyId, companyId),
|
||||
eq(agentWakeupRequests.reason, "missing_issue_comment")))).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("persists the normalized failure without permanently blocking the conversation", async () => {
|
||||
mockAdapterExecute.mockResolvedValueOnce({
|
||||
exitCode: 1,
|
||||
|
||||
@@ -110,6 +110,31 @@ describeEmbeddedPostgres("recovery sweepStaleIssueLocks", () => {
|
||||
return { companyId, agentId, failedRunId, runningRunId };
|
||||
}
|
||||
|
||||
it.each([true, false])("preserves explicit retry ownership when finalization wins the sweep write (settler: %s)", async withSettler => {
|
||||
const { companyId, agentId, runningRunId, failedRunId } = await seed();
|
||||
const issueId = randomUUID();
|
||||
await db.insert(issues).values({ id: issueId, companyId, title: "Retry finalization race",
|
||||
status: "in_progress", assigneeAgentId: agentId, executionRunId: runningRunId });
|
||||
await db.update(heartbeatRuns).set({ processPid: 2_000_000_000,
|
||||
contextSnapshot: { issueId, explicitUserContinuation: { previousRunId: failedRunId, commentId: randomUUID() } },
|
||||
}).where(eq(heartbeatRuns.id, runningRunId));
|
||||
const beforeOrphanedRunTerminalWrite = vi.fn(async () => {
|
||||
await db.update(heartbeatRuns).set({ status: "timed_out", finishedAt: new Date() })
|
||||
.where(eq(heartbeatRuns.id, runningRunId));
|
||||
});
|
||||
const settleExplicitContinuationRetry = vi.fn(async () => {});
|
||||
const result = await recoveryService(db, { enqueueWakeup: vi.fn(), beforeOrphanedRunTerminalWrite,
|
||||
...(withSettler ? { settleExplicitContinuationRetry } : {}),
|
||||
}).sweepStaleIssueLocks();
|
||||
expect(beforeOrphanedRunTerminalWrite).toHaveBeenCalledOnce();
|
||||
expect(result).toEqual({ cleared: 0, issueIds: [], terminalizedRunIds: [] });
|
||||
expect(settleExplicitContinuationRetry).toHaveBeenCalledTimes(withSettler ? 1 : 0);
|
||||
if (withSettler) expect(settleExplicitContinuationRetry).toHaveBeenCalledWith(expect.objectContaining({
|
||||
id: runningRunId, status: "timed_out",
|
||||
}));
|
||||
expect((await db.select().from(issues).where(eq(issues.id, issueId)))[0]!.executionRunId).toBe(runningRunId);
|
||||
});
|
||||
|
||||
it("clears lock columns when checkoutRunId points at a terminal heartbeat run", async () => {
|
||||
const { companyId, agentId, failedRunId } = await seed();
|
||||
const issueId = randomUUID();
|
||||
|
||||
Reference in new issue
Block a user