From abc09ac722b93a0de2356856e11b526fa7ab4b7d Mon Sep 17 00:00:00 2001 From: atlas-bot Date: Tue, 6 Oct 2026 13:57:41 +0000 Subject: [PATCH] fix(paperclip): k8s plugin returns a termination receipt (REN-841) The kubernetes sandbox plugin's release and destroy hooks returned void, so no lease got a termination receipt and every cancelled pod run left its card waiting on remote_cleanup. Return state destroyed once the Sandbox CR / Job (and pod) reads 404, within 20 s; otherwise no receipt. Co-Authored-By: Claude Opus 5.5 --- ci/images/paperclip/README.md | 11 + .../ren-841-k8s-termination-receipt.patch | 384 ++++++++++++++++++ 2 files changed, 395 insertions(+) create mode 100644 ci/images/paperclip/ren-841-k8s-termination-receipt.patch diff --git a/ci/images/paperclip/README.md b/ci/images/paperclip/README.md index 85bb3b97..f33b25b7 100644 --- a/ci/images/paperclip/README.md +++ b/ci/images/paperclip/README.md @@ -41,6 +41,17 @@ stable fingerprint without the transcript makes the resumed turn fail with "Resource not found" (homelab#1111, reverted in #1113). Remote runs are unchanged. No upstream PR yet. +A cancelled or failed k8s pod run blocks its card for good (REN-841). The +owner's next comment waits on `remote_cleanup`, because the server continues +only when every remote lease carries a provider termination receipt +(`explicit-native-continuation.ts`), and the Kubernetes plugin's +`onEnvironmentReleaseLease` / `onEnvironmentDestroyLease` returned `void`. +`ren-841-k8s-termination-receipt.patch` makes both return +`{ providerLeaseId, state: "destroyed" }`, but only after the Sandbox CR or +Job (and, on destroy, the pod) reads 404, polling for up to 20 s, below the +server's 30 s plugin RPC timeout. If that check times out or fails, the hook +returns no receipt, as before. No upstream PR yet. + ## Files - `upstream.env` — pinned commit and the version string stamped into the diff --git a/ci/images/paperclip/ren-841-k8s-termination-receipt.patch b/ci/images/paperclip/ren-841-k8s-termination-receipt.patch new file mode 100644 index 00000000..3a87e886 --- /dev/null +++ b/ci/images/paperclip/ren-841-k8s-termination-receipt.patch @@ -0,0 +1,384 @@ +diff --git a/packages/plugins/sandbox-providers/kubernetes/src/lease-lifecycle.ts b/packages/plugins/sandbox-providers/kubernetes/src/lease-lifecycle.ts +index 87e18ae..2224e60 100644 +--- a/packages/plugins/sandbox-providers/kubernetes/src/lease-lifecycle.ts ++++ b/packages/plugins/sandbox-providers/kubernetes/src/lease-lifecycle.ts +@@ -21,6 +21,7 @@ import { deleteJob, findPodForJob, getJobStatus } from "./job-orchestrator.js"; + import { + deleteSandboxCr, + findPodForSandbox, ++ getSandboxCrStatus, + waitForSandboxReady, + } from "./sandbox-cr-orchestrator.js"; + +@@ -191,3 +192,60 @@ export async function destroyLeaseResources( + ); + } + } ++ ++export interface WorkloadGoneInput { ++ namespace: string; ++ /** Workload resource name (Sandbox CR name or Job name) == providerLeaseId. */ ++ name: string; ++ backend: "sandbox-cr" | "job"; ++ podName?: string | null; ++ /** Stays below the server's 30 s default plugin RPC timeout. */ ++ timeoutMs?: number; ++ pollMs?: number; ++} ++ ++/** ++ * Poll until the workload (and the named pod, if any) returns 404. Deletes use ++ * Foreground propagation, so the workload disappears only after its pod is ++ * gone. True is the evidence behind a `destroyed` termination receipt; false ++ * (still present at the deadline, or a read failed) means the caller must not ++ * claim one. Never throws: the delete itself already succeeded. ++ */ ++export async function waitForWorkloadGone( ++ clients: KubeClients, ++ input: WorkloadGoneInput, ++): Promise { ++ const deadline = Date.now() + (input.timeoutMs ?? 20_000); ++ const pollMs = input.pollMs ?? 500; ++ const gone = async (read: () => Promise): Promise => { ++ try { ++ await read(); ++ return false; ++ } catch (err) { ++ if (isKubeNotFoundError(err)) return true; ++ throw err; ++ } ++ }; ++ try { ++ return await pollUntilGone(); ++ } catch { ++ return false; ++ } ++ ++ async function pollUntilGone(): Promise { ++ for (;;) { ++ const workloadGone = await gone(() => ++ input.backend === "sandbox-cr" ++ ? getSandboxCrStatus(clients, input.namespace, input.name) ++ : getJobStatus(clients, input.namespace, input.name), ++ ); ++ const podName = input.podName; ++ const podGone = !podName || await gone(() => ++ clients.core.readNamespacedPod({ namespace: input.namespace, name: podName }), ++ ); ++ if (workloadGone && podGone) return true; ++ if (Date.now() >= deadline) return false; ++ await new Promise((resolve) => setTimeout(resolve, pollMs)); ++ } ++ } ++} +diff --git a/packages/plugins/sandbox-providers/kubernetes/src/plugin.ts b/packages/plugins/sandbox-providers/kubernetes/src/plugin.ts +index 9801638..bf5ce0a 100644 +--- a/packages/plugins/sandbox-providers/kubernetes/src/plugin.ts ++++ b/packages/plugins/sandbox-providers/kubernetes/src/plugin.ts +@@ -15,6 +15,7 @@ import type { + PluginEnvironmentSyncInParams, + PluginEnvironmentSyncOutParams, + PluginEnvironmentSyncResult, ++ PluginEnvironmentTerminationReceipt, + PluginEnvironmentValidateConfigParams, + PluginEnvironmentValidationResult, + } from "@paperclipai/plugin-sdk"; +@@ -38,7 +39,11 @@ import { + } from "./sandbox-cr-orchestrator.js"; + import { execInPod, execInPodStreaming, wrapCommandWithEnv } from "./pod-exec.js"; + import { performSyncIn, performSyncOut, type PodStreamExec } from "./file-sync.js"; +-import { checkLeaseResumable, destroyLeaseResources } from "./lease-lifecycle.js"; ++import { ++ checkLeaseResumable, ++ destroyLeaseResources, ++ waitForWorkloadGone, ++} from "./lease-lifecycle.js"; + import { + appendNetworkEgressDenyHint, + createScopedNetworkEgressPolicyOrReleaseWorkload, +@@ -578,7 +583,7 @@ const plugin = definePlugin({ + + async onEnvironmentReleaseLease( + params: PluginEnvironmentReleaseLeaseParams, +- ): Promise { ++ ): Promise { + if (!params.providerLeaseId) return; + const config = kubernetesProviderConfigSchema.parse(params.config); + const namespace = +@@ -613,11 +618,22 @@ const plugin = definePlugin({ + ?? (err as { code?: number; statusCode?: number }).statusCode; + if (code !== 404) throw err; + } ++ ++ // The server grants continuation after a stopped run only with a ++ // termination receipt; without one a cancelled run's queued comment ++ // waits on `remote_cleanup` forever (REN-841). Claim `destroyed` only ++ // once the workload is confirmed gone. ++ const gone = await waitForWorkloadGone(clients, { ++ namespace, ++ name: params.providerLeaseId, ++ backend: leaseBackend, ++ }); ++ if (gone) return { providerLeaseId: params.providerLeaseId, state: "destroyed" }; + }, + + async onEnvironmentDestroyLease( + params: PluginEnvironmentDestroyLeaseParams, +- ): Promise { ++ ): Promise { + if (!params.providerLeaseId) return; + const config = kubernetesProviderConfigSchema.parse(params.config); + const namespace = +@@ -658,6 +674,13 @@ const plugin = definePlugin({ + podName, + secretName, + }); ++ const gone = await waitForWorkloadGone(clients, { ++ namespace, ++ name: params.providerLeaseId, ++ backend: leaseBackend, ++ podName, ++ }); ++ if (gone) return { providerLeaseId: params.providerLeaseId, state: "destroyed" }; + }, + + async onEnvironmentExecute( +diff --git a/packages/plugins/sandbox-providers/kubernetes/test/unit/lease-lifecycle.test.ts b/packages/plugins/sandbox-providers/kubernetes/test/unit/lease-lifecycle.test.ts +index af0875f..d775799 100644 +--- a/packages/plugins/sandbox-providers/kubernetes/test/unit/lease-lifecycle.test.ts ++++ b/packages/plugins/sandbox-providers/kubernetes/test/unit/lease-lifecycle.test.ts +@@ -3,6 +3,7 @@ import { + checkLeaseResumable, + destroyLeaseResources, + isKubeNotFoundError, ++ waitForWorkloadGone, + } from "../../src/lease-lifecycle.js"; + + const SANDBOX_GROUP = "agents.x-k8s.io"; +@@ -346,3 +347,64 @@ describe("destroyLeaseResources", () => { + ).rejects.toThrow("forbidden"); + }); + }); ++ ++describe("waitForWorkloadGone", () => { ++ const input = { namespace: "ns", name: "pc-abc", backend: "sandbox-cr" as const, pollMs: 1 }; ++ ++ it("is true once the Sandbox CR and the pod return 404", async () => { ++ const getCr = vi.fn() ++ .mockResolvedValueOnce(readySandboxCr("pc-abc-pod")) ++ .mockRejectedValue(notFound()); ++ const readPod = vi.fn().mockRejectedValue(notFound()); ++ const clients = { custom: { getNamespacedCustomObject: getCr }, core: { readNamespacedPod: readPod } }; ++ ++ await expect( ++ waitForWorkloadGone(clients as never, { ...input, podName: "pc-abc-pod" }), ++ ).resolves.toBe(true); ++ expect(getCr).toHaveBeenCalledTimes(2); ++ expect(getCr).toHaveBeenCalledWith(expect.objectContaining({ ++ group: SANDBOX_GROUP, version: SANDBOX_VERSION, plural: SANDBOX_PLURAL, namespace: "ns", name: "pc-abc", ++ })); ++ }); ++ ++ it("is false while the workload still exists at the deadline", async () => { ++ const clients = { ++ custom: { getNamespacedCustomObject: vi.fn().mockResolvedValue(readySandboxCr()) }, ++ }; ++ ++ await expect( ++ waitForWorkloadGone(clients as never, { ...input, timeoutMs: 5 }), ++ ).resolves.toBe(false); ++ }); ++ ++ it("is false while the pod still exists even if the workload is gone", async () => { ++ const clients = { ++ custom: { getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()) }, ++ core: { readNamespacedPod: vi.fn().mockResolvedValue({ status: { phase: "Running" } }) }, ++ }; ++ ++ await expect( ++ waitForWorkloadGone(clients as never, { ...input, podName: "pc-abc-pod", timeoutMs: 5 }), ++ ).resolves.toBe(false); ++ }); ++ ++ it("checks the Job for job-backend leases", async () => { ++ const readJob = vi.fn().mockRejectedValue(notFound()); ++ const clients = { batch: { readNamespacedJobStatus: readJob } }; ++ ++ await expect( ++ waitForWorkloadGone(clients as never, { ...input, backend: "job" }), ++ ).resolves.toBe(true); ++ expect(readJob).toHaveBeenCalledWith({ namespace: "ns", name: "pc-abc" }); ++ }); ++ ++ it("is false (never throws) when a read fails with another error", async () => { ++ const clients = { ++ custom: { ++ getNamespacedCustomObject: vi.fn().mockRejectedValue(Object.assign(new Error("x"), { code: 403 })), ++ }, ++ }; ++ ++ await expect(waitForWorkloadGone(clients as never, input)).resolves.toBe(false); ++ }); ++}); +diff --git a/packages/plugins/sandbox-providers/kubernetes/test/unit/plugin-lease-lifecycle.test.ts b/packages/plugins/sandbox-providers/kubernetes/test/unit/plugin-lease-lifecycle.test.ts +index 4c23410..f4fd073 100644 +--- a/packages/plugins/sandbox-providers/kubernetes/test/unit/plugin-lease-lifecycle.test.ts ++++ b/packages/plugins/sandbox-providers/kubernetes/test/unit/plugin-lease-lifecycle.test.ts +@@ -184,12 +184,19 @@ describe("onEnvironmentDestroyLease", () => { + const deletePod = vi.fn().mockResolvedValue({}); + const deleteSecret = vi.fn().mockResolvedValue({}); + h.clients = { +- custom: { deleteNamespacedCustomObject: deleteCr }, +- core: { deleteNamespacedPod: deletePod, deleteNamespacedSecret: deleteSecret }, ++ custom: { ++ deleteNamespacedCustomObject: deleteCr, ++ getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ }, ++ core: { ++ deleteNamespacedPod: deletePod, ++ deleteNamespacedSecret: deleteSecret, ++ readNamespacedPod: vi.fn().mockRejectedValue(notFound()), ++ }, + batch: { deleteNamespacedJob: vi.fn() }, + }; + +- await plugin.definition.onEnvironmentDestroyLease!({ ++ const receipt = await plugin.definition.onEnvironmentDestroyLease!({ + driverKey: "kubernetes", + companyId: "acme", + environmentId: "env-1", +@@ -209,14 +216,47 @@ describe("onEnvironmentDestroyLease", () => { + namespace: "paperclip-acme", + name: "pc-abc-env", + }); ++ expect(receipt).toEqual({ providerLeaseId: "pc-abc", state: "destroyed" }); + }); + + it("is idempotent: resolves cleanly when every resource is already gone (404)", async () => { + h.clients = { +- custom: { deleteNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()) }, ++ custom: { ++ deleteNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ }, + core: { + deleteNamespacedPod: vi.fn().mockRejectedValue(notFound()), + deleteNamespacedSecret: vi.fn().mockRejectedValue(notFound()), ++ readNamespacedPod: vi.fn().mockRejectedValue(notFound()), ++ }, ++ batch: { deleteNamespacedJob: vi.fn() }, ++ }; ++ ++ await expect( ++ plugin.definition.onEnvironmentDestroyLease!({ ++ driverKey: "kubernetes", ++ companyId: "acme", ++ environmentId: "env-1", ++ config: CONFIG, ++ providerLeaseId: "pc-abc", ++ leaseMetadata: leaseMetadata(), ++ }), ++ ).resolves.toEqual({ providerLeaseId: "pc-abc", state: "destroyed" }); ++ }); ++ ++ it("returns no receipt when the gone check cannot read the cluster", async () => { ++ h.clients = { ++ custom: { ++ deleteNamespacedCustomObject: vi.fn().mockResolvedValue({}), ++ getNamespacedCustomObject: vi.fn().mockRejectedValue( ++ Object.assign(new Error("forbidden"), { code: 403 }), ++ ), ++ }, ++ core: { ++ deleteNamespacedPod: vi.fn().mockResolvedValue({}), ++ deleteNamespacedSecret: vi.fn().mockResolvedValue({}), ++ readNamespacedPod: vi.fn().mockRejectedValue(notFound()), + }, + batch: { deleteNamespacedJob: vi.fn() }, + }; +@@ -261,8 +301,12 @@ describe("onEnvironmentDestroyLease", () => { + core: { + deleteNamespacedPod: vi.fn().mockResolvedValue({}), + deleteNamespacedSecret: vi.fn().mockResolvedValue({}), ++ readNamespacedPod: vi.fn().mockRejectedValue(notFound()), ++ }, ++ batch: { ++ deleteNamespacedJob: deleteJob, ++ readNamespacedJobStatus: vi.fn().mockRejectedValue(notFound()), + }, +- batch: { deleteNamespacedJob: deleteJob }, + }; + + await plugin.definition.onEnvironmentDestroyLease!({ +@@ -280,3 +324,71 @@ describe("onEnvironmentDestroyLease", () => { + expect(deleteCr).not.toHaveBeenCalled(); + }); + }); ++ ++describe("onEnvironmentReleaseLease", () => { ++ it("returns a destroyed receipt once the Sandbox CR is gone (REN-841)", async () => { ++ const deleteCr = vi.fn().mockResolvedValue({}); ++ h.clients = { ++ custom: { ++ deleteNamespacedCustomObject: deleteCr, ++ getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ }, ++ }; ++ ++ const receipt = await plugin.definition.onEnvironmentReleaseLease!({ ++ driverKey: "kubernetes", ++ companyId: "acme", ++ environmentId: "env-1", ++ config: CONFIG, ++ providerLeaseId: "pc-abc", ++ leaseMetadata: leaseMetadata(), ++ }); ++ ++ expect(deleteCr).toHaveBeenCalledWith( ++ expect.objectContaining({ namespace: "paperclip-acme", name: "pc-abc" }), ++ ); ++ expect(receipt).toEqual({ providerLeaseId: "pc-abc", state: "destroyed" }); ++ }); ++ ++ it("returns a destroyed receipt when the Sandbox CR was already deleted", async () => { ++ h.clients = { ++ custom: { ++ deleteNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ getNamespacedCustomObject: vi.fn().mockRejectedValue(notFound()), ++ }, ++ }; ++ ++ await expect( ++ plugin.definition.onEnvironmentReleaseLease!({ ++ driverKey: "kubernetes", ++ companyId: "acme", ++ environmentId: "env-1", ++ config: CONFIG, ++ providerLeaseId: "pc-abc", ++ leaseMetadata: leaseMetadata(), ++ }), ++ ).resolves.toEqual({ providerLeaseId: "pc-abc", state: "destroyed" }); ++ }); ++ ++ it("returns no receipt when the gone check fails", async () => { ++ h.clients = { ++ custom: { ++ deleteNamespacedCustomObject: vi.fn().mockResolvedValue({}), ++ getNamespacedCustomObject: vi.fn().mockRejectedValue( ++ Object.assign(new Error("server error"), { code: 500 }), ++ ), ++ }, ++ }; ++ ++ await expect( ++ plugin.definition.onEnvironmentReleaseLease!({ ++ driverKey: "kubernetes", ++ companyId: "acme", ++ environmentId: "env-1", ++ config: CONFIG, ++ providerLeaseId: "pc-abc", ++ leaseMetadata: leaseMetadata(), ++ }), ++ ).resolves.toBeUndefined(); ++ }); ++}); -- 2.54.0