fix(ops): scope the PagerDuty cooldown to rows that actually paged - #9810
Conversation
The cooldown counted every `external_notification.pagerduty` row for the dedupKey via `countRecentAuditEventsForActorAndTarget`, which filters neither `outcome` nor `detail`. But that eventType is written for every path: a real page (completed/triggered), a suppression (denied/cooldown_active), a failed page (error), and an auto-resolve (completed/resolved). So a single page self-renewed the window every cron tick (its own denied row kept the count > 0), a non-page silenced a real one, a failed page blocked its own retry, and an auto-resolve suppressed the re-page for a flapping condition. Add `countRecentAuditEventsForActorTargetAndOutcome`, shaped like its sibling with `outcome` + `detail` equality terms, and count only completed/triggered rows for both the loopover and legacy gittensory actors. Export `PAGERDUTY_AUDIT_DETAIL_TRIGGERED`/`_RESOLVED` and use them at both the write and read sites so the two spellings can never drift. The denied/error/resolved rows are still written (the operator's evidence); they are simply no longer counted. Closes JSONbored#9695
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 11:02:12 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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. Decision record
🟩 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.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9810 +/- ##
==========================================
+ Coverage 77.18% 77.84% +0.65%
==========================================
Files 283 285 +2
Lines 59729 61784 +2055
Branches 6696 7377 +681
==========================================
+ Hits 46104 48097 +1993
- Misses 13342 13351 +9
- Partials 283 336 +53
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Held for manual review: the gate and required CI are green, but GitHub reports this pull request's mergeable state as |
What & why
The PagerDuty cooldown suppresses a repeat page for the same
dedupKeywithin the window. It counted recent rows viacountRecentAuditEventsForActorAndTarget, which filters neitheroutcomenordetail. ButauditPagerDutyNotificationwrites the sameexternal_notification.pagerdutyeventType +targetKeyfor every path:completed/triggereddenied/cooldown_activeerrorcompleted/resolvedSo the count was
> 0for causes that never paged anyone. Concretely, at the default 60-minute cooldown with a sub-hourly cron:deniedrow suppresses a later genuine anomaly.The fix
countRecentAuditEventsForActorTargetAndOutcometosrc/db/repositories.ts, shaped exactly likecountRecentAuditEventsForActorAndTargetwith additionaleq(outcome)+eq(detail)terms.triggerPagerDutyIncidentcounts onlyoutcome: "completed",detail: "triggered"rows, for both theloopoverand legacygittensoryactors (the two-actor union is preserved).PAGERDUTY_AUDIT_DETAIL_TRIGGERED/PAGERDUTY_AUDIT_DETAIL_RESOLVEDand use them at both the write (auditPagerDutyNotificationcalls) and the read (cooldown count) sites, so the two spellings can never drift.denied/error/resolvedrows are still written — they are the operator's evidence that suppression/failure/resolution happened; they are simply no longer counted. No change to the cooldown-minutes or min-severity resolution.Tests (
test/unit/notify-pagerduty.test.ts)Three new regression cases, each failing on
main: a recentdenied/cooldown_activerow, acompleted/resolvedrow, and anerrorrow each do not suppress the next page (callslength 1). The existing "a real triggered page suppresses" and two-actor cases still pass.Validation
npm run typecheckgreen; the pagerduty suite (38 tests) green.git diff --check <base> HEADclean; no route/schema/migration change (a DB counter + service logic).Closes #9695