mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-31 14:16:39 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - A parked scheduled retry has no executor to finish an interrupted Stop request. > - Its saved cancellation fence already records the work that remains. > - The orphan reaper currently scans for that work on every heartbeat, including idle periods. > - This PR registers the existing records with the shared delivery scheduler and wakes recovery after committed writes. > - Recovery keeps the existing cancellation guards and settlement behavior, then becomes quiet. ## Linked Issues or Issue Description Refs #17666, the merged preceding PR in the recurring-server-work migration stack. This PR targets master. Related cancellation work found in the dedup search: #12301 and #7340. This PR changes the trigger for interrupted parked-retry cancellation; it does not replace their cancellation or retry policies. **What happened?** The periodic orphan reaper queries scheduled retries with cancellation fences even when no cancellation needs recovery. Its first-20-row scan can also stop behind malformed evidence. **Expected behavior** Committed cancellation intent wakes bounded recovery. Failed work retains a retry. Startup reconstructs durable work. Empty or settled work performs no recurring SQL for this duty. **Steps to reproduce** 1. Start a server with heartbeat scheduling enabled and no interrupted retry cancellations. 2. Observe the orphan reaper repeatedly query for cancellation fences. 3. Cancel a scheduled retry and fail the final status write after its fence commits. Recovery must complete the existing Stop request without running a provider or dropping its metadata. **Paperclip version or commit** Current parent squash `7afee8f0622e273d10506b0febfb68f143d168b1`. Source acceptance `66107ae0a7ad85fd0476f91c4f8f98ae74a7a048`. **Deployment mode** Self-hosted server with the existing single-process transaction signal model. Local evidence uses PostgreSQL 17. ## What Changed - Add one domain-owned recovery worker over existing heartbeat run rows. No new table. It delegates to run control and preserves original cancellation, budget, and operator metadata. - Signal cancellation fences, lifecycle transitions, dispatch deferral/promotion/suppression, relevant settings changes, and deletion within their owning transactions. Wrap only the ordinary database fence in a short transaction; provider calls stay outside it. - Keep startup/manual recovery compatible and remove this duty from the periodic orphan sweep. Other reaper duties remain for separate PRs. - Extend the shared scoped worker to retain pending work with a memory-only admission retry while its owner is paused. Without pending work, it schedules nothing. Existing GitHub consumers retain their behavior. - Add a concurrent idempotent partial index and lifecycle/producer tests. Enable guarded disposable PostgreSQL URLs in affected test harnesses. - Reconstruct one hundred candidates per scope per scheduler pass. Resume from the saved scope cursor and retain fair twenty-item execution batches. Manual recovery starts cancellation before reading the full backlog. - Require the recovered browser reply to be saved before testing reload. Keep all original assertions and time limits. - Record the full producer audit, lifecycle limits, and failed attempts in `doc/plans/2026-10-25-scheduled-retry-cancellation-sleep-audit.md`. ## Verification Published head: `405e533394b26215e22d0452e9b0207d3f784a79`. Source acceptance: `a35fa43b98e2bd378d4a920d2775ce243654838b`. The final commit changes the audit only. - All 363 backend cases in ten complete owner and consumer files pass without skips. This includes actual Stop/retry producers, transaction settlement, interrupted-write recovery, pause/resume, shutdown, fair backlog processing and quiet after settlement. Seven separate fresh PostgreSQL 17 databases prevent fixture interference. - The two embedded-only backend fixtures temporarily use separate external databases on this IPC-exhausted host. Their fixture imports and native client teardown are restored byte for byte afterward. Assertions and timing limits are unchanged. CI must still pass with the original embedded fixtures. - Two paging regressions fail on the prior worker and pass on the fix. They cover cancellation before full reconstruction, a malformed first page, fair batches, a newly committed intent behind the cursor, and no later idle SQL. Per-candidate admission reads remain deliberate safety checks. - The repeated CI reload failure is reproduced with a temporary twelve-second fixture settlement delay. The trace retains the same five-second missing-reply assertion, even though a cleanup race masked it in the top-level error. Requiring exactly one saved reply from the resumed run passes under the identical delay. The fault is removed. All six browser journeys pass on the final source in 40.5 seconds, with zero retries and the original 120-second case and five-second reload limits. - Browser acceptance uses the canonical CLI/server/static-UI path and a fresh external database. Only fixture database-URI setup is temporarily adapted and then restored. This is not an unmodified embedded CI-environment reproduction. The six cases include the controlled case; repeated runs are not added to the distinct count. - Actual database build, server typecheck/build, module boundaries and exact-current migration order pass. The database source tree is unchanged from the verified 0405-to0406 upgrade: all 404 prior receipts and 300 companies stay intact, exactly one receipt is added, replay adds none, and the index is valid on upgraded and fresh databases. The five snapshots remain 0402 through 0406. - All earlier failures remain recorded in the audit. Prior head `48b3fcb83f305616df02a4337a42ec0867bb4b80` had exact-head Greptile 5/5 but failed the repeated browser assertion. It was not retried blindly. Its completed Ponytail review's backlog concern and old migration paragraph are addressed here. Old scores do not satisfy new-head gates. - Current-head CI passes. Exact-head Greptile and Ponytail both give 5/5, with no remaining actionable findings. These checks do not prove deployed instance or database suspension. ## Risks - Only supported in-process writers produce local transaction signals. Independent SQL writers or other replicas need a separate ingress contract. Startup reconstructs saved obligations. - A real failure retains bounded retries. Unknown commit outcomes retain the shared intent hold. Neither is discarded to report quiet. - Per-owner drain uses a memory-only admission recheck. The shared helper change affects existing consumers, which have focused regression coverage. - The partial index is concurrent and idempotent; it adds index storage. Each scoped load reads at most one hundred rows. The shared helper retains discovered pending IDs between passes, so a large unresolved backlog still uses memory. - Cloud alarms, idle-safety exemptions, and other reaper duties remain separate. Merge requires passing current-head CI and review gates. ## Model Used OpenAI Codex, GPT-6 family, with reasoning, shell tools, and local PostgreSQL tests. The runtime does not expose a more precise model identifier or context-window size for this turn. ## 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>
136 lines
9.2 KiB
TypeScript
136 lines
9.2 KiB
TypeScript
import { notifyScheduledRetryCancellation } from "../scheduled-retry-cancellation-notifications.js";
|
|
import { and, eq, sql } from "drizzle-orm";
|
|
import { heartbeatRuns, nativeRunFinalizations, type Db } from "@paperclipai/db";
|
|
import { badRequest, conflict } from "../../errors.js";
|
|
|
|
const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
|
|
const record = (value: unknown): Record<string, unknown> => value !== null && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
|
export function cancellationRequestId(value: unknown): string | undefined {
|
|
if (value === undefined) return undefined;
|
|
if (typeof value !== "string" || !UUID.test(value)) throw badRequest("Invalid cancellationRequestId");
|
|
return value.toLowerCase();
|
|
}
|
|
export const cancellationIntentId = (requestId: string) => `native-cancellation:${requestId}`;
|
|
|
|
/** Run only while holding this company's run row lock. An earlier uncorrelated
|
|
* startup fence is also a conflicting Stop, even before native intent exists. */
|
|
export function assertCancellationRequest(result: unknown, requestId: string, allowUnclaimed = false) {
|
|
const value = record(result), startup = record(value.startupCancellation), native = record(value.nativeCancellation);
|
|
if ((Object.hasOwn(value, "startupCancellation") && startup.cancellationRequestId !== requestId)
|
|
|| (Object.hasOwn(value, "nativeCancellation") && native.intentId !== cancellationIntentId(requestId))
|
|
|| (!allowUnclaimed && startup.cancellationRequestId !== requestId)) {
|
|
throw conflict("Cancellation request conflicts with an earlier Stop");
|
|
}
|
|
}
|
|
|
|
/** Evaluate only while both the run and coordinator are locked. A UUID alone
|
|
* is not retry authority; a resumed Stop needs its exact durable native intent. */
|
|
export function nativeRetryCancellationEligible(input: {
|
|
runId: string; companyId: string; issueId: string; resultJson: unknown; requestId?: string; scope?: string;
|
|
}, coordinator: { phase?: string; failureCode?: string | null } | null | undefined) {
|
|
if (coordinator?.phase === "retryable_failure") return true;
|
|
if (coordinator?.phase !== "terminal_failure" || coordinator.failureCode !== "native_retry_cancelled") return false;
|
|
const result = record(input.resultJson), native = record(result.nativeCancellation);
|
|
return (!input.requestId || (record(result.startupCancellation).cancellationRequestId === input.requestId
|
|
&& native.intentId === cancellationIntentId(input.requestId)))
|
|
&& native.schema === "paperclip.native-cancellation.v1" && typeof native.intentId === "string" && !!native.intentId
|
|
&& native.runId === input.runId && native.companyId === input.companyId && native.issueId === input.issueId
|
|
&& native.scope === (input.scope ?? "run") && ["pending", "acknowledged"].includes(String(native.dispatchState))
|
|
&& typeof native.dispatched === "boolean" && typeof native.intentAuditId === "string" && !!native.intentAuditId
|
|
&& (native.dispatchState === "pending" || (typeof native.acknowledgementAuditId === "string"
|
|
&& !!native.acknowledgementAuditId && native.acknowledgementAuditId !== native.intentAuditId));
|
|
}
|
|
|
|
/** Final failed -> cancelled CAS: a later board/recovery outcome must win.
|
|
* The row-locking subquery prevents a coordinator-only writer from changing
|
|
* the retry disposition between this check and the status update. */
|
|
export function nativeRetryCancellationCommitCondition(expectedResult: unknown) {
|
|
const native = record(record(expectedResult).nativeCancellation);
|
|
if (native.dispatchState !== "acknowledged" || typeof native.runId !== "string"
|
|
|| typeof native.companyId !== "string" || typeof native.issueId !== "string"
|
|
|| !nativeRetryCancellationEligible({ runId: native.runId, companyId: native.companyId,
|
|
issueId: native.issueId, resultJson: expectedResult }, { phase: "terminal_failure", failureCode: "native_retry_cancelled" })) {
|
|
return sql`false`;
|
|
}
|
|
return sql`${heartbeatRuns.status} = 'failed' and ${heartbeatRuns.runtimeMode} = 'native'
|
|
and ${heartbeatRuns.id} = ${native.runId} and ${heartbeatRuns.companyId} = ${native.companyId}
|
|
and ${heartbeatRuns.nativeIssueId} = ${native.issueId}
|
|
and ${heartbeatRuns.resultJson} = ${JSON.stringify(expectedResult)}::jsonb
|
|
and exists (select 1 from ${nativeRunFinalizations}
|
|
where ${nativeRunFinalizations.runId} = ${heartbeatRuns.id}
|
|
and ${nativeRunFinalizations.companyId} = ${heartbeatRuns.companyId}
|
|
and ${nativeRunFinalizations.issueId} = ${heartbeatRuns.nativeIssueId}
|
|
and ${nativeRunFinalizations.phase} = 'terminal_failure'
|
|
and ${nativeRunFinalizations.failureCode} = 'native_retry_cancelled'
|
|
for update nowait)`;
|
|
}
|
|
|
|
export function rethrowNativeCancellationLockConflict(error: unknown): never {
|
|
const value = record(error);
|
|
if (value.code === "55P03" || record(value.cause).code === "55P03") {
|
|
throw conflict("Native retry cancellation is busy; retry the same request");
|
|
}
|
|
throw error;
|
|
}
|
|
|
|
/** Reserve the caller's identity before handoff or dispatch. No schema change:
|
|
* the existing startup fence and native intent carry the correlation. */
|
|
export async function claimCancellationRequest(db: Db, runId: string, companyId: string, requestId: string, userId: string | null) {
|
|
if (cancellationRequestId(requestId) !== requestId) throw badRequest("Invalid cancellationRequestId");
|
|
if (userId !== null && (typeof userId !== "string" || !userId)) throw badRequest("Invalid cancellation request actor");
|
|
return db.transaction(async tx => {
|
|
const run = await tx.select().from(heartbeatRuns)
|
|
.where(and(eq(heartbeatRuns.id, runId), eq(heartbeatRuns.companyId, companyId)))
|
|
.for("update").limit(1).then(rows => rows[0]);
|
|
if (!run || run.runtimeMode !== "native") throw conflict("Correlated Stop requires a native run");
|
|
assertCancellationRequest(run.resultJson, requestId, true);
|
|
const result = record(run.resultJson);
|
|
let pendingRetry = false;
|
|
if (run.status === "failed" && run.nativeIssueId) {
|
|
// Eligibility and the caller fence must share a transaction. Coordinator
|
|
// writers need not lock the run. NOWAIT avoids a lock-order deadlock with
|
|
// execution's coordinator -> run claim; a busy claim can be retried.
|
|
const coordinator = await tx.select().from(nativeRunFinalizations)
|
|
.where(and(eq(nativeRunFinalizations.runId, runId), eq(nativeRunFinalizations.companyId, companyId),
|
|
eq(nativeRunFinalizations.issueId, run.nativeIssueId)))
|
|
.for("update", { noWait: true }).limit(1).then(rows => rows[0]);
|
|
pendingRetry = nativeRetryCancellationEligible({ runId, companyId, issueId: run.nativeIssueId,
|
|
resultJson: result, requestId }, coordinator);
|
|
}
|
|
if (record(result.startupCancellation).cancellationRequestId === requestId) {
|
|
const actor = record(record(result.startupCancellation).requestedBy);
|
|
if (actor.type !== "board" || actor.userId !== userId) throw conflict("Cancellation request belongs to another actor");
|
|
if (!pendingRetry && !["queued", "running", "scheduled_retry"].includes(run.status)) {
|
|
const native = record(result.nativeCancellation);
|
|
if (run.status !== "cancelled" || native.schema !== "paperclip.native-cancellation.v1"
|
|
|| native.intentId !== cancellationIntentId(requestId) || native.runId !== runId || native.companyId !== companyId
|
|
|| native.scope !== "run" || native.dispatchState !== "acknowledged" || typeof native.dispatched !== "boolean"
|
|
|| typeof native.intentAuditId !== "string" || !native.intentAuditId
|
|
|| typeof native.acknowledgementAuditId !== "string" || !native.acknowledgementAuditId
|
|
|| native.intentAuditId === native.acknowledgementAuditId) throw conflict("Correlated Stop run is already terminal without acknowledgement");
|
|
}
|
|
return run;
|
|
}
|
|
if (!pendingRetry && !["queued", "running", "scheduled_retry"].includes(run.status)) throw conflict("Correlated Stop run is already terminal");
|
|
await notifyScheduledRetryCancellation(tx, { companyId, runId });
|
|
const [claimed] = await tx.update(heartbeatRuns).set({ resultJson: { ...result,
|
|
startupCancellation: { requestedAt: new Date().toISOString(), beforeNativeSelection: false, cancellationRequestId: requestId, requestedBy: { type: "board", userId }, ...(run.status === "failed" ? { retryCancellation: true } : {}) },
|
|
} }).where(and(eq(heartbeatRuns.id, runId), eq(heartbeatRuns.companyId, companyId))).returning();
|
|
if (!claimed) throw conflict("Correlated Stop run changed");
|
|
return claimed;
|
|
}).catch(rethrowNativeCancellationLockConflict);
|
|
}
|
|
|
|
/** Used by the default Stop path too: never erase a correlated reservation. */
|
|
export function startupCancellationFence(requestedAt: string) {
|
|
return sql`CASE
|
|
WHEN ${heartbeatRuns.resultJson}->'startupCancellation'->>'cancellationRequestId' IS NOT NULL
|
|
THEN ${heartbeatRuns.resultJson}->'startupCancellation'
|
|
ELSE jsonb_build_object('requestedAt', ${requestedAt}::text,
|
|
'beforeNativeSelection', ${heartbeatRuns.runtimeMode} = 'legacy'
|
|
and ${heartbeatRuns.runtimeModeResolvedAt} is null
|
|
and ${heartbeatRuns.executionStage} = 'preparing'
|
|
and coalesce(${heartbeatRuns.runnerProfileJson}->'adapterDispatch'->>'adapterType' = 'paperclip_runner', false))
|
|
END`;
|
|
}
|