diff --git a/src/db/repositories.ts b/src/db/repositories.ts index 29b8306f85..6a7619ad71 100644 --- a/src/db/repositories.ts +++ b/src/db/repositories.ts @@ -2879,6 +2879,23 @@ export async function mostRecentAuditEventForOtherTarget(env: Env, actor: string return rows[0]?.createdAt ?? null; } +/** The `detail` of the newest `completed` audit event for `(actor, eventType, targetKey)`, or `null` when + * none exists. Backs the PagerDuty ops-anomaly auto-resolve lifecycle (#8903): only a *completed* trigger or + * resolve actually moved the incident's state at PagerDuty (a denied/errored attempt did not), so the newest + * completed detail is the durable "is an incident currently open" signal that survives cron ticks and isolate + * recycling without a bespoke table -- the same `audit_events` row the trigger already writes stands in as the + * persisted previous-tick state. */ +export async function latestCompletedAuditEventDetail(env: Env, actor: string, eventType: string, targetKey: string): Promise { + const db = getDb(env.DB); + const rows = await db + .select({ detail: auditEvents.detail }) + .from(auditEvents) + .where(and(eq(auditEvents.actor, actor), eq(auditEvents.eventType, eventType), eq(auditEvents.targetKey, targetKey), eq(auditEvents.outcome, "completed"))) + .orderBy(desc(auditEvents.createdAt)) + .limit(1); + return rows[0]?.detail ?? null; +} + /** Count-returning, cross-repo variant (#4515): how many recent events of this type has this actor generated, * across EVERY target (repo/PR), within the recency window? Unlike {@link countRecentAuditEventsForActorAndTarget} * (scoped to one target thread), this counts an actor's ACTIVITY VOLUME irrespective of which PR/repo each diff --git a/src/review/ops-wire.ts b/src/review/ops-wire.ts index 44dc72f85a..e212765388 100644 --- a/src/review/ops-wire.ts +++ b/src/review/ops-wire.ts @@ -32,13 +32,13 @@ // auto-apply.ts's own isStrictlyTightening / evaluateShadowPromotion anticipate. This module is READ-ONLY // observability: it reports drift; it never changes what blocks a live PR. -import { findHottestInconclusiveReviewTargetForRepo, findHottestReviewTargetForRepo, listRepositories, sumByokAiUsageForRepoSince } from "../db/repositories"; +import { findHottestInconclusiveReviewTargetForRepo, findHottestReviewTargetForRepo, latestCompletedAuditEventDetail, listRepositories, sumByokAiUsageForRepoSince } from "../db/repositories"; import { incr } from "../selfhost/metrics"; import { isAgentConfigured } from "../settings/autonomy"; import { resolveRepositorySettings } from "../settings/repository-settings"; import { loadGatePrecisionReport, type GatePrecisionReport } from "../services/gate-precision"; import { buildRepoOutcomeCalibration, type OutcomeCalibration } from "../services/outcome-calibration"; -import { triggerPagerDutyIncident, type PagerDutySeverity } from "../services/notify-pagerduty"; +import { isPagerDutyEnabled, resolvePagerDutyIncident, triggerPagerDutyIncident, type PagerDutySeverity } from "../services/notify-pagerduty"; import { loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { resolveLoopOverSelfRepoFullName } from "../config/loopover-repo-focus-manifest"; import { errorMessage, nowIso } from "../utils/json"; @@ -282,7 +282,24 @@ export async function runOpsAlerts(env: Env): Promise> findHottestInconclusiveReviewTargetForRepo(env, repoFullName, reviewBurstSinceIso), ]); const anomalies = detectOutcomeAnomalies({ repoFullName, gatePrecision, calibration, reviewBurst, reviewFailureBurst }); - if (anomalies.length === 0) continue; + const dedupKey = `ops_anomaly:${repoFullName}`; + if (anomalies.length === 0) { + // AUTO-RESOLVE (#8903): a repo that is healthy THIS tick but had an OPEN PagerDuty incident from a + // prior tick (its newest completed pagerduty audit for this dedupKey was a `trigger`, not yet a + // `resolve`) gets a matching `resolve` event so the incident auto-closes instead of waiting for a + // human. The `audit_events` row the trigger already writes IS the persisted previous-tick state -- + // no bespoke table needed, and it survives isolate recycling across cron ticks. Gated on + // isPagerDutyEnabled so a flag-OFF deploy (the default) does zero new work and healthy repos never + // run the audit-log lookup -- byte-identical to today when paging is off. resolvePagerDutyIncident + // is itself a no-op when no routing key resolves and never throws. + if (isPagerDutyEnabled(env)) { + const lastAction = await latestCompletedAuditEventDetail(env, "loopover", "external_notification.pagerduty", dedupKey); + if (lastAction === "triggered") { + await resolvePagerDutyIncident(env, { repoFullName, dedupKey }); + } + } + continue; + } found[repoFullName] = anomalies; // Structured log = loopover's notify path (no Discord/operator webhook exists) AND the Sentry path // (level:"error" + an `event` field reaches forwardStructuredLogToSentry). One line per repo. @@ -301,15 +318,16 @@ export async function runOpsAlerts(env: Env): Promise> // comment for why alert fatigue needed both controls, not just PagerDuty's own dedup_key. Awaited (not // fire-and-forget) so a page failure is captured within THIS tick's own error handling, not orphaned // after runOpsAlerts has already returned -- triggerPagerDutyIncident itself never throws and bounds - // its own HTTP call to a 5s timeout, so this cannot hang the scan. This does not yet send a matching - // "resolve" event once anomalies clear (would need tracking previous-tick state) -- an operator - // currently resolves the incident manually once the underlying condition is fixed. + // its own HTTP call to a 5s timeout, so this cannot hang the scan. A matching "resolve" event auto- + // closes the incident once anomalies clear on a later tick (#8903): the healthy-branch above reads + // this trigger's own persisted `audit_events` row as the previous-tick state, so no operator has to + // resolve it manually anymore. const worst = worstAnomaly(anomalies); await triggerPagerDutyIncident(env, { repoFullName, summary: worst.line, severity: worst.severity, - dedupKey: `ops_anomaly:${repoFullName}`, + dedupKey, customDetails: { anomalies }, }); // #ops-anomaly-metric: Prometheus counterpart to the log line above so a self-host operator can alert on diff --git a/src/services/notify-pagerduty.ts b/src/services/notify-pagerduty.ts index 073aeaf4a0..28f117ec31 100644 --- a/src/services/notify-pagerduty.ts +++ b/src/services/notify-pagerduty.ts @@ -218,3 +218,43 @@ export async function triggerPagerDutyIncident( await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "error", message.slice(0, 280)); } } + +/** Resolve (auto-close) a previously-triggered PagerDuty incident for `repoFullName` via the Events API's + * `resolve` action, keyed by the SAME `dedupKey` the trigger used (#8903). Best-effort — never throws — like + * {@link triggerPagerDutyIncident}, and a no-op under the same not-opted-in conditions (flag off / no routing + * key resolves). Unlike a trigger it applies NO min-severity floor or cooldown: closing an incident that has + * already cleared should always go through, and PagerDuty itself ignores a `resolve` for a `dedup_key` with no + * open incident, so a spurious resolve is harmless. A `resolve` needs only the routing key + dedup key (no + * payload). Audited as `completed`/`resolved` so the auto-resolve is discoverable and the trigger→resolve + * lifecycle is queryable (an invalid-but-present key is audited `denied`, matching the trigger path). */ +export async function resolvePagerDutyIncident( + env: Env, + params: { repoFullName: string; dedupKey: string }, +): Promise { + const resolution = resolvePagerDutyRoutingKey(env, params.repoFullName); + if (resolution.status === "disabled") { + if (resolution.reason !== "flag_off") { + await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "denied", resolution.reason); + } + return; + } + + try { + const response = await fetch(PAGERDUTY_EVENTS_URL, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + routing_key: resolution.routingKey, + event_action: "resolve", + dedup_key: params.dedupKey, + }), + 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 }); + } catch (error) { + const message = errorMessage(error); + console.warn(JSON.stringify({ event: "pagerduty_resolve_failed", repo: params.repoFullName, message: message.slice(0, 200) })); + await auditPagerDutyNotification(env, { repoFullName: params.repoFullName, dedupKey: params.dedupKey }, "error", message.slice(0, 280)); + } +} diff --git a/test/unit/notify-pagerduty.test.ts b/test/unit/notify-pagerduty.test.ts index 9b01efc361..5158dd0032 100644 --- a/test/unit/notify-pagerduty.test.ts +++ b/test/unit/notify-pagerduty.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { isPagerDutyEnabled, resolvePagerDutyCooldownMinutes, + resolvePagerDutyIncident, resolvePagerDutyMinSeverity, resolvePagerDutyRoutingKey, triggerPagerDutyIncident, @@ -249,6 +250,66 @@ describe("triggerPagerDutyIncident — cooldown gate (alert fatigue control #2)" }); }); +describe("resolvePagerDutyIncident — auto-close a cleared incident (#8903)", () => { + const resolve = (env: Env, dedupKey = "ops_anomaly:acme/widgets"): Promise => + resolvePagerDutyIncident(env, { repoFullName: "acme/widgets", dedupKey }); + + it("flag off → no fetch, no audit row (silent, like the trigger path)", async () => { + const calls = stubFetch(); + const env = withEnv(); + await resolve(env); + expect(calls).toEqual([]); + expect(await pagerDutyAudit(env)).toEqual([]); + }); + + it("flag on but no routing key resolves → no fetch, audited denied/missing_global_key", async () => { + const calls = stubFetch(); + const env = withEnv({ LOOPOVER_ENABLE_PAGERDUTY: "1" }); + await resolve(env); + expect(calls).toEqual([]); + expect(await pagerDutyAudit(env)).toEqual([expect.objectContaining({ outcome: "denied", detail: "missing_global_key" })]); + }); + + it("posts a resolve event (routing + dedup key, no payload) and audits completed/resolved on success", async () => { + const calls = stubFetch(202); + const env = enabledEnv(); + await resolve(env); + expect(calls).toHaveLength(1); + expect(calls[0]?.url).toBe("https://events.pagerduty.com/v2/enqueue"); + expect(calls[0]?.body).toEqual({ routing_key: VALID_KEY, event_action: "resolve", dedup_key: "ops_anomaly:acme/widgets" }); + expect(await pagerDutyAudit(env)).toEqual([expect.objectContaining({ outcome: "completed", detail: "resolved" })]); + }); + + it("has NO severity floor or cooldown -- always resolves even right after a recent page for the same dedupKey", async () => { + const calls = stubFetch(202); + const env = enabledEnv(); + // A page recorded moments ago would suppress a repeat TRIGGER via the cooldown -- a resolve must not care. + await recordAuditEvent(env, { eventType: "external_notification.pagerduty", actor: "loopover", targetKey: "ops_anomaly:acme/widgets", outcome: "completed", detail: "triggered" }); + await resolve(env); + expect(calls).toHaveLength(1); + expect(calls[0]?.body).toMatchObject({ event_action: "resolve" }); + }); + + it("a non-ok response is audited as an error and never throws", async () => { + stubFetch(500); + const env = enabledEnv(); + await expect(resolve(env)).resolves.toBeUndefined(); + expect(await pagerDutyAudit(env)).toEqual([expect.objectContaining({ outcome: "error" })]); + }); + + it("a network failure is audited as an error, logs pagerduty_resolve_failed, and never throws", async () => { + vi.stubGlobal("fetch", async () => { + throw new Error("network down"); + }); + const warn = vi.spyOn(console, "warn").mockImplementation(() => undefined); + const env = enabledEnv(); + await expect(resolve(env)).resolves.toBeUndefined(); + expect(await pagerDutyAudit(env)).toEqual([expect.objectContaining({ outcome: "error", detail: expect.stringContaining("network down") })]); + expect(warn).toHaveBeenCalledWith(expect.stringContaining("pagerduty_resolve_failed")); + warn.mockRestore(); + }); +}); + describe("triggerPagerDutyIncident — HTTP delivery", () => { it("posts the PagerDuty Events API v2 payload and audits completed on success", async () => { const calls = stubFetch(202); diff --git a/test/unit/ops-wire.test.ts b/test/unit/ops-wire.test.ts index be20a58d84..020e65c0a4 100644 --- a/test/unit/ops-wire.test.ts +++ b/test/unit/ops-wire.test.ts @@ -611,6 +611,52 @@ describe("runOpsAlerts — cron path over gittensory's outcome data", () => { expect(found["owner/repo"]?.some((a) => /review burst/.test(a))).toBe(true); expect(calls).toEqual([]); }); + + // ── Auto-resolve a cleared incident (#8903): two consecutive ticks, anomaly then healthy ───────────────── + function stubPagerDutyLifecycleFetch(): Array<{ event_action: string; dedup_key: string }> { + const calls: Array<{ event_action: string; dedup_key: string }> = []; + vi.stubGlobal("fetch", async (_url: RequestInfo | URL, init?: RequestInit) => { + calls.push(JSON.parse(String(init?.body)) as { event_action: string; dedup_key: string }); + return new Response(null, { status: 202 }); + }); + return calls; + } + + it("AUTO-RESOLVE (#8903): sends a PagerDuty resolve on the tick a paged repo's anomalies clear", async () => { + const calls = stubPagerDutyLifecycleFetch(); + const env = createTestEnv({ LOOPOVER_ENABLE_PAGERDUTY: "1", PAGERDUTY_ROUTING_KEY: PD_KEY }); + await seedRegisteredRepo(env, "owner/repo"); + // Tick 1: an error-grade review burst pages (triggers) an incident. + for (let i = 0; i < 7; i += 1) { + await recordAuditEvent(env, { eventType: "github_app.pr_public_surface_published", actor: "contributor", targetKey: "owner/repo#99", outcome: "completed" }); + } + vi.spyOn(console, "error").mockImplementation(() => {}); + + const tick1 = await runOpsAlerts(env); + expect(tick1["owner/repo"]?.some((a) => /review burst/.test(a))).toBe(true); + expect(calls).toHaveLength(1); + expect(calls[0]).toMatchObject({ event_action: "trigger", dedup_key: "ops_anomaly:owner/repo" }); + + // The publish surge falls out of the burst window before tick 2, so the repo is healthy again. + await env.DB.prepare("DELETE FROM audit_events WHERE event_type = ?").bind("github_app.pr_public_surface_published").run(); + + const tick2 = await runOpsAlerts(env); + expect(tick2["owner/repo"]).toBeUndefined(); // healthy now + expect(calls).toHaveLength(2); + expect(calls[1]).toMatchObject({ event_action: "resolve", dedup_key: "ops_anomaly:owner/repo" }); + }); + + it("AUTO-RESOLVE (#8903): a still-healthy repo that never paged an incident sends no resolve", async () => { + const calls = stubPagerDutyLifecycleFetch(); + const env = createTestEnv({ LOOPOVER_ENABLE_PAGERDUTY: "1", PAGERDUTY_ROUTING_KEY: PD_KEY }); + await seedRegisteredRepo(env, "owner/clean"); + vi.spyOn(console, "error").mockImplementation(() => {}); + + const found = await runOpsAlerts(env); + + expect(found["owner/clean"]).toBeUndefined(); + expect(calls).toEqual([]); // no trigger (healthy) and no resolve (no open incident to close) + }); }); describe("computeOpsStats — cross-repo outcome aggregate", () => {