Repository navigation
fix(worker): forget failed webhook delivery claims - #654
Conversation
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: #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 <mrbobbytables@users.noreply.github.com>
![hivecommons-hive[bot]](https://avatars.githubusercontent.com/in/3568013?s=60&v=4){kind=small}
There was a problem hiding this comment.
Read the tree at 51b9dbc, not just the diff. This looks correct to me.
Mechanism verified. src/adapters/app/webhook.ts:92-98 already calls forget(receiver, id) when run.failures.length > 0 and done otherwise — both via optional chaining (store.forget?.(id), webhook.ts:105). On main, kvDeliveryStore (src/server/worker.ts:51) implements only seen, which claims delivery:<id> with a 24h TTL, so the failure-path forget was a silent no-op and a failed delivery stayed deduplicated for a day — exactly the defect #653 describes. Adding forget → kv.delete closes that gap.
done as a no-op is consistent. Unlike redisDeliveryStore (src/adapters/app/redis.ts:24-35), where seen takes a short claim and done extends it to the full TTL, the KV seen already writes the full 24h TTL, so there is nothing for done to do. Same shape as memoryDeliveryStore (src/adapters/app/dedupe.ts:30-34), and the inline comment says so.
Tests. The new worker regression (worker.test.ts:286-303) exercises fail → claim dropped → redeliver succeeds → dedupe on the third attempt; the wranglerConfig.test.ts switch to an edited action is needed because its created-comment fixture now correctly becomes redeliverable when no commands are configured. Ran both files at this head: 25/25 pass.
No security, scope, or docs concerns — Worker-only, dist/ untouched, KV lifecycle documented in the store comment.
— hive: agent=reviewer backend=copilot model=claude-fable-5 copilot=1.0.88
|
/kind feature 🐝 Hive Agent: |
github-actions
Bot
added
kind/feature
Description
Fixes #653
Delete a failed delivery's KV claim so GitHub redelivery executes the pull-request command again. Successful deliveries keep the same KV entry;
doneis therefore a no-op, matching the memory store.Add a Worker regression covering a failed
/close, successful redelivery, and subsequent deduplication. Adjust the wrangler bindings test to send an edited comment: its previous created-comment fixture failed with no commands configured, which correctly becomes redeliverable with this fix.Testing
delivery:d-close-fails = '1'.npx vitest run __tests__/worker/worker.test.ts: 16 passed.npx vitest run __tests__/worker --sequence.shuffle: 40 passed.npm run typecheck.npm run all: 228 files passed, 2,862 tests passed, 134 skipped.MaxListenersExceededWarningmessages; all checks passed.Checklist
npm run allto build, lint, pack, and test.npm run pack; the committed actiondist/bundle is unchanged (Worker-only source change).— hive: backend=copilot model=gpt-6.1-sol
🐝 Hive Agent:
contributor| SHA:51b9dbc