From 2d28ce6bc8450599d98721457e9feb7ead317156 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sat, 5 Sep 2026 09:37:01 -0700 Subject: [PATCH] fix: delete skill-card blob when attach fails completeSkillCardJob stored the generated card before attach. A lease mismatch or missing version left an unreferenced Convex blob. Delete the stored id if attach throws. Signed-off-by: Sebastien Tardif --- convex/skillCards.test.ts | 55 +++++++++++++++++++++++++++++++++++++++ convex/skillCards.ts | 29 ++++++++++++--------- 2 files changed, 72 insertions(+), 12 deletions(-) diff --git a/convex/skillCards.test.ts b/convex/skillCards.test.ts index e05defd5d3..af2bf7de43 100644 --- a/convex/skillCards.test.ts +++ b/convex/skillCards.test.ts @@ -738,6 +738,61 @@ describe("skillCards attach", () => { process.env.SECURITY_SCAN_WORKER_TOKEN = previousToken; }); + it("deletes the stored card blob when attach fails", async () => { + const previousToken = process.env.SECURITY_SCAN_WORKER_TOKEN; + process.env.SECURITY_SCAN_WORKER_TOKEN = "test-worker-token"; + const store = vi.fn(async () => "_storage:new-card"); + const deleteStorage = vi.fn(async () => undefined); + const runMutation = vi.fn(async () => { + throw new Error("Lease mismatch"); + }); + + await expect( + completeHandler( + { + storage: { store, delete: deleteStorage }, + runMutation, + }, + { + token: "test-worker-token", + jobId: "skillCardGenerationJobs:1", + leaseToken: "stale-lease", + markdown: "# Card\n", + }, + ), + ).rejects.toThrow(/Lease mismatch/); + + expect(store).toHaveBeenCalledOnce(); + expect(deleteStorage).toHaveBeenCalledWith("_storage:new-card"); + process.env.SECURITY_SCAN_WORKER_TOKEN = previousToken; + }); + + it("keeps the stored card blob when attach succeeds", async () => { + const previousToken = process.env.SECURITY_SCAN_WORKER_TOKEN; + process.env.SECURITY_SCAN_WORKER_TOKEN = "test-worker-token"; + const store = vi.fn(async () => "_storage:new-card"); + const deleteStorage = vi.fn(async () => undefined); + const runMutation = vi.fn(async () => ({ ok: true, bundleFingerprint: "fp" })); + + await expect( + completeHandler( + { + storage: { store, delete: deleteStorage }, + runMutation, + }, + { + token: "test-worker-token", + jobId: "skillCardGenerationJobs:1", + leaseToken: "lease", + markdown: "# Card\n", + }, + ), + ).resolves.toEqual({ ok: true, bundleFingerprint: "fp" }); + + expect(deleteStorage).not.toHaveBeenCalled(); + process.env.SECURITY_SCAN_WORKER_TOKEN = previousToken; + }); + it("replaces skill-card.md, preserves source and prior bundle fingerprints, and inserts current bundle fingerprint", async () => { const version = makeSettledVersion({ files: [ diff --git a/convex/skillCards.ts b/convex/skillCards.ts index 59a732710c..dbcea7f269 100644 --- a/convex/skillCards.ts +++ b/convex/skillCards.ts @@ -529,18 +529,23 @@ export const completeSkillCardJob = action({ const storageId = await ctx.storage.store( new Blob([args.markdown], { type: "text/markdown; charset=utf-8" }), ); - return await runMutationRef(ctx, internalRefs.skillCards.attachCardAndSucceedJobInternal, { - jobId: args.jobId, - leaseToken: args.leaseToken, - runId: args.runId, - cardFile: { - path: SKILL_CARD_FILE_PATH, - size: encoded.byteLength, - storageId, - sha256, - contentType: "text/markdown; charset=utf-8", - }, - }); + try { + return await runMutationRef(ctx, internalRefs.skillCards.attachCardAndSucceedJobInternal, { + jobId: args.jobId, + leaseToken: args.leaseToken, + runId: args.runId, + cardFile: { + path: SKILL_CARD_FILE_PATH, + size: encoded.byteLength, + storageId, + sha256, + contentType: "text/markdown; charset=utf-8", + }, + }); + } catch (error) { + await ctx.storage.delete(storageId).catch(() => undefined); + throw error; + } }, });