mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(execution): keep resumed native runs alive past a stale finalization deadline (#20920)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The execution control reconciler bounds how long a run may spend
settling after its provider work ends
> - A run deadline from one attempt stayed on the run row after the run
was handed to a same-run resume or a restart claim
> - The reconciler then failed the resumed, live run with
`execution_finalization_deadline_exceeded` and told a person to inspect
the provider
> - This breaks two product rules: recovery must work without a person,
and users must not get operator instructions
> - This pull request makes a live native owner or a scheduled same-run
resume the owner of the run, and it replaces the operator text with
plain wording and Retry
> - The benefit is that runs that recover after a restart keep running,
and a genuine expiry shows plain words and one working Retry
## Linked Issues or Issue Description
No public issue exists. This is the bug description, in the fields of
`.github/ISSUE_TEMPLATE/bug_report.yml`.
**What happened?**
On a hosted deploy, 11 tasks became blocked with
`execution_finalization_deadline_exceeded` in two hours. Three runs had
the same sequence. The native session failed or the server restarted. A
same-run resume or a restart claim then took the run. The new controller
held a live native lease and prepared the run again. The reconciler then
found the earlier attempt's run deadline and failed the run. The
recorded original failure code was empty, so no real failure caused the
block. The run error told a person to inspect the failed run and verify
that its provider stopped.
**Expected behavior**
A run that a live controller owns, or that has a scheduled same-run
resume, continues. A deadline that really expires shows plain wording
and a working Retry. Users do not get operator instructions.
**Steps to reproduce**
1. Save a running native run with an `executionControlDeadlineAt` in the
past.
2. Give its finalization row a live lease (a restart claim), or phase
`retryable_failure` with no lease (a scheduled resume).
3. Run `reconcileAbandonedExecutionControl`.
4. The run fails with `execution_finalization_deadline_exceeded`, and
its error tells a person to inspect the provider.
**Paperclip version or commit**
`master` at 6154b9485e.
**Deployment mode**
Hosted (cloud) instance with Daytona remote sandboxes.
**Agent adapter(s) involved**
Not adapter-specific (core bug). Seen with Codex and OpenCode native
runs.
## What Changed
- `execution-control-reconciliation.ts`: when a running native run has
no accepted result and its finalization owner holds a live lease or has
a scheduled same-run resume, the reconciler clears the stale run
deadline and does not fail the run.
- `execution-control-reconciliation.ts`: when the live owner's own
settlement deadline is due, the reconciler moves the deadline to the
lease expiry. It surfaces the failure only after that lease lapses.
- `execution-control-deadline.ts`: adds the cause constant and a plain
message. The reconciler writes this message to the run error, the run
next action, and the recovery action.
- `execution-blocker.ts`: a run saved by an earlier release with this
error code, and an active recovery action with this cause, show the
plain message. Retry stays available. No migration is needed.
- `doc/execution-semantics.md`: documents that a control deadline bounds
one attempt and not a later attempt of the same run.
## Verification
- Red before the fix (5 failures): `npx vitest run
src/services/execution-control-reconciliation.test.ts
src/services/execution-blocker-read.test.ts` in `server/`. The
resumed-run cases failed with `expected 1 to be +0` (the run was
surfaced as failed). The wording cases received the old operator text.
- Green after the fix: the same command passes, 46 of 46.
- Related files pass: `execution-control-work`, `task-retry`,
`workspace-recovery-warm-handoff`, `native-safe-replacement`,
`closed-native-receipt-accounting`, `execution-recovery-resolution`,
`workspace-recovery-admission`, `execution-projection`.
- `npx tsc --noEmit -p server/tsconfig.json` passes.
- `pnpm verify` selects a broad plan because
`doc/execution-semantics.md` changed. CI supplies that coverage.
- Previous-release story: in-flight rows with a stale run deadline and a
live owner continue after deploy. Rows that an earlier release already
failed show plain wording and Retry when read. No schema change.
## Risks
- A native owner that keeps renewing its lease while its settlement
hangs is not failed by this backstop until the lease lapses. The
executor stops lease renewal before it starts failure settlement, so the
wait is bounded by one lease period.
- A scheduled same-run resume now owns the run. Restart recovery
reschedules these resumes at boot. A coordinator deadline that is due
still surfaces the failure.
- Tasks that an earlier release already blocked keep their block. They
now show plain wording and Retry. Chat-bound tasks keep their existing
"send a message" continuation.
## Model Used
- Claude Opus 5.5 (claude-opus-5-5), Anthropic, with tool use and code
execution in Claude Code.
## 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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
64afd22d82
commit
fc14f1ade2
6 files changed
+111
-5
No files matched your search
@@ -3561,6 +3561,8 @@ fails closed as before.
|
||||
|
||||
Provider execution and control-plane finalization have different clocks. A healthy provider can think or execute a long tool without output. Once execution settles, recovery and finalization control steps have a 60-second deadline, checked on startup and every 15 seconds. With a healthy database and scheduler, an abandoned transition must be repaired or surfaced within 90 seconds. Terminal persistence must not wait on provider cleanup or publication; a late finalizer cannot change a reassigned or closed task or release another run's locks. In particular, a done report from a run whose task now belongs to another agent, a user or nobody is kept as an accepted result but records `completion_owner_superseded` and preserves the task status for the current owner. Historical ambiguous runs are never automatically replayed after an upgrade.
|
||||
|
||||
A control deadline bounds the settlement of one attempt, never a later attempt of the same run. While a native run's finalization owner holds a live lease (for example, a controller that reclaimed the run after a server restart) or has a same-run resume scheduled, that owner is responsible for the run. The reconciler clears a run deadline left by an earlier attempt instead of failing the resumed work. When the live owner's own settlement deadline is due, the reconciler waits until that lease lapses before it surfaces the deadline. A run whose deadline genuinely expires fails with `execution_finalization_deadline_exceeded` and shows plain wording with Retry: its saved work is kept, and actions with unconfirmed outcomes are not repeated. Runs saved by earlier releases with the previous operator-facing wording show the same plain wording when the task is read; no migration is needed.
|
||||
|
||||
When that deadline fails a native run with a prepared workspace, the same
|
||||
transaction records the exact source's file-preservation obligation. Existing
|
||||
stop-and-retain recovery contains the provider and saves the files before
|
||||
|
||||
@@ -4,6 +4,7 @@ import { eq } from "drizzle-orm";
|
||||
import { afterAll, beforeAll, describe, expect, it } from "vitest";
|
||||
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "../__tests__/helpers/embedded-postgres.js";
|
||||
import { EXECUTION_BLOCKER_PERSON_RETRY_ACTION, getExecutionBlocker } from "./execution-blocker.js";
|
||||
import { EXECUTION_FINALIZATION_DEADLINE_MESSAGE } from "./execution-control-deadline.js";
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
describe.skipIf(!support.supported)("execution blocker PostgreSQL read projection", () => {
|
||||
@@ -84,6 +85,20 @@ describe.skipIf(!support.supported)("execution blocker PostgreSQL read projectio
|
||||
expect(EXECUTION_BLOCKER_PERSON_RETRY_ACTION).toContain("Agents cannot retry it for them.");
|
||||
});
|
||||
|
||||
it("shows plain wording and Retry for a finalization deadline saved by the previous release", async () => {
|
||||
const previousReleaseText = "Inspect the failed run and verify its provider has stopped. Reconcile any uncertain external action before explicitly continuing this task.";
|
||||
const f = await fixture({ runtimeMode: "native", errorCode: "execution_finalization_deadline_exceeded",
|
||||
error: previousReleaseText, nextAction: previousReleaseText });
|
||||
await db.update(issueRecoveryActions).set({ cause: "execution_finalization_deadline_exceeded", ownerType: "board",
|
||||
nextAction: previousReleaseText }).where(eq(issueRecoveryActions.sourceIssueId, f.issueId));
|
||||
|
||||
const blocker = await getExecutionBlocker(db, f.companyId, f.issueId);
|
||||
|
||||
expect(blocker).toMatchObject({ canRetry: true, runError: EXECUTION_FINALIZATION_DEADLINE_MESSAGE,
|
||||
nextAction: `${EXECUTION_FINALIZATION_DEADLINE_MESSAGE} ${EXECUTION_BLOCKER_PERSON_RETRY_ACTION}` });
|
||||
expect(JSON.stringify(blocker)).not.toMatch(/Inspect the failed run|verify its provider|Reconcile any uncertain/);
|
||||
});
|
||||
|
||||
it("retains the claimed adapter and startup receipts for a cancelled pre-provider run", async () => {
|
||||
const f = await fixture({ status: "cancelled", startedAt: null, errorCode: "operator_interrupted",
|
||||
resultJson: { startupCancellation: { beforeNativeSelection: true }, startupPreparationSettledAt: "2020-01-01T00:01:00Z" } });
|
||||
|
||||
@@ -18,6 +18,7 @@ import { completedWorkspaceRecoveryCondition, hasVerifiedWorkspaceRepair, worksp
|
||||
import { nativeManualAdmissionHoldsExecution } from "./native-runtime/native-runner-ownership.js";
|
||||
import { PROCESS_IDENTITY_RECORDED, PROCESS_START_REQUESTED } from "./native-local-process-stop.js";
|
||||
import { NATIVE_CLEANUP_SETTLING_MESSAGE, NATIVE_SESSION_CLEANUP_QUARANTINED } from "./native-runtime/completed-turn-cleanup.js";
|
||||
import { EXECUTION_FINALIZATION_DEADLINE_EXCEEDED, EXECUTION_FINALIZATION_DEADLINE_MESSAGE } from "./execution-control-deadline.js";
|
||||
|
||||
const blockerContextKeys = ["issueId", "taskId", "source", "connectionIntentResolution",
|
||||
"interactionKind", "interactionStatus", "mutation", "interactionId"] as const;
|
||||
@@ -410,6 +411,7 @@ export async function getExecutionBlocker(db: Db, companyId: string, issueId: st
|
||||
// Earlier releases saved operator-facing text for the cleanup gate. Show
|
||||
// the plain wording from the code instead of the stored message.
|
||||
: run?.errorCode === NATIVE_SESSION_CLEANUP_QUARANTINED ? NATIVE_CLEANUP_SETTLING_MESSAGE
|
||||
: run?.errorCode === EXECUTION_FINALIZATION_DEADLINE_EXCEEDED ? EXECUTION_FINALIZATION_DEADLINE_MESSAGE
|
||||
: cancellation?.reason ?? (run?.status === "cancelled"
|
||||
? "Execution was cancelled; its source was not recorded."
|
||||
// Rows written before SQL text was kept out of run errors still carry it.
|
||||
@@ -424,7 +426,8 @@ export async function getExecutionBlocker(db: Db, companyId: string, issueId: st
|
||||
)).where(and(
|
||||
eq(chatConversations.companyId, companyId), eq(chatConversations.issueId, issueId),
|
||||
)).limit(1) : [];
|
||||
let nextAction = action.nextAction;
|
||||
let nextAction = action.status === "active" && action.cause === EXECUTION_FINALIZATION_DEADLINE_EXCEEDED
|
||||
? EXECUTION_FINALIZATION_DEADLINE_MESSAGE : action.nextAction;
|
||||
// Retry is a person's control on this task. Say so, so an agent reading this
|
||||
// record points the person to it instead of re-running the work itself.
|
||||
if (canRetry && !eligibleContinuation) nextAction += ` ${EXECUTION_BLOCKER_PERSON_RETRY_ACTION}`;
|
||||
|
||||
@@ -20,3 +20,6 @@ export async function boundedExecutionCleanup(
|
||||
if (timer) clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
export const EXECUTION_FINALIZATION_DEADLINE_EXCEEDED = "execution_finalization_deadline_exceeded";
|
||||
export const EXECUTION_FINALIZATION_DEADLINE_MESSAGE =
|
||||
"The run stopped before it finished saving its work. Saved work is kept, and actions with unconfirmed outcomes will not be repeated.";
|
||||
@@ -19,6 +19,7 @@ vi.mock("../sentry.js", async () => {
|
||||
});
|
||||
|
||||
import { reconcileAbandonedExecutionControl } from "./execution-control-reconciliation.js";
|
||||
import { EXECUTION_FINALIZATION_DEADLINE_MESSAGE } from "./execution-control-deadline.js";
|
||||
|
||||
const external = resolveExternalTestDatabaseUrl("PAPERCLIP_EXECUTION_CONTROL_TEST_DATABASE_URL");
|
||||
const embeddedPostgresSupport = external ? { supported: true } : await getEmbeddedPostgresTestSupport();
|
||||
@@ -224,6 +225,70 @@ describeEmbeddedPostgres("reconcileAbandonedExecutionControl reports a genuine f
|
||||
}, { timeout: 5_000 });
|
||||
});
|
||||
|
||||
async function seedResumedNativeRun(coordinator: Partial<typeof nativeRunFinalizations.$inferInsert>) {
|
||||
const f = await seedAbandonedRunFixture();
|
||||
await db.update(heartbeatRuns).set({ runtimeMode: "native", nativeIssueId: f.issueId })
|
||||
.where(eq(heartbeatRuns.id, f.runId));
|
||||
await db.insert(nativeRunFinalizations).values({ runId: f.runId, companyId: f.companyId, issueId: f.issueId,
|
||||
phase: "observed", controlDeadlineAt: null, ...coordinator });
|
||||
return f;
|
||||
}
|
||||
|
||||
it.each([
|
||||
["reclaimed after a server restart", { phase: "observed", leaseOwner: "restart-claim", controllerGeneration: 2,
|
||||
recoveryState: "awaiting_runner_reattach", leaseExpiresAt: new Date(Date.now() + 60_000) }],
|
||||
["waiting for its scheduled same-run resume", { phase: "retryable_failure", leaseOwner: null, leaseExpiresAt: null,
|
||||
recoveryState: "resuming_session", nextAttemptAt: new Date(Date.now() + 120_000) }],
|
||||
] as const)("keeps a native run %s running past the previous attempt's run deadline", async (_shape, coordinator) => {
|
||||
const f = await seedResumedNativeRun(coordinator);
|
||||
const captureCallsBefore = mockCaptureRunFailure.mock.calls.length;
|
||||
|
||||
const result = await reconcileAbandonedExecutionControl(db);
|
||||
|
||||
expect(result.surfaced).toBe(0);
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
|
||||
expect(run).toMatchObject({ status: "running", errorCode: null, error: null, executionControlDeadlineAt: null });
|
||||
const [owner] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
expect(owner).toMatchObject({ phase: coordinator.phase, leaseOwner: coordinator.leaseOwner, failureCode: null });
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||
expect(task).toMatchObject({ status: "in_progress", executionRunId: f.runId, checkoutRunId: f.runId });
|
||||
expect(await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, f.issueId))).toHaveLength(0);
|
||||
expect(mockCaptureRunFailure.mock.calls.slice(captureCallsBefore)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("waits for a live native owner to finish its own failure settlement before surfacing the deadline", async () => {
|
||||
const leaseExpiresAt = new Date(Date.now() + 30_000);
|
||||
const f = await seedResumedNativeRun({ phase: "observed", leaseOwner: "settling-controller", leaseExpiresAt,
|
||||
controlDeadlineAt: new Date(Date.now() - 1_000) });
|
||||
|
||||
expect((await reconcileAbandonedExecutionControl(db)).surfaced).toBe(0);
|
||||
const [waiting] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
|
||||
const [owner] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
expect(waiting).toMatchObject({ status: "running", errorCode: null });
|
||||
expect(owner.controlDeadlineAt?.getTime()).toBe(leaseExpiresAt.getTime());
|
||||
|
||||
const lapsed = new Date(Date.now() - 1_000);
|
||||
await db.update(nativeRunFinalizations).set({ leaseExpiresAt: lapsed, controlDeadlineAt: lapsed })
|
||||
.where(eq(nativeRunFinalizations.runId, f.runId));
|
||||
expect((await reconcileAbandonedExecutionControl(db)).surfaced).toBe(1);
|
||||
const [failed] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
|
||||
expect(failed).toMatchObject({ status: "failed", errorCode: "execution_finalization_deadline_exceeded" });
|
||||
});
|
||||
|
||||
it("explains a genuine finalization deadline in plain words without asking a person to repair the run", async () => {
|
||||
const f = await seedAbandonedRunFixture();
|
||||
|
||||
await reconcileAbandonedExecutionControl(db);
|
||||
|
||||
const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, f.runId));
|
||||
const [action] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, f.issueId));
|
||||
expect(run.status).toBe("failed");
|
||||
for (const text of [run.error, run.nextAction, action?.nextAction]) {
|
||||
expect(text).toBe(EXECUTION_FINALIZATION_DEADLINE_MESSAGE);
|
||||
expect(text).not.toMatch(/inspect|verify|reconcile|provider/i);
|
||||
}
|
||||
});
|
||||
|
||||
it("reports zero events for a repeated sweep over the same already-failed run", async () => {
|
||||
const { runId } = await seedAbandonedRunFixture();
|
||||
await reconcileAbandonedExecutionControl(db);
|
||||
|
||||
@@ -22,7 +22,8 @@ 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";
|
||||
import { EXECUTION_RECONCILIATION_INTERVAL_MS } from "./execution-control-deadline.js";
|
||||
import { EXECUTION_FINALIZATION_DEADLINE_EXCEEDED, EXECUTION_FINALIZATION_DEADLINE_MESSAGE,
|
||||
EXECUTION_RECONCILIATION_INTERVAL_MS } from "./execution-control-deadline.js";
|
||||
import { DELIVERY_QUEUES, notifyDeliveryWork } from "./delivery-work-notifications.js";
|
||||
import { NATIVE_STARTUP_CLEANUP_PENDING, readNativeStartupContext, promoteNativeStartupCleanup } from "./native-runtime/native-startup-cleanup.js";
|
||||
import { releaseAbandonedNativeOwners } from "./native-runtime/native-failed-run-owner-release.js";
|
||||
@@ -176,6 +177,24 @@ export async function reconcileAbandonedExecutionControl(
|
||||
}
|
||||
return false;
|
||||
}
|
||||
if (coordinator && !coordinator.resultId && run.status === "running") {
|
||||
const nativeOwnerLeaseAt = coordinator.leaseOwner && coordinator.leaseExpiresAt && coordinator.leaseExpiresAt > now
|
||||
? coordinator.leaseExpiresAt : null;
|
||||
const sameRunResumeScheduled = coordinator.phase === "retryable_failure" && !coordinator.leaseOwner;
|
||||
const nativeSettlementDue = Boolean(coordinator.controlDeadlineAt && coordinator.controlDeadlineAt <= now);
|
||||
if (!nativeSettlementDue && (nativeOwnerLeaseAt || sameRunResumeScheduled)) {
|
||||
await tx.update(heartbeatRuns).set({ executionControlDeadlineAt: null }).where(eq(heartbeatRuns.id, run.id));
|
||||
return false;
|
||||
}
|
||||
if (nativeOwnerLeaseAt) {
|
||||
await notifyDeliveryWork(tx, DELIVERY_QUEUES.executionFinalization);
|
||||
await tx.update(nativeRunFinalizations).set({ controlDeadlineAt: nativeOwnerLeaseAt })
|
||||
.where(eq(nativeRunFinalizations.runId, run.id));
|
||||
await tx.update(heartbeatRuns).set({ executionControlDeadlineAt: run.executionControlDeadlineAt ? nativeOwnerLeaseAt : null })
|
||||
.where(eq(heartbeatRuns.id, run.id));
|
||||
return false;
|
||||
}
|
||||
}
|
||||
await tx
|
||||
.update(heartbeatRuns)
|
||||
.set({ executionControlDeadlineAt: null })
|
||||
@@ -217,9 +236,8 @@ export async function reconcileAbandonedExecutionControl(
|
||||
.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.";
|
||||
const cause = EXECUTION_FINALIZATION_DEADLINE_EXCEEDED;
|
||||
const nextAction = EXECUTION_FINALIZATION_DEADLINE_MESSAGE;
|
||||
// The deadline replaces the run's error code. Keep the earlier
|
||||
// diagnosis as its root cause, as the native finalizer does.
|
||||
const priorFailureCode = coordinator?.failureCode ?? run.errorCode;
|
||||
|
||||
Reference in new issue
Block a user