Skip to content

fix(ops): scope the PagerDuty cooldown to rows that actually paged - #9810

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/pagerduty-cooldown-scope-9695
Jul 29, 2026
Merged

fix(ops): scope the PagerDuty cooldown to rows that actually paged#9810
JSONbored merged 1 commit into
JSONbored:mainfrom
shin-core:fix/pagerduty-cooldown-scope-9695

Conversation

@shin-core

Copy link
Copy Markdown
Contributor

What & why

The PagerDuty cooldown suppresses a repeat page for the same dedupKey within the window. It counted recent rows via countRecentAuditEventsForActorAndTarget, which filters neither outcome nor detail. But auditPagerDutyNotification writes the same external_notification.pagerduty eventType + targetKey for every path:

  • a real page — completed / triggered
  • a suppression — denied / cooldown_active
  • a failed page — error
  • an auto-resolve — completed / resolved

So the count was > 0 for causes that never paged anyone. Concretely, at the default 60-minute cooldown with a sub-hourly cron:

  1. Self-renewing cooldown — a page writes a row; the next tick suppresses and writes another row; the window slides forward forever and the repo never pages again.
  2. A non-page silences a real page — a denied row suppresses a later genuine anomaly.
  3. A failed page blocks its own retry.
  4. An auto-resolve suppresses the re-page for a flapping condition.

The fix

  • Add countRecentAuditEventsForActorTargetAndOutcome to src/db/repositories.ts, shaped exactly like countRecentAuditEventsForActorAndTarget with additional eq(outcome) + eq(detail) terms.
  • triggerPagerDutyIncident counts only outcome: "completed", detail: "triggered" rows, for both the loopover and legacy gittensory actors (the two-actor union is preserved).
  • Export PAGERDUTY_AUDIT_DETAIL_TRIGGERED / PAGERDUTY_AUDIT_DETAIL_RESOLVED and use them at both the write (auditPagerDutyNotification calls) and the read (cooldown count) sites, so the two spellings can never drift.
  • The denied / error / resolved rows 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 recent denied/cooldown_active row, a completed/resolved row, and an error row each do not suppress the next page (calls length 1). The existing "a real triggered page suppresses" and two-actor cases still pass.

Validation

  • npm run typecheck green; the pagerduty suite (38 tests) green.
  • Diff coverage on both changed src files is 100% line and branch.
  • git diff --check <base> HEAD clean; no route/schema/migration change (a DB counter + service logic).

Closes #9695

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
@shin-core
shin-core requested a review from JSONbored as a code owner July 29, 2026 10:36
@superagent-security superagent-security Bot added the contributor:flagged Contributor flagged for review by trust analysis. label Jul 29, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

🚨 Contributor flagged. Click here for more info: Superagent Dashboard

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Warning

⏸️ LoopOver review result - manual review recommended

Review updated: 2026-07-29 11:02:12 UTC

3 files · 1 AI reviewer · no blockers · CI green · unstable

⏸️ Suggested Action - Manual Review

Review summary
This fixes a real bug: the PagerDuty cooldown counter previously matched any row sharing eventType+targetKey regardless of outcome/detail, so a suppression, error, or auto-resolve row could count as a 'recent page' and either self-renew the cooldown forever or block a genuine retry. The fix adds a properly outcome/detail-scoped counter, uses shared exported constants at both write and read sites to prevent spelling drift, and preserves the dual-actor (loopover/gittensory) union. The three new regression tests correctly reproduce the exact failure modes described (denied, resolved, error rows no longer suppressing), and each test seeds real data via recordAuditEvent and calls the real trigger path, so they are not fabricated — they exercise the actual counting logic that shipped.

Nits — 4 non-blocking
  • src/db/repositories.ts:3165 references issue orb(ops): scope the PagerDuty cooldown to rows that actually paged #9695 in a comment but the number itself isn't otherwise used; consider a short named constant only if this magic number recurs elsewhere.
  • The new function largely duplicates countRecentAuditEventsForActorAndTarget's query shape (same and/eq/gte pattern with two extra eq() terms) — consider whether the original could take optional outcome/detail params instead of adding a sibling function, though the current approach keeps call sites explicit.
  • Consider whether countRecentAuditEventsForActorAndTarget still has any other callers relying on the old unscoped behavior, since a similarly-shaped audit-count bug could exist elsewhere in the codebase.
  • The PR description's truncated final line ('a recent suppression (denied/cooldown_active) row does NOT...') matches the diff exactly, so no scope concerns there.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9695
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ❌ 5/25 Preflight is holding this PR: the review lane is unavailable, so it is not ready for automated review.
Contributor workload ✅ 10/10 Author activity: 63 registered-repo PR(s), 46 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor shin-core; Gittensor profile; 63 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Partially addressed
The PR correctly filters the cooldown counter to real triggers (outcome=completed AND detail='triggered'), fixing the four failure modes and adding regression tests for the denied/error/resolved false-suppression cases, but it deviates from the required function signature (adds a detail param not in the issue's spec) and the diff shows no dedicated unit test for countRecentAuditEventsForActorTarge

Review context
  • Author: shin-core
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is registered but has no active allocation in the current snapshot.
  • Public profile languages: not available
  • Official Gittensor activity: 63 PR(s), 0 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Start here: Await review-lane availability.
  • Then work through the remaining 1 step in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

Decision record
  • action: hold · clause: success
  • config: cd2425c9e93c4e37e51cb40d35189e84889ccde8579397aeeae5567e1a5e399a · pack: oss-anti-slop · ci: passed
  • record: fbfe0cc87e444f954bcd9135421b0e29457c7c9d001a8a249e577dc49d6e2e2b (schema v5, head 42506b8)

🟩 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.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 29, 2026
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.84%. Comparing base (afb3b84) to head (42506b8).
⚠️ Report is 3 commits behind head on main.

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     
Flag Coverage Δ
backend 96.98% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/db/repositories.ts 96.86% <100.00%> (ø)
src/services/notify-pagerduty.ts 100.00% <100.00%> (ø)

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 29, 2026
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Held for manual review: the gate and required CI are green, but GitHub reports this pull request's mergeable state as unstable because a non-required check or status is not passing, so LoopOver will not auto-merge. A maintainer can resolve the failing check or review and merge manually. This is an automated maintenance action.

@JSONbored
JSONbored merged commit 9b228f3 into JSONbored:main Jul 29, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:flagged Contributor flagged for review by trust analysis. gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(ops): scope the PagerDuty cooldown to rows that actually paged

2 participants