mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-30 05:16:50 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs can retain accepted results after an unsafe workspace export fails. > - Historical recovery can retire an old notice or resume the accepted result without another provider turn. > - The periodic reaper repeatedly scans failures that cannot currently recover. > - Existing rows and committed changes can identify the actual recovery obligation. > - This pull request wakes recovery from those changes and retries only owed work. > - Dormant history becomes quiet while ownership and explicit operator decisions remain protected. ## Linked Issues or Issue Description **What happened?** The native finalization reaper repeatedly visits historical unsafe workspace-export failures. Cancelled tasks, newer runs, changed contracts, and explicit operator decisions can make those rows permanently ineligible. They still produce periodic database reads. **What did you expect?** Retire stale unsafe-export notices and recover eligible accepted results. Stop reading dormant history until a relevant committed change occurs. Keep a retry while a real obligation is blocked by ownership or a transient failure. **Steps to reproduce** Retain an unsafe-export terminal failure whose task is cancelled or superseded. Leave the instance idle after startup. The periodic reaper continues reading that history. Prerequisite #17680 is merged into master. Related: #14402 established safe export finalization; #14329 was an earlier historical recovery proposal. #17679 owns separate policy-review cleanup. This PR changes only historical unsafe-export scheduling and its producers. ## What Changed - Register a scoped worker through the existing coordinator and scheduler. Remove this duty from periodic reaping. Preserve the manual/startup recovery entry point. - Distinguish stale notices, eligible accepted results, dormant history, and concrete ownership contention. Reuse the existing mutation owner and lock order. Preserve explicit settlement vetoes and exact result/contract identity. - Signal relevant task, run, action, and deletion writes before they commit. Existing transaction boundaries handle nested commits and uncertain outcomes. Notifications contain only scope IDs. - Retain a retry only for owed work. Reconstruct work at startup, preserve drain/resume, and stop through the shared lifecycle. - Add concurrent, idempotent partial index migration 0410 for unsafe terminal failures. Keep the five newest generated schema snapshots. - Document producer coverage, query plans, supported writer boundaries, and test failures in the unsafe-export audit. ## Verification - Source `2a13dd08a6cc52f0adab09e3df9ce31e97b746b9` passes all 2188 cases across 24 complete files, without skips. The main selection passes 2129 cases in 192.58 seconds; safe-replacement passes 59 in 26.04 seconds. Coverage includes all 968 native-session cases, real export/action producers, explicit continuation, workspace reconstruction, interaction routing, Stop, queue and Slack consumers. The audit gives per-file counts. - Twenty separate fresh PostgreSQL 17 databases migrate through 0410 before setup. Four embedded-only fixture imports temporarily use their own external database on this IPC-exhausted host, then are restored byte for byte. Assertions and timing limits stay unchanged. This is qualified external-database acceptance, not an unmodified embedded CI run. - Database build, server typecheck/build, module boundaries and exact-head migration order pass. Normal integration preserves the complete original runtime/schema patch and unrelated upstream blobs. Original concurrent index SQL, all prior executable history, journal entries and retained snapshot bytes are unchanged; five snapshots 0406 through 0410 remain. Added comment removal preserves the full non-comment TypeScript token streams. - A populated 0409-to0410 upgrade preserves all 408 prior migration receipts and 38 companies. It appends exactly one receipt; replay adds none. The index is valid and ready on fresh and upgraded databases. - The owning audit retains earlier fixture failures, skipped checks and notice-retirement regression history. Older combined stack checks retain their original scope in their owning audits. Actual parent `73d24aa5acfcf17b3f676b4da7cd05eb70b60b99` is reconciled through a normal merge. Its tree equals the reviewed ancestor and the complete accepted child tree is unchanged. Fresh published-head CI and Greptile 5/5 remain required. No deployed sleep, fleet-cost or live provider qualification claim is made. - Inherited prefix correction: current integration `8f43c9f81fac23b50997e7f0e1d0d9cf383a8109` changes only two values in a previously unselected corpus fixture. All production code, the complete package tree and all 24 accepted test files remain byte-identical to the 2188-case source above. The additional complete 72-case corpus passes in 14.00 seconds and again on the same database in 13.08 seconds. Retained PSC/OUT and sentinel rows remain unchanged. The repeat is not double-counted. The parent audit retains the original collision and rejected cleanup attempt. - Latest parent context: integration `4d51231abe8b68c21ce896d0d2ac16865978c8ab` carries only the parent's one-argument GitHub observation correction and an independent master cleanup assertion. All production code, packages, 24 accepted suite files and the additional corpus are unchanged. The two focused native-presentation cases pass in 15.14 seconds and the complete cleanup-removal suite passes all six cases in 8.31 seconds on two fresh 0410 PostgreSQL fixtures. This does not claim the full parent workflow was rerun here. Current head `b28399babac72ba9db1c4a4ef882c5e87aded38c` has passing CI and Greptile 5/5 with no actionable findings. The first CI attempt lost runners: fourteen jobs explicitly received simultaneous shutdown signals, two reported lost runner communication, and two aggregate gates failed downstream. One failed-job retry on unchanged source passes. The underlying host shutdown cause remains unconfirmed; no code fix or extra retry is claimed. [CI run and attempts](https://github.com/paperclipai/paperclip/actions/runs/40819408528). No Ponytail verdict is present. ## Risks - A missing future producer can delay dormant-history discovery until restart. The audit names current producers. Independent SQL writers and other server processes remain outside the process-local signal contract. - Physical ownership and uncertain operations retain retries. Time alone does not authorize taking workspace ownership. No provider turn or unsafe archive retry is added. - The recovery owner remains authoritative if state changes after discovery. Terminal UI status does not authorize resource destruction or accepted-result replay. - The partial index adds matching-row write/storage cost. It builds concurrently through the existing restart-safe migration runner. - This proves the changed worker becomes quiet. It does not prove host suspension, external wake coverage, or fleet cost savings. Other sleep migrations remain. This slice still requires fresh current-head CI and reviews. ## Model Used OpenAI Codex, based on GPT-6, with reasoning and code-execution tools. The exact serving model ID 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>
288 lines
11 KiB
TypeScript
288 lines
11 KiB
TypeScript
import { notifyUnsafeWorkspaceExportRunChange } from "./unsafe-workspace-export-notifications.js";
|
|
import { notifyCancelledWorkspacePreservation } from "./cancelled-workspace-preservation-notifications.js";
|
|
import { notifyInstructionCollection } from "./instruction-collection-notifications.js";
|
|
import { notifyExecutionRunWork } from "./execution-control-notifications.js";
|
|
import { randomUUID } from "node:crypto";
|
|
import { logger } from "../middleware/logger.js";
|
|
import { and, eq, isNotNull, lte, sql } from "drizzle-orm";
|
|
import {
|
|
agents,
|
|
heartbeatRuns,
|
|
issues,
|
|
nativeRunFinalizations,
|
|
type Db,
|
|
} from "@paperclipai/db";
|
|
import { parseIssueExecutionState } from "./issue-execution-policy.js";
|
|
import { issueRecoveryActionService } from "./issue-recovery-actions.js";
|
|
import { hasLiveLegacyController } from "./legacy-controller-lease.js";
|
|
import { reportRunFailure } from "./run-failure-report.js";
|
|
import { getNativeReviewAssignment, readNativeReviewAssignmentContext } from "./native-runtime/native-review-participant.js";
|
|
import { preserveNativeFailedWorkspace } from "./native-runtime/native-failed-workspace-recovery.js";
|
|
|
|
/** Only newly recorded control deadlines are eligible. Upgrades never replay ambiguous historical runs. */
|
|
export async function reconcileAbandonedExecutionControl(
|
|
db: Db,
|
|
now = new Date(),
|
|
) {
|
|
const nativeDue = await db
|
|
.select({
|
|
runId: nativeRunFinalizations.runId,
|
|
issueId: nativeRunFinalizations.issueId,
|
|
companyId: nativeRunFinalizations.companyId,
|
|
})
|
|
.from(nativeRunFinalizations)
|
|
.where(
|
|
and(
|
|
isNotNull(nativeRunFinalizations.controlDeadlineAt),
|
|
lte(nativeRunFinalizations.controlDeadlineAt, now),
|
|
),
|
|
)
|
|
.limit(50);
|
|
const controlDue = await db
|
|
.select({
|
|
runId: heartbeatRuns.id,
|
|
companyId: heartbeatRuns.companyId,
|
|
context: heartbeatRuns.contextSnapshot,
|
|
})
|
|
.from(heartbeatRuns)
|
|
.where(
|
|
and(
|
|
isNotNull(heartbeatRuns.executionControlDeadlineAt),
|
|
lte(heartbeatRuns.executionControlDeadlineAt, now),
|
|
),
|
|
)
|
|
.limit(50);
|
|
const due = [
|
|
...new Map(
|
|
[
|
|
...nativeDue,
|
|
...controlDue.map((row) => ({
|
|
...row,
|
|
issueId:
|
|
typeof row.context?.issueId === "string"
|
|
? row.context.issueId
|
|
: null,
|
|
})),
|
|
].map((row) => [row.runId, row]),
|
|
).values(),
|
|
];
|
|
let surfaced = 0;
|
|
// Bound contention latency across independent tasks: a locked task must not
|
|
// consume the entire reconciliation window for every task behind it.
|
|
let nextCandidate = 0;
|
|
await Promise.all(Array.from({ length: Math.min(5, due.length) }, async () => {
|
|
while (nextCandidate < due.length) {
|
|
const candidate = due[nextCandidate++]!;
|
|
try {
|
|
// Set inside the transaction only when the write below genuinely
|
|
// transitions the run into "failed". Read after the transaction
|
|
// commits, so a rolled-back write never reports a false failure.
|
|
let terminalRunToReport: typeof heartbeatRuns.$inferSelect | null = null;
|
|
const repaired = await db.transaction(async (tx) => {
|
|
await tx.execute(
|
|
sql`select set_config('statement_timeout', '15000', true), set_config('lock_timeout', '1000', true)`,
|
|
);
|
|
const task = candidate.issueId
|
|
? (
|
|
await tx
|
|
.select()
|
|
.from(issues)
|
|
.where(
|
|
and(
|
|
eq(issues.id, candidate.issueId),
|
|
eq(issues.companyId, candidate.companyId),
|
|
),
|
|
)
|
|
.for("update")
|
|
)[0]
|
|
: null;
|
|
const [coordinator] = await tx
|
|
.select()
|
|
.from(nativeRunFinalizations)
|
|
.where(
|
|
and(
|
|
eq(nativeRunFinalizations.runId, candidate.runId),
|
|
eq(nativeRunFinalizations.companyId, candidate.companyId),
|
|
),
|
|
)
|
|
.for("update");
|
|
const [run] = await tx
|
|
.select()
|
|
.from(heartbeatRuns)
|
|
.where(
|
|
and(
|
|
eq(heartbeatRuns.id, candidate.runId),
|
|
eq(heartbeatRuns.companyId, candidate.companyId),
|
|
),
|
|
)
|
|
.for("update");
|
|
if (
|
|
!run ||
|
|
![
|
|
coordinator?.controlDeadlineAt,
|
|
run.executionControlDeadlineAt,
|
|
].some((deadline) => deadline && deadline <= now)
|
|
)
|
|
return false;
|
|
await tx
|
|
.update(heartbeatRuns)
|
|
.set({ executionControlDeadlineAt: null })
|
|
.where(eq(heartbeatRuns.id, run.id));
|
|
if (
|
|
[
|
|
"succeeded",
|
|
"failed",
|
|
"cancelled",
|
|
"timed_out",
|
|
"interrupted",
|
|
].includes(run.status)
|
|
) {
|
|
if (coordinator?.controlDeadlineAt)
|
|
await tx
|
|
.update(nativeRunFinalizations)
|
|
.set({ controlDeadlineAt: null })
|
|
.where(eq(nativeRunFinalizations.runId, run.id));
|
|
return true;
|
|
}
|
|
// A persisted result belongs to the existing finalizer, not a replacement provider.
|
|
if (coordinator?.resultId) {
|
|
await tx
|
|
.update(nativeRunFinalizations)
|
|
.set({
|
|
controlDeadlineAt: null,
|
|
leaseOwner: null,
|
|
leaseExpiresAt: null,
|
|
updatedAt: now,
|
|
})
|
|
.where(eq(nativeRunFinalizations.runId, run.id));
|
|
return true;
|
|
}
|
|
if (await hasLiveLegacyController(tx as unknown as Db, run)) {
|
|
await tx
|
|
.update(heartbeatRuns)
|
|
.set({ executionControlDeadlineAt: run.controllerLeaseExpiresAt })
|
|
.where(eq(heartbeatRuns.id, run.id));
|
|
return false;
|
|
}
|
|
const cause = "execution_finalization_deadline_exceeded";
|
|
const nextAction =
|
|
"Inspect the failed run and verify its provider has stopped. Reconcile any uncertain external action before explicitly continuing this task.";
|
|
if (coordinator)
|
|
await tx
|
|
.update(nativeRunFinalizations)
|
|
.set({
|
|
phase: "terminal_failure",
|
|
controlDeadlineAt: null,
|
|
leaseOwner: null,
|
|
leaseExpiresAt: null,
|
|
recoveryState: "blocked",
|
|
nextAttemptAt: null,
|
|
failureCode: cause,
|
|
failureDetail: {
|
|
...coordinator.failureDetail,
|
|
nextAction,
|
|
recoveryOwner: { kind: "board" },
|
|
},
|
|
updatedAt: now,
|
|
})
|
|
.where(eq(nativeRunFinalizations.runId, run.id));
|
|
await notifyInstructionCollection(tx, { companyId: run.companyId, runId: run.id });
|
|
await notifyCancelledWorkspacePreservation(tx, { companyId: run.companyId, runId: run.id });
|
|
await notifyUnsafeWorkspaceExportRunChange(tx, run);
|
|
await notifyExecutionRunWork(tx);
|
|
const [updatedRun] = await tx
|
|
.update(heartbeatRuns)
|
|
.set({
|
|
status: "failed",
|
|
executionStatusDeliveryId: randomUUID(),
|
|
finishedAt: now,
|
|
...(coordinator
|
|
? { nativePhase: "terminal_failure", nativePhaseUpdatedAt: now }
|
|
: {}),
|
|
errorCode: cause,
|
|
error: nextAction,
|
|
nextAction,
|
|
updatedAt: now,
|
|
})
|
|
.where(eq(heartbeatRuns.id, run.id))
|
|
.returning();
|
|
if (updatedRun && updatedRun.status !== run.status) {
|
|
terminalRunToReport = updatedRun;
|
|
}
|
|
await tx
|
|
.update(agents)
|
|
.set({ status: "idle", updatedAt: now })
|
|
.where(
|
|
and(
|
|
eq(agents.id, run.agentId),
|
|
eq(agents.companyId, run.companyId),
|
|
eq(agents.status, "running"),
|
|
eq(agents.lifecycleState, "ready"),
|
|
sql`not exists (select 1 from ${heartbeatRuns} where ${heartbeatRuns.agentId} = ${run.agentId} and ${heartbeatRuns.status} = 'running')`,
|
|
),
|
|
);
|
|
const nativeReviewContext = readNativeReviewAssignmentContext(run.contextSnapshot);
|
|
const nativeReviewAssignment = nativeReviewContext && task
|
|
? await getNativeReviewAssignment(tx as unknown as Db, {
|
|
companyId: run.companyId,
|
|
issueId: task.id,
|
|
agentId: run.agentId,
|
|
contextSnapshot: nativeReviewContext,
|
|
})
|
|
: null;
|
|
const review = task?.status === "in_review" ? parseIssueExecutionState(task.executionState) : null;
|
|
const isCurrentReviewer = (review?.status === "pending" &&
|
|
review.currentParticipant?.type === "agent" && review.currentParticipant.agentId === run.agentId)
|
|
|| nativeReviewAssignment?.interaction.status === "pending";
|
|
if (
|
|
!task ||
|
|
(task.assigneeAgentId !== run.agentId && !isCurrentReviewer) ||
|
|
["done", "cancelled"].includes(task.status) ||
|
|
(task.executionRunId && task.executionRunId !== run.id) ||
|
|
(task.checkoutRunId && task.checkoutRunId !== run.id)
|
|
) {
|
|
if (updatedRun) await preserveNativeFailedWorkspace(tx as unknown as Db, updatedRun);
|
|
return true;
|
|
}
|
|
await tx
|
|
.update(issues)
|
|
.set({ executionRunId: null, checkoutRunId: null, updatedAt: now })
|
|
.where(eq(issues.id, task.id));
|
|
await issueRecoveryActionService(
|
|
tx as unknown as Db,
|
|
).upsertSourceScoped({
|
|
companyId: run.companyId,
|
|
sourceIssueId: task.id,
|
|
kind: "active_run_watchdog",
|
|
ownerType: "board",
|
|
ownerAgentId: null,
|
|
returnOwnerAgentId: task.assigneeAgentId,
|
|
cause,
|
|
fingerprint: `execution-control:${run.id}`,
|
|
evidence: {
|
|
runId: run.id,
|
|
...(isCurrentReviewer ? { reviewParticipantAgentId: run.agentId } : {}),
|
|
originalFailureCode: coordinator?.failureCode ?? run.errorCode,
|
|
providerOwnership: "unverified",
|
|
},
|
|
nextAction,
|
|
wakePolicy: null,
|
|
maxAttempts: 3,
|
|
supersedeOnIdentityChange: true,
|
|
});
|
|
if (updatedRun) await preserveNativeFailedWorkspace(tx as unknown as Db, updatedRun);
|
|
return true;
|
|
});
|
|
if (repaired) surfaced += 1;
|
|
if (terminalRunToReport) void reportRunFailure(db, terminalRunToReport);
|
|
} catch {
|
|
logger.warn(
|
|
{ runId: candidate.runId },
|
|
"Execution finalization reconciliation remains pending; continuing with other runs",
|
|
);
|
|
}
|
|
}
|
|
}));
|
|
return { scanned: due.length, surfaced };
|
|
}
|