⚠️ 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/services/notify-pagerduty.ts:19-21 states the cooldown contract: "COOLDOWN — a repeat trigger for the SAME dedup_key within PAGERDUTY_COOLDOWN_MINUTES ... is suppressed, so a still-ongoing condition re-checked every cron tick doesn't re-page every tick."
The implementation counts the wrong population — :184-192 calls countRecentAuditEventsForActorAndTarget twice (for the loopover and legacy gittensory actors) and suppresses when the sum is > 0. countRecentAuditEventsForActorAndTarget (src/db/repositories.ts:3153-3162) filters on actor + eventType + targetKey + createdAt and not on outcome. auditPagerDutyNotification (:123-140) writes that same eventType and targetKey for every path in the file: :171 denied/below_min_severity; :190 denied/cooldown_active; :218 error (the page failed; nobody was woken); :254 completed/resolved, written by resolvePagerDutyIncident, which src/review/ops-wire.ts:298 calls with the same dedupKey the trigger at :326 uses.
Four concrete failures at the default 60-minute cooldown with a sub-hourly cron:
- Self-renewing cooldown. T0 pages -> row. T+5m still anomalous -> counts the T0 row -> suppressed, and writes another row. T+10m counts the T+5m row. The window slides forward on every tick, so after the first page the repo never pages again.
- A non-page silences a real page. A
warning anomaly denied at :171 writes a row; an error anomaly 5 minutes later is suppressed as cooldown_active even though nothing was ever paged.
- A failed page blocks its own retry. The
error row at :218 suppresses the next attempt for the whole window.
- Auto-resolve suppresses the re-page. For a flapping condition the
resolved row at :254 is counted by the trigger's cooldown, so the recurrence after a clear is dropped.
The analogous cooldowns in this codebase do the opposite deliberately: src/review/loop-escalation-wire.ts:206-208 returns reason: "cooldown" without recording an audit event, and src/review/pending-closure-watchdog.ts:87-108 writes its counted event only on the success path.
Requirements
- Add
countRecentAuditEventsForActorTargetAndOutcome(env, actor, eventType, targetKey, outcome, sinceIso) to src/db/repositories.ts, adjacent to and shaped exactly like countRecentAuditEventsForActorAndTarget (:3153-3162), with one extra eq(auditEvents.outcome, outcome) term.
triggerPagerDutyIncident uses it with outcome: "completed" for both the loopover and legacy gittensory actor counts, preserving the two-actor union at notify-pagerduty.ts:184-192.
- Additionally exclude the auto-resolve rows, which are also
completed: auditPagerDutyNotification's detail for a resolve is the literal "resolved" (:254) and for a successful trigger the literal "triggered" (:216). The cooldown must count only rows whose detail is "triggered". Add a module-level exported constant for each of those two detail strings in notify-pagerduty.ts and use it at both the write and the read site, so the two spellings can never drift.
- Do not stop writing the
denied / cooldown_active row — it is the operator's evidence that suppression happened. It simply must no longer be counted.
- No change to
resolvePagerDutyCooldownMinutes, resolvePagerDutyMinSeverity, or the min-severity gate.
⚠️ Required pattern: countRecentAuditEventsForActorAndTarget at src/db/repositories.ts:3153-3162 is the shape to copy for the new counter, and src/review/pending-closure-watchdog.ts:87-108 is the precedent for "only the success path is counted". It does NOT satisfy this issue to stop writing the denied/error audit rows so the existing counter accidentally works; to filter in TypeScript by loading rows and inspecting them; or to add the outcome filter but leave resolved rows counted.
Deliverables
All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example filtering to completed without excluding the resolved detail — does not resolve this issue.
Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on src/**; both src/services/notify-pagerduty.ts and src/db/repositories.ts are inside coverage.include. Both arms of the new outcome/detail predicate need a test, and both actor spellings (loopover, gittensory) must remain exercised.
Expected Outcome
The PagerDuty cooldown suppresses a repeat page only when a page actually went out. A denied, failed, or auto-resolved notification no longer silences the next real incident, and a still-ongoing condition re-paged after the window no longer has its window pushed forward by its own suppression records.
Links & Resources
src/services/notify-pagerduty.ts:12-21, :123-140, :160-220, :230-260; src/db/repositories.ts:3149-3162; src/review/ops-wire.ts:298, :326; src/review/loop-escalation-wire.ts:206-208; src/review/pending-closure-watchdog.ts:87-108.
Context
src/services/notify-pagerduty.ts:19-21states the cooldown contract: "COOLDOWN — a repeat trigger for the SAMEdedup_keywithin PAGERDUTY_COOLDOWN_MINUTES ... is suppressed, so a still-ongoing condition re-checked every cron tick doesn't re-page every tick."The implementation counts the wrong population —
:184-192callscountRecentAuditEventsForActorAndTargettwice (for theloopoverand legacygittensoryactors) and suppresses when the sum is> 0.countRecentAuditEventsForActorAndTarget(src/db/repositories.ts:3153-3162) filters onactor + eventType + targetKey + createdAtand not onoutcome.auditPagerDutyNotification(:123-140) writes that sameeventTypeandtargetKeyfor every path in the file::171denied/below_min_severity;:190denied/cooldown_active;:218error(the page failed; nobody was woken);:254completed/resolved, written byresolvePagerDutyIncident, whichsrc/review/ops-wire.ts:298calls with the samededupKeythe trigger at:326uses.Four concrete failures at the default 60-minute cooldown with a sub-hourly cron:
warninganomaly denied at:171writes a row; anerroranomaly 5 minutes later is suppressed ascooldown_activeeven though nothing was ever paged.errorrow at:218suppresses the next attempt for the whole window.resolvedrow at:254is counted by the trigger's cooldown, so the recurrence after a clear is dropped.The analogous cooldowns in this codebase do the opposite deliberately:
src/review/loop-escalation-wire.ts:206-208returnsreason: "cooldown"without recording an audit event, andsrc/review/pending-closure-watchdog.ts:87-108writes its counted event only on the success path.Requirements
countRecentAuditEventsForActorTargetAndOutcome(env, actor, eventType, targetKey, outcome, sinceIso)tosrc/db/repositories.ts, adjacent to and shaped exactly likecountRecentAuditEventsForActorAndTarget(:3153-3162), with one extraeq(auditEvents.outcome, outcome)term.triggerPagerDutyIncidentuses it withoutcome: "completed"for both theloopoverand legacygittensoryactor counts, preserving the two-actor union atnotify-pagerduty.ts:184-192.completed:auditPagerDutyNotification'sdetailfor a resolve is the literal"resolved"(:254) and for a successful trigger the literal"triggered"(:216). The cooldown must count only rows whosedetailis"triggered". Add a module-level exported constant for each of those two detail strings innotify-pagerduty.tsand use it at both the write and the read site, so the two spellings can never drift.denied/cooldown_activerow — it is the operator's evidence that suppression happened. It simply must no longer be counted.resolvePagerDutyCooldownMinutes,resolvePagerDutyMinSeverity, or the min-severity gate.Deliverables
countRecentAuditEventsForActorTargetAndOutcomeexists insrc/db/repositories.tsand is unit-tested for both the matching and non-matching outcome.denied/below_min_severityrow inside the window does not suppress a subsequent qualifying trigger — the trigger fires and writes acompleted/triggeredrow.errorrow inside the window does not suppress the next trigger.completed/resolvedrow inside the window does not suppress the next trigger.cooldownMinutes + 1after the page fires, even thoughcooldown_activerows were written in between.completed/triggeredrow inside the window still suppresses (the cooldown's actual purpose) — asserted so the fix is not a silent removal of the cooldown.All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example filtering to
completedwithout excluding theresolveddetail — does not resolve this issue.Test Coverage Requirements
99%+ Codecov patch coverage, branch-counted, on
src/**; bothsrc/services/notify-pagerduty.tsandsrc/db/repositories.tsare insidecoverage.include. Both arms of the new outcome/detail predicate need a test, and both actor spellings (loopover,gittensory) must remain exercised.Expected Outcome
The PagerDuty cooldown suppresses a repeat page only when a page actually went out. A denied, failed, or auto-resolved notification no longer silences the next real incident, and a still-ongoing condition re-paged after the window no longer has its window pushed forward by its own suppression records.
Links & Resources
src/services/notify-pagerduty.ts:12-21,:123-140,:160-220,:230-260;src/db/repositories.ts:3149-3162;src/review/ops-wire.ts:298,:326;src/review/loop-escalation-wire.ts:206-208;src/review/pending-closure-watchdog.ts:87-108.