Skip to content

fix(orb): review-nag and monitored-mention throttles never got the webhook-redelivery guard applied to the command rate limiter #8681

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

src/queue/processors.ts:13665 (maybeThrottleLoopOverCommand) and :13779
(maybeThrottleIntentRouting) both call hasAuditEventForDelivery to guard against a redelivered
GitHub webhook double-counting toward a rate limit. The guard was added by commit 066398eba2 with
an explicit comment: "Gate review finding: returning false here let a redelivered webhook fall
through to normal dispatch," and mirrored into intent-routing by commit f8f281b87.

Two older, structurally identical throttle functions in the same file never received the same
guard: maybeThrottleReviewNagPing (lines 13274-13429) and maybeThrottleMonitoredMentions (lines
13464-13615). Both are the functions the redelivery guard was originally generalized from, but the
backport was never done.

Consequence: a GitHub issue_comment webhook redelivery (a known, already-solved problem elsewhere
in this same file) double-counts a review-nag ping or a monitored-mention toward its throttle
threshold in these two functions specifically, which can push a legitimate pinger over threshold
early and can cause a duplicate cooldown/close comment to be posted.

Requirements

  • Add the same hasAuditEventForDelivery guard to maybeThrottleReviewNagPing and
    maybeThrottleMonitoredMentions, mirroring maybeThrottleLoopOverCommand's (or
    maybeThrottleIntentRouting's) existing implementation exactly.
  • Do not change either function's other throttle logic.

Deliverables

  • maybeThrottleReviewNagPing checks hasAuditEventForDelivery before counting a ping,
    matching the existing guard's pattern.
  • maybeThrottleMonitoredMentions has the same fix.
  • A new test fires the same issue_comment webhook twice with the same delivery ID within the
    redelivery window for each function, asserting the ping/mention count only increments once
    (currently increments twice for both).

All three Deliverables are required in the same PR.

Test Coverage Requirements

src/** is measured by codecov/patch (99%+ target, branch-counted). The new tests must exercise
the redelivery-guard branch directly for both functions, matching the existing test pattern already
used for maybeThrottleLoopOverCommand/maybeThrottleIntentRouting's redelivery guard.

Expected Outcome

All four command/mention/nag throttle functions in this file consistently guard against webhook
redelivery, closing the two remaining gaps in an otherwise-fixed bug class.

Links & Resources

  • src/queue/processors.ts:13274-13429 (maybeThrottleReviewNagPing), :13464-13615
    (maybeThrottleMonitoredMentions)
  • src/queue/processors.ts:13665,13779 (the already-correct siblings to mirror)
  • Introduced by commit 066398eba2, mirrored by commit f8f281b87

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions