From 12d8c3043ea3f385dec12e34dacac246d8b6a487 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:25:16 -0500 Subject: [PATCH] fix(native): fence retries when replaying an audited Stop (#18457) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Native runs keep Stop intent and retry state in durable records. > - A provider error can leave a failed retry with an earlier acknowledged Stop. > - The replay path returned the existing receipt before it closed that retry. > - This pull request applies the existing retry fence to the validated replay. > - The same Stop can settle without another provider cancellation or a new receipt. ## Linked Issues or Issue Description Refs #14733 for the related caller-bound Stop contract. This change covers a later retry that already has a Stop acknowledgement. The older CI-only #14743 is not a production delivery PR. **What happened?** A failed native run can have a retryable coordinator and an earlier audited Stop. A repeat Stop returns the old acknowledgement without moving the coordinator to `terminal_failure`. The final failed-to-cancelled transition requires that fence, so the Stop cannot settle through that path. **Expected behavior** A valid replay closes the same retry and keeps the original intent and acknowledgement. It does not send a second provider cancellation. Physical stop and workspace cleanup still require their own evidence. **Steps to reproduce** 1. Store a failed native run with a `retryable_failure` coordinator. 2. Store a matching run-scoped Stop with a dispatched acknowledgement and audit IDs. 3. Call `cancelNativeSession` for that same run and inspect the coordinator. **Paperclip version or commit** Reproduced on public master `f2db909393de124caf6a5380a52d4e0e1d621661`. **Deployment mode** Core native execution. The regression uses the real cancellation executor with controlled persistence and provider boundaries. ## What Changed - Reuse the existing guarded coordinator fence for a validated Stop replay. - Test repeated replay, unchanged receipts, one fence, and no second provider cancellation. - Reject foreign company, run, issue, scope, effect, and missing-audit receipts before any write. - Check that an acknowledged Stop with an unconfirmed remote lease still fails the physical-stop guard. - Document the separate retry fence and physical-cleanup requirements. ## Verification - RED on the selected public base: the new executor case returned success but left the coordinator in `retryable_failure` with a future retry. One case executed and failed for that assertion. - GREEN: 62 focused tests passed across `native-session-executor.test.ts` and `native-cancellation-request.test.ts`. These include ownership, caller conflict, changed-phase, and receipt controls. - Command from `server/`: `node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-session-executor.test.ts src/services/native-runtime/native-cancellation-request.test.ts -t 'native session cancellation|caller-bound native cancellation|failed native retry caller admission'`. - Canonical Linux CI passed all 1,018 executor cases and all 32 caller-cancellation cases. The exact file completion lines are in [server shard 2](https://github.com/paperclipai/paperclip/actions/runs/41203989386/job/124545366271) and [server shard 10](https://github.com/paperclipai/paperclip/actions/runs/41203989386/job/124545366219). - Node 24.22.2 and Vitest 5.0.3. Map contract checks and `git diff --check` pass. - Local dependencies are linked from an existing checkout. Full local TypeScript validation reports older Runner/shared declarations that lack current Kimi and continuity APIs. The changed ranges have no diagnostics. The canonical clean CI Typecheck + Release Registry gate passed. - A broader two-file run was stopped when an unchanged integration case tried to start embedded Postgres and hit the host listen restriction. It is not a passing full-suite result. No host settings or other services were changed. - Repository-wide `pnpm -r typecheck`, `pnpm test:run`, and `pnpm build` were not completed locally. Current-head CI passed against clean source and dependencies. - Fresh [Greptile review](https://github.com/paperclipai/paperclip/pull/18457#issuecomment-6543772487) on `dbc1c1a5b71450194d568725592969660864cb5a`: 5/5 with no actionable findings. The review-thread inventory has no threads and no pagination. Full [current-head CI](https://github.com/paperclipai/paperclip/actions/runs/41203989386) passed: 59 terminal checks, 55 successful, 3 skipped, and 1 neutral. No failure or pending check remains. Merge status is CLEAN. No live recovery, rollout, or physical termination is claimed. ## Risks - A wrong or stale Stop must not close another retry. Existing run/coordinator locks, caller checks, receipt binding, and the final compare-and-set remain in place. - This fix changes logical retry settlement only. Provider termination, workspace preservation, lease cleanup, and continuation gates remain separate. - The regression controls persistence and provider boundaries. It does not establish the physical state of a remote provider. ## Model Used - OpenAI GPT-6 through Codex, with reasoning, code editing, and tool use. The session does not expose an exact model build ID or context-window capacity. ## 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 references) - [x] My branch name describes the change 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 --- doc/execution-semantics.md | 4 ++ .../native-session-executor.test.ts | 55 +++++++++++++++++++ .../native-runtime/native-session-executor.ts | 44 +++++++++------ 3 files changed, 85 insertions(+), 18 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 9409d0d608e..17ee6c4f445 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -330,6 +330,10 @@ persisted cancellations of parked retries through the normal guarded cancellatio path if the original owner stops or its final database write fails. They preserve the original cancellation request and never start a provider turn for that work. +Replaying an audited native Stop also fences a later same-run retry. It preserves +the original intent and acknowledgement. That retry fence does not prove that +the provider stopped or that workspace cleanup finished. + Recovery instructions in a wake describe only an active or escalated recovery action. Resolving or cancelling that action removes its repair instructions from subsequent wakes, so the original worker can resume the task. This prompt change diff --git a/server/src/services/native-runtime/native-session-executor.test.ts b/server/src/services/native-runtime/native-session-executor.test.ts index fea3271f66d..59d7a94e408 100644 --- a/server/src/services/native-runtime/native-session-executor.test.ts +++ b/server/src/services/native-runtime/native-session-executor.test.ts @@ -99,6 +99,7 @@ import { buildNativeHeartbeatPreparationSpans, createNativeRunTrace } from "./na import { NativeRunnerOwnershipUnverifiedError } from "./native-runner-ownership.js"; import type { AdapterRuntimeEvent } from "../../adapters/index.js"; import type { AdapterUsageCheckpoint } from "@paperclipai/adapter-utils"; +import { acknowledgedNativeStopExecutionHasStopped, isAcknowledgedNativeStop } from "../acknowledged-native-stop.js"; vi.mock("node:fs", async (importOriginal) => { const actual = await importOriginal(); @@ -7852,6 +7853,27 @@ describe("native session cancellation", () => { expect(state.persistActivity).not.toHaveBeenCalled(); }); + it.each([ + { companyId: "another-company" }, { runId: "another-run" }, { issueId: "another-issue" }, + { scope: "issue" }, { effects: ["cancel_issue"] }, { intentAuditId: null }, + ])("does not fence a failed retry for a foreign or stale Stop receipt %j", async changed => { + const intent = { schema: "paperclip.native-cancellation.v1", intentId: "native-cancellation:earlier-stop", + ...execution.binding, scope: "run", reasonCode: "cancellation_run_only", effects: ["release_run_resources"], + dispatchState: "acknowledged", dispatched: true, intentAuditId: "earlier-intent-audit", + acknowledgementAuditId: "earlier-ack-audit", ...changed }; + const coordinator = { runId: execution.binding.runId, assessmentId: null, phase: "retryable_failure" }; + const persistence = cancellationDb({ status: "failed", coordinator, resultJson: { nativeCancellation: intent } }); + const before = structuredClone(persistence.getResultJson()); + state.cancel.mockClear(); + state.persistActivity.mockClear(); + await expect(cancelNativeSession(execution.binding.runId, "retry Stop", { db: persistence.db, scope: "run" })).rejects.toThrow(); + expect(persistence.updates).toEqual([]); + expect(coordinator.phase).toBe("retryable_failure"); + expect(persistence.getResultJson()).toEqual(before); + expect(state.cancel).not.toHaveBeenCalled(); + expect(state.persistActivity).not.toHaveBeenCalled(); + }); + it("does not mistake a recovered running attempt for the failed retry that reserved Stop", async () => { const id = "11111111-1111-4111-8111-111111111111"; const persistence = cancellationDb({ status: "running", @@ -7877,6 +7899,39 @@ describe("native session cancellation", () => { expect(persistence.getResultJsonUpdateCount()).toBe(writes); }); + it("fences a failed retry whose earlier Stop is already acknowledged without changing its receipts", async () => { + const intent = { schema: "paperclip.native-cancellation.v1", intentId: "native-cancellation:earlier-stop", + ...execution.binding, scope: "run", reasonCode: "cancellation_run_only", + effects: ["release_run_resources"], dispatchState: "acknowledged", dispatched: true, + intentAuditId: "earlier-intent-audit", acknowledgementAuditId: "earlier-ack-audit", + acknowledgedAt: "2026-01-01T00:00:00.000Z", decisionId: null, priorCoordinatorDecisionId: null }; + const coordinator = { runId: execution.binding.runId, assessmentId: null, phase: "retryable_failure", + nextAttemptAt: new Date(Date.now() + 30_000) }; + const persistence = cancellationDb({ status: "failed", coordinator, + resultJson: { nativeCancellation: intent, executionCancellation: { state: "requested", requestedAt: "later-stop" } } }); + const before = structuredClone(persistence.getResultJson()); + state.cancel.mockClear(); + state.persistActivity.mockClear(); + for (let attempt = 0; attempt < 2; attempt++) { + await expect(cancelNativeSession(execution.binding.runId, "retry Stop", { db: persistence.db, scope: "run" })) + .resolves.toMatchObject({ dispatched: true, auditId: intent.intentAuditId }); + expect(coordinator).toMatchObject({ phase: "terminal_failure", failureCode: "native_retry_cancelled", nextAttemptAt: null }); + expect(persistence.getResultJson()).toEqual(before); + } + expect(persistence.updates.filter(update => update.table === nativeRunFinalizations)).toHaveLength(1); + expect(persistence.getResultJsonUpdateCount()).toBe(0); + expect(state.cancel).not.toHaveBeenCalled(); + expect(state.persistActivity).not.toHaveBeenCalled(); + const stopped = { id: execution.binding.runId, companyId: execution.binding.companyId, + nativeIssueId: execution.binding.issueId, status: "cancelled", processPid: null, processGroupId: null, + resultJson: { ...persistence.getResultJson(), cancelledByActorType: "user", cancelledByUserId: "operator" } }; + const lease = { provider: "daytona", providerLeaseId: "unconfirmed-provider-resource", status: "active", + releasedAt: null, cleanupStatus: null, metadata: {} }; + const leaseDb = { select: () => ({ from: () => ({ where: async () => [lease] }) }) } as unknown as Db; + expect(isAcknowledgedNativeStop(stopped)).toBe(true); + await expect(acknowledgedNativeStopExecutionHasStopped(leaseDb, stopped)).resolves.toBe(false); + }); + it("uses the reserved caller UUID and permits same-intent and default retries", async () => { const id = "11111111-1111-4111-8111-111111111111"; const persistence = cancellationDb({ resultJson: { startupCancellation: { cancellationRequestId: id } } }); diff --git a/server/src/services/native-runtime/native-session-executor.ts b/server/src/services/native-runtime/native-session-executor.ts index 604bff67972..37abb1b88be 100644 --- a/server/src/services/native-runtime/native-session-executor.ts +++ b/server/src/services/native-runtime/native-session-executor.ts @@ -7541,6 +7541,27 @@ export async function cancelNativeSession( runId, companyId: cancellationContext.companyId, issueId: cancellationContext.issueId, resultJson, requestId: requestedId, scope: options.scope ?? "run", }, coordinator)) throw conflict("Native retry is no longer cancellable"); + const fenceRetryCancellation = async () => { + if (coordinator.phase !== "retryable_failure") return; + await tx + .update(nativeRunFinalizations) + .set({ + phase: "terminal_failure", + nextAttemptAt: null, + leaseOwner: null, + leaseExpiresAt: null, + recoveryState: "blocked", + failureCode: "native_retry_cancelled", + updatedAt: new Date(), + }) + .where( + and( + eq(nativeRunFinalizations.runId, runId), + eq(nativeRunFinalizations.companyId, cancellationContext.companyId), + eq(nativeRunFinalizations.phase, "retryable_failure"), + ), + ); + }; const existing = record(resultJson.nativeCancellation); const existingIntentId = typeof existing.intentId === "string" && existing.intentId.length > 0 @@ -7564,6 +7585,10 @@ export async function cancelNativeSession( : null; if (!existingAuditId) throw new Error("native_cancellation_intent_audit_missing"); + if (retryCancellation && coordinator.phase === "retryable_failure") { + await notifyNativeResultCommit(tx, { companyId: cancellationContext.companyId, runId }); + await fenceRetryCancellation(); + } return { intentId: existingIntentId, auditId: existingAuditId, @@ -7643,24 +7668,7 @@ export async function cancelNativeSession( // Cancellation is also an authority fence for a durable retry whose // preceding provider has already failed. No in-memory session is needed. await notifyNativeResultCommit(tx, { companyId: cancellationContext.companyId, runId }); - await tx - .update(nativeRunFinalizations) - .set({ - phase: "terminal_failure", - nextAttemptAt: null, - leaseOwner: null, - leaseExpiresAt: null, - recoveryState: "blocked", - failureCode: "native_retry_cancelled", - updatedAt: new Date(), - }) - .where( - and( - eq(nativeRunFinalizations.runId, runId), - eq(nativeRunFinalizations.companyId, cancellationContext.companyId), - eq(nativeRunFinalizations.phase, "retryable_failure"), - ), - ); + await fenceRetryCancellation(); intentPublication = activity.publication; return { intentId,