From 51b9dbc6654f5431f6068aa2103e6bfceb6300db Mon Sep 17 00:00:00 2001 From: mrbobbytables Date: Sun, 1 Nov 2026 01:30:10 +0000 Subject: [PATCH] fix(worker): forget failed webhook delivery claims Delete failed KV delivery claims so pull-request commands can be redelivered, while retaining successful deliveries for deduplication. Cover the retry lifecycle and make the bindings dedupe fixture use a successful event. Hive-Run: cncf/prow-github-actions#653 Hive-Plan: failed-delivery-redelivery Hive-Spec: issue-653#kv-claim-lifecycle Assisted-by: Auto via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: mrbobbytables --- __tests__/worker/worker.test.ts | 24 +++++++++++++++++++++++- __tests__/worker/wranglerConfig.test.ts | 2 ++ src/server/worker.ts | 5 +++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/__tests__/worker/worker.test.ts b/__tests__/worker/worker.test.ts index 53ae0d06..c98049af 100644 --- a/__tests__/worker/worker.test.ts +++ b/__tests__/worker/worker.test.ts @@ -20,7 +20,7 @@ interface Seen { method: string, path: string, auth: string | null, body: string // the fake GitHub: every request is logged under its own LOG key (seen:-, so two in flight at once — // the org- and repo-tier config probes — cannot lose each other's write); LOG.prow.yaml, when set, is served as // the repository's .github/prow.yaml. LOG.fail names, comma-separated, the calls that answer 500: tokens, -// repositories, pulls, labels. LOG.slow = 'patch' holds a PATCH for a second and logs PATCH-DONE as it answers. +// repositories, pulls, labels, patch. LOG.slow = 'patch' holds a PATCH for a second and logs PATCH-DONE as it answers. const githubScript = ` const log = (env, entry) => env.LOG.put('seen:' + String(Date.now()).padStart(15, '0') + '-' + crypto.randomUUID(), JSON.stringify(entry)) export default { @@ -42,6 +42,9 @@ export default { return new Response(null, { status: 204 }) } if (request.method === 'PATCH' && /\\/issues\\/[12]$/.test(url.pathname)) { + if (failing.includes('patch')) { + return fail() + } if ((await env.LOG.get('slow')) === 'patch') { await new Promise(resolve => setTimeout(resolve, 1000)) await log(env, { method: 'PATCH-DONE', path: url.pathname, auth: null, body: '' }) @@ -280,6 +283,25 @@ describe('the Worker', () => { expect(await deliveries.get('delivery:d-twice')).toBe('1') }) + it('forgets a pull request delivery whose run fails, so GitHub\'s redelivery runs it again', async () => { + await failing('patch') + const payload = closeComment() + + expect((await deliver('issue_comment', payload, 'd-close-fails')).status).toBe(202) + await eventually(async () => (await githubSaw()).some(s => s.method === 'PATCH')) + await settle() + + const deliveries = await mf.getKVNamespace('DELIVERIES', 'prow') + expect(await deliveries.get('delivery:d-close-fails')).toBeNull() + + await failing() + expect((await deliver('issue_comment', payload, 'd-close-fails')).status).toBe(202) + await eventually(async () => (await githubSaw()).filter(s => s.method === 'PATCH').length === 2) + await settle() + expect(await deliveries.get('delivery:d-close-fails')).toBe('1') + expect((await deliver('issue_comment', payload, 'd-close-fails')).status).toBe(200) + }) + it('scheduled(): one queue message per live installation, whose consumer runs the cron jobs for its unarchived repositories', async () => { const response = await mf.dispatchFetch('https://prow.test/cdn-cgi/handler/scheduled') expect(response.status).toBe(200) diff --git a/__tests__/worker/wranglerConfig.test.ts b/__tests__/worker/wranglerConfig.test.ts index e8f60fa5..fa60f59e 100644 --- a/__tests__/worker/wranglerConfig.test.ts +++ b/__tests__/worker/wranglerConfig.test.ts @@ -142,6 +142,8 @@ export default { async function deliver(id: string) { const payload = structuredClone(issueCommentEvent) as typeof issueCommentEvent & { installation?: { id: number } } payload.installation = { id: 7 } + // edited comments succeed without commands configured in wrangler.toml; failed deliveries are forgotten + payload.action = 'edited' payload.comment.body = 'no command here' const body = JSON.stringify(payload) return mf.dispatchFetch('https://prow.test/webhook', { diff --git a/src/server/worker.ts b/src/server/worker.ts index 320f8f00..5e335c93 100644 --- a/src/server/worker.ts +++ b/src/server/worker.ts @@ -58,6 +58,11 @@ export function kvDeliveryStore(kv: KVNamespace): DeliveryStore { await kv.put(key, '1', { expirationTtl: 86_400 }) return false }, + // a claim and a handled id are the same KV entry, so finishing changes nothing and failing drops the claim + async done() {}, + async forget(id) { + await kv.delete(`delivery:${id}`) + }, } }