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

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 #9695

Author: shin-core

src/db/repositories.ts

@@ -3161,6 +3161,39 @@ export async function countRecentAuditEventsForActorAndTarget(env: Env, actor: s
   return row.count;
 }
 
+/** Same as {@link countRecentAuditEventsForActorAndTarget} but additionally scoped to a specific `outcome` +
+ *  `detail`. Backs the PagerDuty cooldown (#9695): a repeat page is suppressed only by rows that ACTUALLY paged
+ *  (outcome "completed", detail "triggered"), never by the suppression rows or the auto-resolve rows that share
+ *  the same eventType/targetKey — otherwise a single page (or even a `denied` non-page) self-renewed the window
+ *  every cron tick and the repo never paged again. */
+export async function countRecentAuditEventsForActorTargetAndOutcome(
+  env: Env,
+  actor: string,
+  eventType: string,
+  targetKey: string,
+  outcome: string,
+  detail: string,
+  sinceIso: string,
+): Promise<number> {
+  const db = getDb(env.DB);
+  const [row] = await db
+    .select({ count: sql<number>`count(*)` })
+    .from(auditEvents)
+    .where(
+      and(
+        eq(auditEvents.actor, actor),
+        eq(auditEvents.eventType, eventType),
+        eq(auditEvents.targetKey, targetKey),
+        eq(auditEvents.outcome, outcome),
+        eq(auditEvents.detail, detail),
+        gte(auditEvents.createdAt, sinceIso),
+      ),
+    );
+  /* v8 ignore next -- count(*) always returns exactly one row; the empty-array guard only satisfies the destructure type. */
+  if (!row) return 0;
+  return row.count;
+}
+
 /** Shared by every `targetKey` literal-prefix `LIKE` scan below ({@link countRecentAuditEventsForActorInRepo},
  *  {@link findHottestReviewTargetForRepo}) so a repo name containing a SQL `LIKE` wildcard (`%`/`_`) is always
  *  matched literally, never as a pattern -- e.g. `owner/foo_bar` must never spuriously match `owner/fooXbar#...`'s

src/services/notify-pagerduty.ts

@@ -1,4 +1,4 @@
-import { countRecentAuditEventsForActorAndTarget, recordAuditEvent } from "../db/repositories";
+import { countRecentAuditEventsForActorTargetAndOutcome, recordAuditEvent } from "../db/repositories";
 import { errorMessage } from "../utils/json";
 import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity } from "./severity-threshold";
 
@@ -21,6 +21,12 @@ import { meetsSeverityThreshold, resolveSeverityThreshold, type LoopoverSeverity
 //     every cron tick doesn't re-page every tick.
 
 const PAGERDUTY_EVENTS_URL = "https://events.pagerduty.com/v2/enqueue";
+
+// The audit `detail` written for a real page vs an auto-resolve. Both share outcome "completed", so the cooldown
+// must key off `detail` to count only rows that ACTUALLY paged (#9695). Exported + used at both the write and
+// the read site so the two spellings can never drift.
+export const PAGERDUTY_AUDIT_DETAIL_TRIGGERED = "triggered";
+export const PAGERDUTY_AUDIT_DETAIL_RESOLVED = "resolved";
 // PagerDuty routing/integration keys are 32 lowercase hex characters.
 const ROUTING_KEY_RE = /^[a-f0-9]{32}$/i;
 const DEFAULT_MIN_SEVERITY: PagerDutySeverity = "error";
@@ -182,8 +188,8 @@ export async function triggerPagerDutyIncident(
   // window can reach back across the deploy boundary. Querying both actors costs one extra indexed count and
   // removes the whole risk category rather than requiring a precisely-timed follow-up cleanup.
   const [recentPagesNewActor, recentPagesLegacyActor] = await Promise.all([
-    countRecentAuditEventsForActorAndTarget(env, "loopover", "external_notification.pagerduty", params.dedupKey, cooldownSinceIso),
-    countRecentAuditEventsForActorAndTarget(env, "gittensory", "external_notification.pagerduty", params.dedupKey, cooldownSinceIso),
+    countRecentAuditEventsForActorTargetAndOutcome(env, "loopover", "external_notification.pagerduty", params.dedupKey, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, cooldownSinceIso),
+    countRecentAuditEventsForActorTargetAndOutcome(env, "gittensory", "external_notification.pagerduty", params.dedupKey, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, cooldownSinceIso),
   ]);
   const recentPages = recentPagesNewActor + recentPagesLegacyActor;
   if (recentPages > 0) {
@@ -211,7 +217,7 @@ export async function triggerPagerDutyIncident(
       signal: AbortSignal.timeout(5000),
     });
     if (!response.ok) throw new Error(`pagerduty_events_http_${response.status}`);
-    await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", "triggered", { source: resolution.source });
+    await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", PAGERDUTY_AUDIT_DETAIL_TRIGGERED, { source: resolution.source });
   } catch (error) {
     const message = errorMessage(error);
     console.warn(JSON.stringify({ event: "pagerduty_trigger_failed", repo: params.repoFullName, message: message.slice(0, 200) }));
@@ -251,7 +257,7 @@ export async function resolvePagerDutyIncident(
       signal: AbortSignal.timeout(5000),
     });
     if (!response.ok) throw new Error(`pagerduty_events_http_${response.status}`);
-    await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", "resolved", { source: resolution.source });
+    await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "completed", PAGERDUTY_AUDIT_DETAIL_RESOLVED, { source: resolution.source });
   } catch (error) {
     const message = errorMessage(error);
     console.warn(JSON.stringify({ event: "pagerduty_resolve_failed", repo: params.repoFullName, message: message.slice(0, 200) }));

test/unit/notify-pagerduty.test.ts

@@ -230,6 +230,60 @@ describe("triggerPagerDutyIncident — cooldown gate (alert fatigue control #2)"
     expect(calls).toHaveLength(1);
   });
 
+  it("REGRESSION: a recent suppression (denied/cooldown_active) row does NOT itself suppress the next page (#9695)", async () => {
+    const calls = stubFetch();
+    const env = enabledEnv();
+    const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
+    // Before #9695 this `denied` row was counted (the counter did not filter outcome/detail), so a single
+    // suppression self-renewed the window and the repo could never page again.
+    await recordAuditEvent(env, {
+      eventType: "external_notification.pagerduty",
+      actor: "loopover",
+      targetKey: "ops_anomaly:acme/widgets",
+      outcome: "denied",
+      detail: "cooldown_active",
+      metadata: {},
+      createdAt: fiveMinutesAgo,
+    });
+    await trigger(env);
+    expect(calls).toHaveLength(1); // it pages, because no ACTUAL page happened in the window
+  });
+
+  it("REGRESSION: a recent auto-resolve (completed/resolved) row does NOT suppress the next page (#9695)", async () => {
+    const calls = stubFetch();
+    const env = enabledEnv();
+    const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
+    // resolved shares outcome "completed" with a real page, so an outcome-only filter would wrongly count it.
+    await recordAuditEvent(env, {
+      eventType: "external_notification.pagerduty",
+      actor: "loopover",
+      targetKey: "ops_anomaly:acme/widgets",
+      outcome: "completed",
+      detail: "resolved",
+      metadata: {},
+      createdAt: fiveMinutesAgo,
+    });
+    await trigger(env);
+    expect(calls).toHaveLength(1); // a re-page for a flapping condition is not blocked by its own resolve
+  });
+
+  it("REGRESSION: a recent failed page (error) does NOT suppress its own retry (#9695)", async () => {
+    const calls = stubFetch();
+    const env = enabledEnv();
+    const fiveMinutesAgo = new Date(Date.now() - 5 * 60 * 1000).toISOString();
+    await recordAuditEvent(env, {
+      eventType: "external_notification.pagerduty",
+      actor: "loopover",
+      targetKey: "ops_anomaly:acme/widgets",
+      outcome: "error",
+      detail: "The page failed",
+      metadata: {},
+      createdAt: fiveMinutesAgo,
+    });
+    await trigger(env);
+    expect(calls).toHaveLength(1); // nobody was paged, so the retry must go through
+  });
+
   it("REGRESSION: a recent page recorded under the pre-rebrand 'loopover' actor still suppresses a duplicate within the cooldown window", async () => {
     const calls = stubFetch();
     const env = enabledEnv();