fix(orb): backport the webhook-redelivery guard to two throttle functions - #8740
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…ions maybeThrottleReviewNagPing and maybeThrottleMonitoredMentions never received the hasAuditEventForDelivery redelivery guard that maybeThrottleLoopOverCommand and maybeThrottleIntentRouting carry, so a redelivered issue_comment double-counted a ping toward the throttle (and could re-post a cooldown/close). Adds the same guard to both; a redelivery is now suppressed, counted once. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8740 +/- ##
==========================================
+ Coverage 90.56% 91.19% +0.63%
==========================================
Files 96 97 +1
Lines 22490 25686 +3196
Branches 3884 5020 +1136
==========================================
+ Hits 20367 23425 +3058
- Misses 1945 1994 +49
- Partials 178 267 +89
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-26 05:25:27 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Fixes #8681
Root cause
maybeThrottleLoopOverCommandandmaybeThrottleIntentRoutingguard against a redelivered GitHub webhook double-counting toward a rate limit viahasAuditEventForDelivery(added by066398eba2, mirrored byf8f281b87). Two older, structurally identical throttle functions —maybeThrottleReviewNagPingandmaybeThrottleMonitoredMentions, the ones that guard was originally generalized from — never received the backport.So a redelivered
issue_comment(a known, already-solved problem elsewhere in this file) double-counted a review-nag ping or a monitored-mention toward its threshold, which could push a legitimate pinger over threshold early and post a duplicate cooldown/close comment.Fix
Added the same
hasAuditEventForDeliveryguard to both functions, mirroringmaybeThrottleIntentRouting's exact shape: right after thetargetKeyis computed, if a prior audit event already exists for thisdeliveryIdwithin the redelivery window, short-circuit (return true) before counting. No other throttle logic changed.Tests
reviewNag): fires the sameissue_commenttwice with onedeliveryId; assertsgithub_app.review_nag_pingis recorded once, not twice.monitoredMentions): an existing test atqueue-5.test.tsexplicitly documented the bug — it asserted 2 pings from 2 deliveries with a comment "does not dedup by deliveryId... documents CURRENT behavior." Updated it to assert 1 (the fixed behavior) and rewrote the comment. This is the monitored-mention half of the deliverable.Both tests are genuinely non-vacuous: reverting the two guards makes each fail (count 2, expected 1). The whole
queue-5suite (199 tests) stays green — no other test assumed double-counting.Validation
queue-5.test.ts, rebased onto currentmainprocessors.ts: both guard blocks' added lines and branches coverednpm run typecheckclean;oxlintclean on the changed lines;git diff --checkclean