diff --git a/packages/plugins/paperclip-plugin-alertmanager/README.md b/packages/plugins/paperclip-plugin-alertmanager/README.md index f21902e1a61d..2bd952247786 100644 --- a/packages/plugins/paperclip-plugin-alertmanager/README.md +++ b/packages/plugins/paperclip-plugin-alertmanager/README.md @@ -20,8 +20,10 @@ See `docs/specs/2026-04-29-alertmanager-plugin-spec.md` for the full design. - Deduplicates by `alert.fingerprint` per spec §5.3 — re-fires bump the state row and refresh the issue body, they don't create a second issue. - Re-opens issues the plugin auto-cancelled on resolve when the same - fingerprint re-fires (§8.3 option A), while preserving operator-cancelled - suppressions. + fingerprint re-fires (§8.3 option A). An issue closed by an *operator* while + its alert was still firing suppresses re-opens instead — but only for + `operatorSuppressionHours` (default 24h), after which a still-firing alert + re-opens it with an explanatory comment. See "Operator suppression" below. - Resolves issues per `autoCloseOnResolve`: either close the issue (status → cancelled) or post an `Alert resolved at ` comment. - Renders observability drill-in links (Grafana / Tempo / Pyroscope / Hubble @@ -57,6 +59,7 @@ Configured per-instance via the host's plugin settings UI. Schema lives in | `acceptOnlyLabels` | object | no | Accept-only label filter, e.g. `{ paperclip: "true" }`. | | `severityToPriority` | object | no | Override the default severity map. | | `autoCloseOnResolve` | boolean | no | Defaults to true (status → cancelled). Set false for comment-only. | +| `operatorSuppressionHours` | number | no | How long an operator-closed issue mutes re-fires before the plugin re-opens it anyway. Defaults to 24. `0` = suppress indefinitely (pre-BLO-24234 behaviour). | | `ownerMap` | object | no | `{ : { : } }`. | | `issueRouteMap` | object | no | `{ : { : { projectId, goalId, assigneeAgentId, status } } }`. | @@ -193,6 +196,45 @@ ending in `_url` is ignored. | `runbook_url` | Runbook | | (alert.generatorURL) | Source query in Prometheus | +### Operator suppression, and what a re-fire does (BLO-24234) + +Every re-fire of a known fingerprint takes exactly one of four branches. The +branch is decided by `decideRefire()` in `webhook-handler.ts` and each one emits +a distinct metric, so "the alert delivered but I see no issue" is answerable +from telemetry rather than by reading the issue body's `Started:` timestamp. + +| Issue status at re-fire | `resolvedAt` in state | Outcome | Metric | +|---|---|---|---| +| open (any non-terminal) | — | refresh description | `alertmanager.firing.deduped` | +| `done` / `cancelled` | set (plugin closed it on resolve) | re-open → `todo` | `alertmanager.firing.reopened` | +| `done` / `cancelled` | null (**operator** closed it) — inside window | stay closed, stay quiet | `alertmanager.firing.suppressed` | +| `done` / `cancelled` | null — window expired | re-open → `todo` + comment | `alertmanager.firing.suppression_expired` | +| issue unreadable / deleted | — | leave state intact | `alertmanager.firing.issue_missing` | + +`alertmanager.firing.deduped` is still emitted on **every** re-fire, so existing +dashboards keep working; the metrics above narrate what the re-fire actually did. + +**Why the window exists.** Closing an alert issue by hand means "stop nagging +me", and the plugin honours that. But an unbounded mute is a footgun: a +fingerprint is `hash(sorted(labels))`, so a provider-agnostic alert such as +`LLMProxyHighErrorRate` re-uses **one** fingerprint across every future root +cause. Before this change, one operator closing a noisy issue muted that alert +permanently — the webhook kept delivering 200s, the state row kept updating, and +nothing was visible in any open-status view. That is the failure mode behind the +2026-08-08 investigation in BLO-23405/BLO-24234. + +Suppression is anchored on the **first re-fire observed against the closed +issue**, not on the close itself (the plugin never sees the close), and the +anchor is not refreshed by later re-fires — otherwise the window would slide +forever and never expire. Closing the issue again after a re-open starts a fresh +window. Set `operatorSuppressionHours: 0` to restore the old unbounded mute. + +**Known asymmetry, deliberate:** if the state row is lost *and* the issue is +terminal, `recoverStateFromIssue()` declines to adopt it and a fresh issue is +filed instead. After a state loss the plugin cannot tell whether the close was +its own or an operator's, and for a paging system a visible duplicate is a safer +failure than an inherited mute. + ## Security - **Always set `webhookToken`.** Without a token the diff --git a/packages/plugins/paperclip-plugin-alertmanager/src/__tests__/worker.test.ts b/packages/plugins/paperclip-plugin-alertmanager/src/__tests__/worker.test.ts index 292457d3b117..8abfd62a2e73 100644 --- a/packages/plugins/paperclip-plugin-alertmanager/src/__tests__/worker.test.ts +++ b/packages/plugins/paperclip-plugin-alertmanager/src/__tests__/worker.test.ts @@ -11,6 +11,7 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; import { AlertDeliveryIncompleteError, WebhookUnauthorizedError, + decideRefire, handleWebhook, verifyBearerToken, } from "../webhook-handler.js"; @@ -705,6 +706,248 @@ describe("handleWebhook — dedup on re-fire", () => { }); }); +// --------------------------------------------------------------------------- +// BLO-24234 — operator suppression is bounded and observable +// +// The pre-existing contract (test above) is that an operator closing an alert +// issue by hand suppresses re-opens. That is deliberate and preserved. What was +// wrong was that the suppression was *permanent* and *silent*: the re-fire +// emitted only `firing.deduped`, indistinguishable from a healthy re-fire +// against an open issue, so a delivered page could produce no visible artifact +// forever. These tests pin the four decision points. +// --------------------------------------------------------------------------- + +describe("handleWebhook — operator suppression (BLO-24234)", () => { + const suppressedState = ( + overrides: Partial = {}, + ): AlertStateRecord => ({ + paperclipIssueId: "issue-existing", + paperclipCompanyId: "company-1", + assigneeUserId: "user-42", + assigneeAgentId: null, + alertname: "CiliumPolicyDropsHigh", + severity: "critical", + firstSeenAt: "2026-04-29T08:00:00Z", + lastFiredAt: "2026-04-29T08:00:00Z", + resolvedAt: null, + ...overrides, + }); + + const hoursAgo = (h: number) => new Date(Date.now() - h * 60 * 60 * 1000).toISOString(); + + it("stamps the suppression anchor and emits firing.suppressed on first sight", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce(suppressedState()); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + expect(mocks.issues.update).not.toHaveBeenCalled(); + expect(mocks.metrics.write).toHaveBeenCalledWith( + "alertmanager.firing.suppressed", + 1, + { alertname: "CiliumPolicyDropsHigh", severity: "critical" }, + ); + // The anchor must be persisted, or the window can never expire. + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toEqual(expect.any(String)); + expect(Date.parse(written.operatorSuppressedAt as string)).toBeGreaterThan(0); + // A muted fingerprint is a warning, not routine. + expect(mocks.logger.warn).toHaveBeenCalledWith( + expect.stringContaining("suppressing re-open until"), + ); + }); + + it("keeps suppressing — and preserves the original anchor — inside the window", async () => { + const { ctx, mocks } = mkCtx(); + const anchor = hoursAgo(5); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: anchor }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + expect(mocks.issues.update).not.toHaveBeenCalled(); + expect(mocks.metrics.write).toHaveBeenCalledWith( + "alertmanager.firing.suppressed", + 1, + expect.any(Object), + ); + // Re-anchoring on every re-fire would make the window slide forever and + // recreate the permanent mute this change exists to remove. + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toBe(anchor); + }); + + it("re-opens once the suppression window expires, with an explanatory comment", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: hoursAgo(25) }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + expect(mocks.issues.update).toHaveBeenCalledWith( + "issue-existing", + expect.objectContaining({ status: "todo" }), + "company-1", + ); + expect(mocks.metrics.write).toHaveBeenCalledWith( + "alertmanager.firing.suppression_expired", + 1, + { alertname: "CiliumPolicyDropsHigh", severity: "critical" }, + ); + expect(mocks.issues.createComment).toHaveBeenCalledWith( + "issue-existing", + expect.stringContaining("kept firing past"), + "company-1", + ); + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toBeNull(); + }); + + it("re-arms the escalation ladder on a suppression-expiry re-open", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ + operatorSuppressedAt: hoursAgo(25), + // Frozen while the issue sat closed; a re-open that left these alone + // would surface the issue but never page anyone about it again. + nextEscalationAt: "2026-04-29T09:00:00Z", + escalationComplete: true, + }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.nextEscalationAt).not.toBe("2026-04-29T09:00:00Z"); + expect(Date.parse(written.nextEscalationAt as string)).toBeGreaterThan(Date.now()); + }); + + it("suppresses indefinitely when operatorSuppressionHours=0", async () => { + const { ctx, mocks } = mkCtx(); + const config = { ...baseConfig(), operatorSuppressionHours: 0 }; + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: hoursAgo(24 * 365) }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, config, TOKEN, baseInput()); + + expect(mocks.issues.update).not.toHaveBeenCalled(); + expect(mocks.metrics.write).toHaveBeenCalledWith( + "alertmanager.firing.suppressed", + 1, + expect.any(Object), + ); + }); + + it("clears a stale suppression anchor once the issue is open again", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: hoursAgo(5) }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "in_progress" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + // Body refresh, no status change — the ordinary re-fire path. + const updatePatch = mocks.issues.update.mock.calls[0][1]; + expect(updatePatch.status).toBeUndefined(); + // A carried-over anchor would let a later close inherit an already-expired + // window and re-open immediately, defeating the operator's decision. + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toBeNull(); + }); + + it("re-anchors rather than muting forever when the anchor is unparseable", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: "not-a-timestamp" }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "cancelled" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(Date.parse(written.operatorSuppressedAt as string)).toBeGreaterThan(0); + }); + + it("reports a re-fire whose tracked issue has vanished", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce(suppressedState()); + mocks.issues.get.mockResolvedValueOnce(null); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + expect(mocks.metrics.write).toHaveBeenCalledWith( + "alertmanager.firing.issue_missing", + 1, + { alertname: "CiliumPolicyDropsHigh", severity: "critical" }, + ); + expect(mocks.issues.update).not.toHaveBeenCalled(); + }); + + it("does not bank a suppression anchor when the issue RPC failed", async () => { + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce(suppressedState()); + mocks.issues.get.mockRejectedValueOnce(new Error("issues.get exploded")); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + // Persisting an anchor off a call that never landed would start the + // suppression clock on a status nobody actually observed. + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toBeNull(); + }); + + it("restarts the ladder on resolve→re-fire even when the issue is already open", async () => { + // Regression guard: an operator can re-open the issue by hand between the + // resolve and the re-fire, which makes this a plain description refresh + // rather than a plugin re-open. `handleResolved` has still nulled + // `nextEscalationAt` and set `escalationComplete`, so gating the ladder + // restart on the re-open branch would leave this alert permanently + // un-escalatable. + const { ctx, mocks } = mkCtx(); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ + resolvedAt: "2026-04-29T09:00:00Z", + nextEscalationAt: null, + escalationComplete: true, + escalationAttempt: 3, + }), + ); + mocks.issues.get.mockResolvedValueOnce({ id: "issue-existing", status: "todo" }); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.escalationComplete).toBe(false); + expect(written.escalationAttempt).toBe(0); + expect(Date.parse(written.nextEscalationAt as string)).toBeGreaterThan(Date.now()); + }); + + it("preserves the suppression anchor when the issue could not be read", async () => { + const { ctx, mocks } = mkCtx(); + const anchor = hoursAgo(5); + mocks.state.get.mockResolvedValueOnce( + suppressedState({ operatorSuppressedAt: anchor }), + ); + mocks.issues.get.mockResolvedValueOnce(null); + + await handleWebhook(ctx, baseConfig(), TOKEN, baseInput()); + + // Dropping it would restart the window on the next readable re-fire, + // extending the mute past what the operator's close bought. + const written = mocks.state.set.mock.calls.at(-1)?.[1] as AlertStateRecord; + expect(written.operatorSuppressedAt).toBe(anchor); + }); +}); + describe("handleWebhook — resolved", () => { it("posts a comment when autoCloseOnResolve=false", async () => { const { ctx, mocks } = mkCtx(); @@ -1610,3 +1853,140 @@ describe("BLO-20467 — firing retries are idempotent across create/state-write" expect(persisted.escalationComplete).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// decideRefire — the re-fire decision table as a pure function (BLO-24234) +// +// The handler tests above drive these branches through a whole webhook +// delivery, which is the right level for asserting side effects (metrics, +// comments, state writes). These assert the decision itself, so the table in +// the README has a direct, cheap counterpart in code — and so a future change +// to the branch order fails here with an obvious diff rather than as a +// surprising side effect three layers up. +// --------------------------------------------------------------------------- + +describe("decideRefire", () => { + const NOW = Date.parse("2026-05-01T12:00:00Z"); + const cfg = (hours?: number): AlertmanagerPluginConfig => ({ + defaultCompanyId: "company-1", + ...(hours === undefined ? {} : { operatorSuppressionHours: hours }), + }); + const ago = (h: number) => new Date(NOW - h * 60 * 60 * 1000).toISOString(); + + it("refreshes any non-terminal issue regardless of suppression state", () => { + for (const status of ["todo", "in_progress", "in_review", "blocked"]) { + expect( + decideRefire({ status }, { resolvedAt: null, operatorSuppressedAt: ago(99) }, cfg(), NOW), + ).toEqual({ kind: "refresh" }); + } + }); + + it("re-opens a terminal issue the plugin closed on resolve", () => { + for (const status of ["done", "cancelled"]) { + expect( + decideRefire({ status }, { resolvedAt: ago(1), operatorSuppressedAt: null }, cfg(), NOW), + ).toEqual({ kind: "reopen", reason: "plugin_resolved" }); + } + }); + + it("suppresses an operator close, anchoring on first observation", () => { + expect( + decideRefire({ status: "cancelled" }, { resolvedAt: null, operatorSuppressedAt: null }, cfg(), NOW), + ).toEqual({ + kind: "suppressed", + suppressedAt: new Date(NOW).toISOString(), + firstObservation: true, + }); + }); + + it("keeps the original anchor while inside the window", () => { + const anchor = ago(23); + expect( + decideRefire({ status: "cancelled" }, { resolvedAt: null, operatorSuppressedAt: anchor }, cfg(), NOW), + ).toEqual({ kind: "suppressed", suppressedAt: anchor, firstObservation: false }); + }); + + it("re-opens once the window has elapsed", () => { + expect( + decideRefire({ status: "cancelled" }, { resolvedAt: null, operatorSuppressedAt: ago(24) }, cfg(), NOW), + ).toEqual({ kind: "reopen", reason: "suppression_expired" }); + }); + + it("treats the boundary as expired, not as still-suppressed", () => { + // Exactly 24h. `>=` matters: a `>` here would leave a re-fire landing on + // the tick suppressed for another whole window. + const anchor = new Date(NOW - 24 * 60 * 60 * 1000).toISOString(); + expect( + decideRefire({ status: "cancelled" }, { resolvedAt: null, operatorSuppressedAt: anchor }, cfg(), NOW).kind, + ).toBe("reopen"); + }); + + it("honours a custom window", () => { + const existing = { resolvedAt: null, operatorSuppressedAt: ago(2) }; + expect(decideRefire({ status: "cancelled" }, existing, cfg(1), NOW).kind).toBe("reopen"); + expect(decideRefire({ status: "cancelled" }, existing, cfg(4), NOW).kind).toBe("suppressed"); + }); + + it("never expires when operatorSuppressionHours is 0", () => { + expect( + decideRefire( + { status: "cancelled" }, + { resolvedAt: null, operatorSuppressedAt: ago(24 * 365) }, + cfg(0), + NOW, + ).kind, + ).toBe("suppressed"); + }); + + it("falls back to the default window for nonsense settings", () => { + // A negative or NaN setting must not read as 0 ("mute forever") — that + // would turn a config typo into a silently unpageable alert. + for (const bad of [-5, Number.NaN, Number.POSITIVE_INFINITY]) { + expect( + decideRefire( + { status: "cancelled" }, + { resolvedAt: null, operatorSuppressedAt: ago(25) }, + cfg(bad), + NOW, + ).kind, + ).toBe("reopen"); + } + }); + + it("re-anchors an unparseable anchor instead of muting forever", () => { + expect( + decideRefire( + { status: "cancelled" }, + { resolvedAt: null, operatorSuppressedAt: "garbage" }, + cfg(), + NOW, + ), + ).toEqual({ + kind: "suppressed", + suppressedAt: new Date(NOW).toISOString(), + firstObservation: true, + }); + }); + + it("reports a missing issue", () => { + for (const missing of [null, undefined]) { + expect( + decideRefire(missing, { resolvedAt: ago(1), operatorSuppressedAt: null }, cfg(), NOW), + ).toEqual({ kind: "issue_missing" }); + } + }); + + it("prefers the plugin-resolved re-open over suppression when both could apply", () => { + // A row carrying both a resolve and a stale anchor is a close→re-fire→ + // close→resolve history. The resolve is the more recent fact, so this must + // not be read as an operator mute. + expect( + decideRefire( + { status: "cancelled" }, + { resolvedAt: ago(1), operatorSuppressedAt: ago(2) }, + cfg(), + NOW, + ), + ).toEqual({ kind: "reopen", reason: "plugin_resolved" }); + }); +}); diff --git a/packages/plugins/paperclip-plugin-alertmanager/src/constants.ts b/packages/plugins/paperclip-plugin-alertmanager/src/constants.ts index 4ef60e6191b5..fb23cc833054 100644 --- a/packages/plugins/paperclip-plugin-alertmanager/src/constants.ts +++ b/packages/plugins/paperclip-plugin-alertmanager/src/constants.ts @@ -82,6 +82,17 @@ export const DEFAULT_ESCALATION_DEADLINE_MINUTES: Record = { */ export const DEFAULT_COVER_DEDUP_WINDOW_MINUTES = 120; +/** + * How long an operator-closed issue mutes re-fires of its fingerprint before + * the plugin re-opens it anyway (BLO-24234). + * + * 24h is chosen to outlast a single on-call shift — long enough that closing a + * noisy issue actually buys quiet for the rest of the day, short enough that a + * still-firing alert cannot stay invisible across a handover. Operators who + * want the old unbounded mute can set `operatorSuppressionHours: 0`. + */ +export const DEFAULT_OPERATOR_SUPPRESSION_HOURS = 24; + /** Default owner routes shipped with the bundled Blockcast Alertmanager plugin. */ export const DEFAULT_OWNER_MAP: OwnerMap = { class: { diff --git a/packages/plugins/paperclip-plugin-alertmanager/src/types.ts b/packages/plugins/paperclip-plugin-alertmanager/src/types.ts index 8cd622e73286..4be107f7392e 100644 --- a/packages/plugins/paperclip-plugin-alertmanager/src/types.ts +++ b/packages/plugins/paperclip-plugin-alertmanager/src/types.ts @@ -101,6 +101,23 @@ export interface AlertmanagerPluginConfig { * goal, status, and queue defaults to created issues. */ issueRouteMap?: IssueRouteMap; + /** + * How long (hours) an operator-closed issue suppresses re-fires of its + * fingerprint before the plugin re-opens it anyway (BLO-24234). + * + * Closing an alert issue by hand means "stop nagging me about this", so the + * plugin honours it — but only for a bounded window. Without an expiry the + * suppression is permanent and silent: the fingerprint is muted forever, and + * because Alertmanager fingerprints are `hash(sorted(labels))`, a + * provider-agnostic alert re-uses one fingerprint across every future root + * cause. One operator closing a noisy issue would mute an unrelated outage + * months later. + * + * Defaults to `DEFAULT_OPERATOR_SUPPRESSION_HOURS`. Set to `0` to suppress + * indefinitely (the pre-BLO-24234 behaviour) — only safe for alerts you are + * willing to never hear from again. + */ + operatorSuppressionHours?: number; escalationDeadlineMinutes?: Record; /** * Width (minutes) of the board-cover dedup window (BLO-15982). Concurrent @@ -175,6 +192,17 @@ export interface AlertStateRecord { firstSeenAt: string; lastFiredAt: string; resolvedAt: string | null; + /** + * When the plugin FIRST saw this fingerprint re-fire against an issue that + * an operator (not the plugin) had closed — i.e. terminal status with no + * `resolvedAt` (BLO-24234). Anchors the `operatorSuppressionHours` window. + * + * Cleared whenever the issue is observed open again, so a close/re-open + * cycle restarts the window rather than carrying a stale anchor forward. + * Optional: rows written before BLO-24234 do not have it, and `undefined` + * is treated as "suppression starts now". + */ + operatorSuppressedAt?: string | null; nextEscalationAt?: string | null; escalationAttempt?: number; escalationComplete?: boolean; diff --git a/packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts b/packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts index d94600543990..671c4c503bac 100644 --- a/packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts +++ b/packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts @@ -11,6 +11,7 @@ import { timingSafeEqual } from "node:crypto"; import type { PluginContext, PluginWebhookInput } from "@paperclipai/plugin-sdk"; import { ACCEPTED_SCHEMA_VERSIONS, + DEFAULT_OPERATOR_SUPPRESSION_HOURS, WEBHOOK_KEYS, alertStateRef, legacyInstanceAlertStateRef, @@ -163,6 +164,94 @@ async function readAlertState( return { ref, record: null }; } +/** + * Milliseconds an operator-closed issue suppresses re-fires, or `null` for + * "suppress indefinitely" (`operatorSuppressionHours: 0`, the pre-BLO-24234 + * behaviour). A negative or non-finite setting is treated as unset rather than + * silently disabling suppression in either direction. + */ +function operatorSuppressionMs(config: AlertmanagerPluginConfig): number | null { + const hours = config.operatorSuppressionHours; + const effective = + typeof hours === "number" && Number.isFinite(hours) && hours >= 0 + ? hours + : DEFAULT_OPERATOR_SUPPRESSION_HOURS; + return effective === 0 ? null : effective * 60 * 60 * 1000; +} + +/** + * Decide what a re-fire should do to an issue that already exists for this + * fingerprint. Split out from `handleFiring` so the four decision points the + * incident review asked for are enumerable in one place, and testable without + * driving a whole webhook delivery. + * + * `terminal + resolvedAt` means the plugin closed it when the alert cleared, so + * a re-fire is a genuine recurrence → re-open. `terminal` with no `resolvedAt` + * means a human closed it while the alert was still firing → honour that, but + * only until the suppression window expires (BLO-24234). + */ +type RefireDecision = + | { kind: "refresh" } + | { kind: "reopen"; reason: "plugin_resolved" | "suppression_expired" } + | { kind: "suppressed"; suppressedAt: string; firstObservation: boolean } + | { kind: "issue_missing" }; + +export function decideRefire( + issue: { status: string } | null | undefined, + existing: Pick, + config: AlertmanagerPluginConfig, + nowMs: number, +): RefireDecision { + if (!issue) return { kind: "issue_missing" }; + + const terminal = issue.status === "done" || issue.status === "cancelled"; + if (!terminal) return { kind: "refresh" }; + if (existing.resolvedAt) return { kind: "reopen", reason: "plugin_resolved" }; + + // Operator-closed. Anchor the window on the first re-fire we see against the + // closed issue — not on the close itself, which the plugin never observes. + const suppressedAt = existing.operatorSuppressedAt ?? new Date(nowMs).toISOString(); + const firstObservation = !existing.operatorSuppressedAt; + const windowMs = operatorSuppressionMs(config); + if (windowMs === null) return { kind: "suppressed", suppressedAt, firstObservation }; + + const anchorMs = Date.parse(suppressedAt); + // An unparseable anchor (hand-edited or corrupted state row) must not mute the + // alert forever — re-anchor to now and keep suppressing for one more window. + if (!Number.isFinite(anchorMs)) { + return { + kind: "suppressed", + suppressedAt: new Date(nowMs).toISOString(), + firstObservation: true, + }; + } + if (nowMs - anchorMs >= windowMs) { + return { kind: "reopen", reason: "suppression_expired" }; + } + return { kind: "suppressed", suppressedAt, firstObservation }; +} + +/** + * Human-readable suppression window for log lines and the re-open comment. + */ +function operatorSuppressionHoursLabel(config: AlertmanagerPluginConfig): string { + const ms = operatorSuppressionMs(config); + if (ms === null) return "indefinite"; + return `${ms / (60 * 60 * 1000)}h`; +} + +/** When the current suppression window runs out, for operator-facing logs. */ +function suppressionExpiryLabel( + suppressedAt: string, + config: AlertmanagerPluginConfig, +): string { + const ms = operatorSuppressionMs(config); + if (ms === null) return "never (operatorSuppressionHours=0)"; + const anchorMs = Date.parse(suppressedAt); + if (!Number.isFinite(anchorMs)) return "unknown (unparseable suppression anchor)"; + return new Date(anchorMs + ms).toISOString(); +} + /** * §8.1 — first time we see a fingerprint, create an issue. On re-fire, just * bump `lastFiredAt` and re-emit the firing event. On re-fire after a manual @@ -206,55 +295,140 @@ export async function handleFiring( if (existing && existing.paperclipIssueId) { // Re-fire: refresh body (drill-in URLs may carry a fresh time range) and - // re-open if the plugin previously auto-cancelled it on resolve. + // re-open if the plugin previously auto-cancelled it on resolve, or if an + // operator's close has aged past the suppression window (BLO-24234). const newDescription = buildIssueDescription(alert); + // Carried out of the try so the state write below records what actually + // happened. A decision the RPC then failed to apply must not be persisted + // as applied — otherwise a transient issues.update outage would bank the + // suppression anchor (or clear it) on the strength of a call that never + // landed, and the next re-fire would reason from a fiction. + let decision: RefireDecision = { kind: "issue_missing" }; + let decisionApplied = false; try { const issue = await ctx.issues.get( existing.paperclipIssueId, existing.paperclipCompanyId, ); - if ( - issue && - (issue.status === "done" || issue.status === "cancelled") && - existing.resolvedAt - ) { + decision = decideRefire(issue, existing, config, Date.now()); + + if (decision.kind === "reopen") { await ctx.issues.update( existing.paperclipIssueId, { status: "todo", description: newDescription }, existing.paperclipCompanyId, ); + if (decision.reason === "suppression_expired") { + // Say why the close did not stick, on the issue itself — an operator + // who closed this yesterday needs to know it re-opened because the + // alert never stopped firing, not because something ignored them. + try { + await ctx.issues.createComment( + existing.paperclipIssueId, + `Re-opened by paperclip-plugin-alertmanager: this issue was closed by hand, but \`${alertname}\` has kept firing past the ${operatorSuppressionHoursLabel(config)} suppression window. Closing it again will suppress it for another window; silence the alert rule itself if it should stop paging.`, + existing.paperclipCompanyId, + ); + } catch (commentErr) { + // The re-open is the load-bearing half and has already landed. + ctx.logger.warn( + `Re-opened issue ${existing.paperclipIssueId} after suppression expiry but could not post the explanatory comment: ${String(commentErr)}`, + ); + } + await ctx.metrics.write("alertmanager.firing.suppression_expired", 1, { + alertname, + severity, + }); + } await ctx.metrics.write("alertmanager.firing.reopened", 1, { alertname, severity, }); - } else if (issue && issue.status !== "done" && issue.status !== "cancelled") { + } else if (decision.kind === "refresh") { await ctx.issues.update( existing.paperclipIssueId, { description: newDescription }, existing.paperclipCompanyId, ); + } else if (decision.kind === "suppressed") { + // The whole point of BLO-24234: this path used to be entirely silent, + // emitting only `firing.deduped` — indistinguishable from a healthy + // re-fire against an open issue. A muted fingerprint must be visible + // as muted, every time it fires, or nobody can tell that a delivered + // page produced no actionable artifact. + if (decision.firstObservation) { + ctx.logger.warn( + `Alert ${alertname} (${alert.fingerprint}) re-fired against operator-closed issue ${existing.paperclipIssueId}; suppressing re-open until ${suppressionExpiryLabel(decision.suppressedAt, config)}`, + ); + } else { + ctx.logger.info( + `Alert ${alertname} (${alert.fingerprint}) still suppressed by operator close of issue ${existing.paperclipIssueId} (until ${suppressionExpiryLabel(decision.suppressedAt, config)})`, + ); + } + await ctx.metrics.write("alertmanager.firing.suppressed", 1, { + alertname, + severity, + }); + } else { + // `issues.get` returned nothing — the issue was hard-deleted out from + // under the state row. Previously this fell through both branches in + // silence; say so, since the fingerprint is now tracking a ghost. + ctx.logger.warn( + `Alert ${alertname} (${alert.fingerprint}) re-fired but its tracked issue ${existing.paperclipIssueId} could not be read; leaving state intact`, + ); + await ctx.metrics.write("alertmanager.firing.issue_missing", 1, { + alertname, + severity, + }); } + decisionApplied = true; } catch (err) { ctx.logger.warn( `Failed to re-sync existing issue ${existing.paperclipIssueId} on re-fire: ${String(err)}`, ); } + // Ladder restart keeps its original trigger — the alert going + // resolved → firing — which is independent of the issue's status: an + // operator may have re-opened the issue by hand, in which case the branch + // above is a plain `refresh` but `handleResolved` has still left + // `nextEscalationAt` null and `escalationComplete` true. Gating this on the + // re-open would silently disarm escalation for exactly that case. + // + // A suppression-expiry re-open is the one new trigger: the ladder has been + // frozen for the whole suppression window, so the now-visible issue needs a + // live deadline or it will never page anyone. + const suppressionExpiryReopen = + decisionApplied && + decision.kind === "reopen" && + decision.reason === "suppression_expired"; + const ladderRestart = Boolean(existing.resolvedAt) || suppressionExpiryReopen; + // Only a decision we actually applied may move the anchor. `issue_missing` + // preserves it: the issue was unreadable, so we learned nothing about + // whether the operator's close still stands, and dropping the anchor would + // restart the whole window on the next readable re-fire. + const suppressionAnchor = + !decisionApplied || decision.kind === "issue_missing" + ? (existing.operatorSuppressedAt ?? null) + : decision.kind === "suppressed" + ? decision.suppressedAt + : null; + const updated: AlertStateRecord = { ...existing, alertname, severity, lastFiredAt: nowIso, resolvedAt: null, - nextEscalationAt: existing.resolvedAt + operatorSuppressedAt: suppressionAnchor, + nextEscalationAt: ladderRestart ? (() => { const delay = escalationDeadlineMs(alert, config); return delay === null ? null : new Date(Date.now() + delay).toISOString(); })() : existing.nextEscalationAt, - escalationAttempt: existing.resolvedAt ? 0 : existing.escalationAttempt, - escalationComplete: existing.resolvedAt ? false : existing.escalationComplete, - escalationIntervalMs: existing.resolvedAt + escalationAttempt: ladderRestart ? 0 : existing.escalationAttempt, + escalationComplete: ladderRestart ? false : existing.escalationComplete, + escalationIntervalMs: ladderRestart ? escalationDeadlineMs(alert, config) : (existing.escalationIntervalMs ?? escalationDeadlineMs(alert, config)), }; @@ -516,6 +690,14 @@ async function recoverStateFromIssue( }); const issue = matches[0]; if (!issue) return null; + // A terminal issue is deliberately NOT adopted here, which means a lost state + // row plus a closed issue files a fresh one rather than reviving the old. + // That diverges from the state-present path (which suppresses per + // BLO-24234) — on purpose: after a state loss the plugin cannot tell whether + // the close was its own resolve or an operator's, and the safe failure mode + // for a paging system is a visible duplicate, not a silent mute. Do not + // "unify" this branch by returning the terminal issue; that would let a state + // loss inherit a suppression nobody chose. if (issue.status === "done" || issue.status === "cancelled") return null; return {