mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 05:56:19 +01:00
## Thinking Path > - Paperclip lets people manage agents and their saved work. > - Native runs can leave a prepared remote workspace after a control deadline. > - The deadline repair in #17788 records preservation for future failures. > - Older failed runs can still have an active lease without a preservation owner. > - Retry correctly refuses another executor, but nothing can finish the missing preservation. > - This pull request registers the exact prepared source through the existing recovery services when the user retries. > - The bounded maintenance worker can then stop the provider and save its files before normal admission continues. ## Linked Issues or Issue Description **What happened?** A historical failed native run has a prepared workspace and an active retained allocation. It has no workspace preservation record. Ordinary Retry cannot prove that the old executor stopped and has no maintenance owner to produce that proof. **Expected behavior** A deliberate Retry registers missing preservation for the exact unowned source. Existing recovery proves termination and saves the files. Existing admission then checks current user authority, effects, pause and budget gates before one successor can enter the queue. **Steps to reproduce** 1. Persist a failed native run with a prepared workspace, an active lease and a terminal, unowned finalization. 2. Omit the workspace preservation record, as an older deadline handler did. 3. Call the normal task Retry route. Before this fix, preservation remains absent and Retry cannot continue. **Paperclip version or commit** The regression fails on `46de15ff4b99bbee59c38f32f4f30b80ad5bbd24`, which already includes #17788. Related: Refs #17788 (future deadline failures), #17741 (committed recovery wakes), #17733 (connected Retry authority). #17773 adds source diagnostics and does not repair this missing registration. ## What Changed - Use native workspace preservation inside the existing issue-locked Retry transaction. A live finalization owner remains held. - Adopt a valid unowned preservation obligation through the existing bounded maintenance service. Bind it to the deliberate Retry intent without resetting an active generation. - Add real PostgreSQL regressions for exact, mismatched and owned sources. Verify that registration cannot create a termination receipt or a successor. - Exercise the HTTP route, worker, real temporary file merge and queue admission with the stop reply held. Duplicate requests admit one queued turn after preservation. - Extend connected GitHub and Telegram Retry coverage for missing historical preservation. Document the contract and access boundaries. ## Verification - Red: the new historical-state regression fails on the base commit because `workspaceRestoreRecovery` remains absent. The mismatched and owned negative controls pass. - Green: the three focused historical-state cases pass with the fix against isolated local PostgreSQL 17. - The local HTTP journey restores saved files and retains newer host edits. It admits no successor while the provider stop reply is held. It admits one successor after verified preservation. The real GitHub and Telegram admission paths wait behind that recovery. - `pnpm -r typecheck`, `pnpm build`, `node scripts/check-maps.mjs` and `git diff --check` pass. - Green: 152 tests across four relevant suites, including all connected Retry safety cases. The six new cases pass again on final head `b8900801a3181fa673ea45f1129f968bbb08d1fd`. - Final-head GitHub checks: 55 successful, two intentionally skipped Storybook checks, and one neutral Paperclip bot review. Greptile is 5/5 with no unresolved threads. The Paperclip bot stopped before finishing and supplied no approval. - The full local command imported an earlier fixture revision before its assertions were corrected. It reported those two stale connected-fixture assertions and later hit the host PostgreSQL semaphore limit again. It was stopped after final-head CI completed. It is not counted as a full local pass; the complete CI test lanes passed on the final head. - The first local rerun hit the Mac PostgreSQL semaphore limit before assertions. Verification moved to an isolated disposable PostgreSQL container. Early assembled fixtures also exposed the existing destruction-cleanup gate; the retained-workspace fixture now uses its actual `keep_running` disposition. This fix does not weaken cleanup. ## Risks - Retry now registers maintenance for historical native failures. Exact descriptor, allocation and coordinator checks remain owned by the existing preservation service. Physical recovery still requires its existing exclusive owner and provider stop receipt. Registration alone grants no execution authority. - No database migration or provider protocol changes. No model, trust, assignment, retry-budget or external identity changes. - Local recovery uses real database, filesystem, HTTP, worker and admission code. Only the provider lifecycle boundary is simulated. No hosted allocation or live model turn has been verified for this new change. - The workspace recovery test file crosses 1,000 lines because this case shares its real database, filesystem and assembled route fixture. Keeping it there avoids duplicating that fixture or extracting unrelated production behavior. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, repository tools and code execution. The exact model identifier and context-window size are not exposed in this session. ## 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>
244 lines
16 KiB
TypeScript
244 lines
16 KiB
TypeScript
import { writeExecutionControl } from "./execution-control-notifications.js";
|
|
import { DELIVERY_QUEUES } from "./delivery-work-notifications.js";
|
|
import { z } from "zod";
|
|
import { randomUUID } from "node:crypto";
|
|
import { and, desc, eq, inArray, or, sql } from "drizzle-orm";
|
|
import { agentWakeupRequests, chatActions, chatConversations, chatEndpoints, heartbeatRuns, issues, issueRecoveryActions, type Db } from "@paperclipai/db";
|
|
import { conflict, forbidden, notFound } from "../errors.js";
|
|
import { persistActivity } from "./activity-log.js";
|
|
import { createSystemRecovery, readSystemRecovery } from "./system-recovery-state.js";
|
|
import { adoptSystemRecovery, updateSystemRecovery } from "./system-recovery.js";
|
|
import { instanceSettingsService } from "./instance-settings.js";
|
|
import { notifyChatActionWork } from "./chat-work-notifications.js";
|
|
import { authorizationService } from "./authorization.js";
|
|
import { preserveNativeFailedWorkspace } from "./native-runtime/native-failed-workspace-recovery.js";
|
|
import { NativeWorkspaceFinalizationBusyError } from "./native-runtime/native-workspace-finalization-ownership.js";
|
|
import { hasRequiredWorkspaceRecovery } from "./workspace-restore-recovery-state.js";
|
|
|
|
export async function taskRetryMutationAllowed(db: Db, input: {
|
|
companyId: string; issueId: string; actorId: string; actorSource: TaskRetryIntent["actorSource"];
|
|
}) {
|
|
const [issue] = await db.select({ status: issues.status, assigneeAgentId: issues.assigneeAgentId,
|
|
assigneeUserId: issues.assigneeUserId }).from(issues).where(and(eq(issues.companyId, input.companyId), eq(issues.id, input.issueId)));
|
|
if (!issue) return false;
|
|
const permission = await authorizationService(db).decide({
|
|
actor: { type: "board", userId: input.actorId, source: input.actorSource }, action: "issue:mutate",
|
|
resource: { type: "issue", companyId: input.companyId, issueId: input.issueId, ...issue },
|
|
});
|
|
return permission.allowed;
|
|
}
|
|
|
|
export async function assertTaskRetryConversationAllowed(
|
|
db: Db,
|
|
issue: { conversationAgentId?: string | null; conversationUserId?: string | null },
|
|
actorId: string,
|
|
) {
|
|
if (!issue.conversationAgentId) return;
|
|
if (!(await instanceSettingsService(db).getExperimental()).enableAgentChat) throw notFound("Agent Chat is disabled");
|
|
if (issue.conversationUserId !== actorId) throw forbidden("Only the conversation owner can retry a chat run");
|
|
}
|
|
|
|
const taskRetryIntentSchema = z.object({
|
|
schema: z.literal("paperclip.task-retry.v1"), generationId: z.string().uuid(), sourceRunId: z.string().uuid(),
|
|
actorId: z.string().min(1), actorSource: z.enum(["local_implicit", "session", "board_key", "cloud_tenant"]).default("session"), strategy: z.enum(["evaluate", "continue", "fresh_isolated_workspace", "finalize_result"]),
|
|
chatActionId: z.string().uuid().optional(), canonicalIntentId: z.string().uuid().optional(),
|
|
});
|
|
export type TaskRetryIntent = z.infer<typeof taskRetryIntentSchema>;
|
|
export function readTaskRetryIntent(payload: Record<string, unknown> | null | undefined): TaskRetryIntent | null {
|
|
const result = taskRetryIntentSchema.safeParse(payload?.taskRetry);
|
|
return result.success ? result.data : null;
|
|
}
|
|
|
|
/** Caller holds the issue dispatch lock. Newer work supersedes an unadmitted
|
|
* Retry; retiring its authority does not acknowledge or discard saved input. */
|
|
export async function retireSupersededTaskRetry(db: Db, input: {
|
|
companyId: string; issueId: string; requestId: string; sourceRunId: string;
|
|
}) {
|
|
const [latest] = await db.select({ id: heartbeatRuns.id }).from(heartbeatRuns).where(and(
|
|
eq(heartbeatRuns.companyId, input.companyId),
|
|
or(eq(heartbeatRuns.issueId, input.issueId), eq(heartbeatRuns.nativeIssueId, input.issueId),
|
|
sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${input.issueId}`),
|
|
)).orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1);
|
|
if (latest?.id === input.sourceRunId) return false;
|
|
const retired = await db.update(agentWakeupRequests).set({ status: "skipped", error: "task_retry_superseded",
|
|
finishedAt: new Date(), updatedAt: new Date() }).where(and(
|
|
eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.id, input.requestId),
|
|
eq(agentWakeupRequests.reason, "task_retry"), sql`${agentWakeupRequests.runId} is null`,
|
|
sql`${agentWakeupRequests.payload}->>'issueId' = ${input.issueId}`,
|
|
inArray(agentWakeupRequests.status, ["deferred_issue_execution", "queued", "claimed"]),
|
|
)).returning({ id: agentWakeupRequests.id });
|
|
return retired.length > 0;
|
|
}
|
|
|
|
/** Persist a deliberate click under dispatch's issue-first lock order. This
|
|
* transaction never contacts a provider. The normal wake queue owns execution. */
|
|
export async function requestTaskRetry(input: {
|
|
db: Db; companyId: string; issueId: string; actorId: string;
|
|
idempotencyKey: string; observedRunId: string | null; actorSource?: string;
|
|
prepareChatRetry?: (tx: Db, scope: { companyId: string; issueId: string; agentId: string;
|
|
failedRunId: string; initiatedByUserId: string; retryIntentId?: string }) => Promise<{ actionId: string }>;
|
|
}) {
|
|
return writeExecutionControl(input.db, DELIVERY_QUEUES.workspaceRecovery, async tx => {
|
|
const db = tx as unknown as Db;
|
|
const [issue] = await tx.select().from(issues).where(and(eq(issues.companyId, input.companyId),
|
|
eq(issues.id, input.issueId))).for("update");
|
|
if (!issue) throw notFound("Task not found");
|
|
await assertTaskRetryConversationAllowed(db, issue, input.actorId);
|
|
const key = `task-retry:${input.idempotencyKey}`;
|
|
const [sameRequest] = await tx.select().from(agentWakeupRequests).where(and(
|
|
eq(agentWakeupRequests.companyId, input.companyId), eq(agentWakeupRequests.idempotencyKey, key)));
|
|
if (sameRequest) {
|
|
if (sameRequest.payload?.issueId !== issue.id || sameRequest.requestedByActorId !== input.actorId)
|
|
throw conflict("This retry request belongs to a different task or user.");
|
|
const intent = readTaskRetryIntent(sameRequest.payload);
|
|
return { requestId: intent?.canonicalIntentId ?? sameRequest.id, accepted: true, stale: false,
|
|
observedRunId: intent?.sourceRunId ?? null, chatActionId: intent?.chatActionId };
|
|
}
|
|
const [latest] = await tx.select().from(heartbeatRuns).where(and(eq(heartbeatRuns.companyId, issue.companyId),
|
|
or(eq(heartbeatRuns.issueId, issue.id), eq(heartbeatRuns.nativeIssueId, issue.id),
|
|
sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issue.id}`)))
|
|
.orderBy(desc(heartbeatRuns.createdAt), desc(heartbeatRuns.id)).limit(1);
|
|
if (["done", "cancelled"].includes(issue.status) || !issue.assigneeAgentId || !latest ||
|
|
latest.agentId !== issue.assigneeAgentId || input.observedRunId !== latest.id) {
|
|
return { requestId: null, accepted: false, stale: true, observedRunId: latest?.id ?? null };
|
|
}
|
|
let active = (await tx.select().from(agentWakeupRequests).where(and(
|
|
eq(agentWakeupRequests.companyId, issue.companyId), eq(agentWakeupRequests.agentId, issue.assigneeAgentId),
|
|
sql`${agentWakeupRequests.payload}->>'issueId' = ${issue.id}`,
|
|
sql`${agentWakeupRequests.payload}->'taskRetry'->>'schema' = 'paperclip.task-retry.v1'`,
|
|
inArray(agentWakeupRequests.status, ["deferred_issue_execution", "queued", "claimed"]),
|
|
)).orderBy(desc(agentWakeupRequests.requestedAt)).limit(1)).at(0);
|
|
const activeIntent = readTaskRetryIntent(active?.payload);
|
|
if (active && !active.runId && activeIntent && await retireSupersededTaskRetry(db, {
|
|
companyId: issue.companyId, issueId: issue.id, requestId: active.id, sourceRunId: activeIntent.sourceRunId,
|
|
})) active = undefined;
|
|
if (active && !active.runId && issue.conversationAgentId && active.requestedByActorId !== issue.conversationUserId) {
|
|
// A former owner's unadmitted receipt cannot own the new owner's Retry.
|
|
await tx.update(agentWakeupRequests).set({ status: "failed", finishedAt: new Date(), updatedAt: new Date() })
|
|
.where(eq(agentWakeupRequests.id, active.id));
|
|
active = undefined;
|
|
}
|
|
const busy = ["queued", "running", "scheduled_retry"].includes(latest.status);
|
|
if (!busy) {
|
|
// Historical deadline failures can predate their preservation record.
|
|
// Register the exact prepared source; this proves neither a stopped
|
|
// executor nor permission to replay its work.
|
|
try { await preserveNativeFailedWorkspace(db, latest); }
|
|
catch (error) { if (!(error instanceof NativeWorkspaceFinalizationBusyError)) throw error; }
|
|
}
|
|
const records = await tx.select().from(issueRecoveryActions).where(and(
|
|
eq(issueRecoveryActions.companyId, issue.companyId), eq(issueRecoveryActions.sourceIssueId, issue.id),
|
|
sql`${issueRecoveryActions.evidence}->>'runId' = ${latest.id}`)).for("update");
|
|
for (let index = 0; !busy && index < records.length; index++) {
|
|
const action = records[index];
|
|
if (!hasRequiredWorkspaceRecovery(action.evidence) || readSystemRecovery(action.evidence)) continue;
|
|
await adoptSystemRecovery(db, { companyId: issue.companyId, issueId: issue.id, runId: latest.id, actionId: action.id });
|
|
[records[index]] = await tx.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.id, action.id));
|
|
}
|
|
// An unadmitted request can outlive its exhausted repair. A new deliberate
|
|
// click replaces that generation; a replay of the same key returned above.
|
|
if (active && !busy && !active.runId && records.some(action =>
|
|
readSystemRecovery(action.evidence)?.state === "exhausted")) {
|
|
await tx.update(agentWakeupRequests).set({ status: "failed", finishedAt: new Date(), updatedAt: new Date() })
|
|
.where(eq(agentWakeupRequests.id, active.id));
|
|
active = undefined;
|
|
}
|
|
const id = randomUUID();
|
|
const intent: TaskRetryIntent = { schema: "paperclip.task-retry.v1", generationId: active?.id ?? id,
|
|
sourceRunId: latest.id, actorId: input.actorId,
|
|
actorSource: taskRetryIntentSchema.shape.actorSource.parse(input.actorSource ?? "session"), strategy: "evaluate",
|
|
...(active && activeIntent?.chatActionId ? { chatActionId: activeIntent.chatActionId } : {}),
|
|
...(active ? { canonicalIntentId: active.id } : {}) };
|
|
// Preserve external chat authority through its existing admission owner.
|
|
// Generic board Retry must never manufacture a chat-inbound request.
|
|
const [chat] = await tx.select({ id: chatConversations.id }).from(chatConversations)
|
|
.innerJoin(chatEndpoints, eq(chatEndpoints.id, chatConversations.endpointId)).where(and(
|
|
eq(chatConversations.companyId, issue.companyId), eq(chatConversations.issueId, issue.id),
|
|
eq(chatEndpoints.externalExecutionPolicy, "restricted"))).limit(1);
|
|
await tx.insert(agentWakeupRequests).values({ id, companyId: issue.companyId, agentId: issue.assigneeAgentId,
|
|
source: "on_demand", triggerDetail: "manual", reason: "task_retry", idempotencyKey: key,
|
|
requestedByActorType: "user", requestedByActorId: input.actorId,
|
|
status: active || busy ? "coalesced" : "deferred_issue_execution",
|
|
runId: active?.runId ?? (busy ? latest.id : null),
|
|
payload: { issueId: issue.id, taskRetry: intent }, finishedAt: active || busy ? new Date() : null });
|
|
if (chat && !active && !busy) {
|
|
if (!input.prepareChatRetry) throw conflict("Retry is unavailable for this connected conversation.");
|
|
intent.chatActionId = (await input.prepareChatRetry(db, { companyId: issue.companyId, issueId: issue.id,
|
|
agentId: issue.assigneeAgentId, failedRunId: latest.id, initiatedByUserId: input.actorId, retryIntentId: id })).actionId;
|
|
await tx.update(agentWakeupRequests).set({ payload: { issueId: issue.id, taskRetry: intent } })
|
|
.where(eq(agentWakeupRequests.id, id));
|
|
}
|
|
if (!busy) {
|
|
for (const action of records) {
|
|
const previous = readSystemRecovery(action.evidence);
|
|
if (!previous || previous.state === "complete") continue;
|
|
if (previous.state === "running") {
|
|
await updateSystemRecovery(db, { companyId: issue.companyId, issueId: issue.id, runId: latest.id,
|
|
actionId: action.id }, previous, { ...previous, intentId: active?.id ?? id });
|
|
continue;
|
|
}
|
|
// Active repair keeps its attempt and cooldown. An exhausted generation
|
|
// gets a new budget only because this click is a new durable intent.
|
|
const next = previous.state === "exhausted"
|
|
? { ...createSystemRecovery(latest.id, id), revision: previous.revision + 1,
|
|
preservationOperationIds: previous.preservationOperationIds,
|
|
providerCooldownAt: previous.providerCooldownAt, nextAttemptAt: previous.providerCooldownAt }
|
|
: { ...previous, revision: previous.revision + 1, intentId: active?.id ?? id,
|
|
// Retry may advance our backoff, never a provider's cooldown and
|
|
// never an operation whose physical owner is still running.
|
|
nextAttemptAt: Math.max(Date.now(), previous.providerCooldownAt ?? 0) };
|
|
if (next.generationId !== previous.generationId) await persistActivity(db, {
|
|
companyId: issue.companyId, actorType: "system", actorId: "task-retry", entityType: "issue", entityId: issue.id,
|
|
action: "issue.recovery_generation_replaced", runId: latest.id,
|
|
details: { recoveryActionId: action.id, previousGeneration: previous, nextGenerationId: next.generationId },
|
|
});
|
|
await updateSystemRecovery(db, { companyId: issue.companyId, issueId: issue.id, runId: latest.id,
|
|
actionId: action.id }, previous, next);
|
|
}
|
|
}
|
|
await persistActivity(db, { companyId: issue.companyId, actorType: "user", actorId: input.actorId,
|
|
action: "issue.retry_requested", entityType: "issue", entityId: issue.id, runId: latest.id,
|
|
details: { requestId: active?.id ?? id, sourceRunId: latest.id, attachedToExistingWork: Boolean(active || busy) } });
|
|
return { requestId: active?.id ?? id, accepted: true, stale: false, observedRunId: latest.id,
|
|
chatActionId: intent.chatActionId };
|
|
});
|
|
}
|
|
|
|
/** Chat owns admission and authorization. The task receipt follows that
|
|
* durable outcome; it never launches a parallel generic board execution. */
|
|
export async function reconcileTaskChatRetryReceipts(db: Db) {
|
|
const pending = await db.select().from(agentWakeupRequests).where(and(
|
|
eq(agentWakeupRequests.reason, "task_retry"), eq(agentWakeupRequests.status, "deferred_issue_execution"),
|
|
sql`${agentWakeupRequests.payload}#>>'{taskRetry,chatActionId}' is not null`,
|
|
)).orderBy(agentWakeupRequests.updatedAt).limit(50);
|
|
for (const wake of pending) {
|
|
const intent = readTaskRetryIntent(wake.payload);
|
|
if (!intent?.chatActionId) continue;
|
|
const [chat] = await db.select().from(chatActions).where(and(eq(chatActions.companyId, wake.companyId),
|
|
eq(chatActions.id, intent.chatActionId), eq(chatActions.kind, "failed_run_retry"),
|
|
sql`${chatActions.payload}->>'issueId' = ${String(wake.payload?.issueId)}`));
|
|
const [receipt] = chat ? await db.select().from(agentWakeupRequests).where(and(
|
|
eq(agentWakeupRequests.companyId, wake.companyId), eq(agentWakeupRequests.id, chat.id),
|
|
eq(agentWakeupRequests.agentId, wake.agentId))) : [];
|
|
// A deferred chat receipt is admitted input, not a completed dispatch.
|
|
// Re-stage that exact action through chat's owner; it rechecks authority
|
|
// and normal queue policy before updating the existing receipt.
|
|
if (chat?.status === "processed" && chat.payload.taskRetryIntentId === wake.id &&
|
|
chat.payload.failedRunId === intent.sourceRunId && receipt?.status === "deferred_issue_execution" &&
|
|
!receipt.runId && receipt.payload?.taskRetryRequestId === wake.id) {
|
|
await db.transaction(async tx => {
|
|
await notifyChatActionWork(tx, "failed_run_retry");
|
|
await tx.update(chatActions).set({ status: "issued", updatedAt: new Date() })
|
|
.where(and(eq(chatActions.id, chat.id), eq(chatActions.companyId, wake.companyId),
|
|
eq(chatActions.status, "processed"), eq(chatActions.updatedAt, chat.updatedAt)));
|
|
});
|
|
}
|
|
const failed = chat && ["failed", "cancelled"].includes(chat.status);
|
|
await db.update(agentWakeupRequests).set({
|
|
...(receipt?.runId ? { status: "coalesced", runId: receipt.runId, finishedAt: new Date() }
|
|
: failed ? { status: "failed", finishedAt: new Date() } : {}),
|
|
updatedAt: new Date(),
|
|
}).where(and(eq(agentWakeupRequests.id, wake.id), eq(agentWakeupRequests.status, "deferred_issue_execution")));
|
|
}
|
|
}
|