refactor(selfhost): remove or wire the dead checkAndMarkDelivery helper#2572
Conversation
checkAndMarkDelivery (the documented webhook-delivery-dedup helper in redis-cache.ts) was exported but never called anywhere in src/. The real webhook dedup path in server.ts implements its own get-then-set inline, duplicating the same logic with a deliberate and CORRECT mark-after-success ordering difference: it marks a delivery ID as seen only once the response is confirmed ok, so a failed/rejected webhook stays retryable. checkAndMarkDelivery marked on the FIRST call regardless of outcome, which would have incorrectly suppressed a retry of a webhook whose processing actually failed, had anything called it. Delete it -- the inline version in server.ts is already correct and live, and two divergent implementations of the same dedup logic can silently drift out of sync.
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ Gittensory review result - approve/merge recommendedReview updated: 2026-07-02 09:20:44 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2572 +/- ##
==========================================
+ Coverage 95.95% 95.98% +0.02%
==========================================
Files 226 229 +3
Lines 25425 25807 +382
Branches 9244 9390 +146
==========================================
+ Hits 24397 24771 +374
- Misses 417 425 +8
Partials 611 611
🚀 New features to boost your workflow:
|
Summary
checkAndMarkDelivery(the documented webhook-delivery-dedup helper insrc/selfhost/redis-cache.ts) was exported but never called anywhere insrc/. The real webhook dedup path insrc/server.tsimplements its own get-then-set inline instead, duplicating the same logic with a deliberate and correct mark-after-success ordering difference: it marks a delivery ID as seen only once the response is confirmedok, so a failed/rejected webhook stays retryable.checkAndMarkDeliverymarked as seen on the FIRST call regardless of outcome — a real (if unreachable, since it had zero call sites) bug: had anything called it, it would have incorrectly suppressed GitHub's retry of a webhook whose processing actually failed.server.tsto call it: the inline version is already correct and live, and fixingcheckAndMarkDelivery's mark-timing to match it first (per the issue's option 2) would be strictly more code for no behavior change, when option 1 is the smaller, safer change.test/unit/selfhost-redis-cache.test.ts); the still-livecreateRedisCachecoverage (get/set/del/claim) is untouched and stays at 100%.Closes #2506.
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. (A pure deletion —createRedisCachestays at 100%.)npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateSafety
UI Evidencesection below — N/A, no UI changes.Notes