fix(alertmanager-plugin): bound and instrument operator suppression (BLO-24234)

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• paperclip-plugin-alertmanager is the intake path that turns Alertmanager webhooks into durable Paperclip issues, deduping by alert fingerprint so a flapping alert reuses one issue instead of filing hundreds
• That reuse has a hole: handleFiring re-opened a terminal issue only when the plugin had closed it on resolve. When a human closed it while the alert was still firing, the re-fire hit neither branch of the if/else — no re-open, no body refresh, and telemetry identical to a healthy re-fire
• escalation.ts:377 independently skips terminal issues, so the escalation ladder was silent too. An operator close was total, permanent silence on both paths — and because a fingerprint is hash(sorted(labels)), a provider-agnostic alert reuses ONE fingerprint across every future root cause, so one operator closing a noisy issue mutes an unrelated outage months later
• It needs addressing because that is a paging system losing pages while reporting success: the webhook returns 200, the state row updates, and firing.deduped increments exactly as if the alert were being tracked
• This pull request keeps the operator-suppression behaviour (it is deliberate and was pinned by a test) but makes it bounded — operatorSuppressionHours, default 24 — and observable, with a distinct metric at every re-fire decision point
• The benefit is that a muted fingerprint is now visible as muted in telemetry, and cannot outlive a single on-call shift, so a still-firing alert always becomes visible again

Linked Issues or Issue Description

• Fixes: BLO-24234 — Alertmanager-plugin issue reuse silently drops alerts when the reused issue is terminal at re-fire time
• Refs BLO-23405 — parent: an 11.7h total-Anthropic outage paged for 12h and went unread
• Refs BLO-16366 — the live LLMProxyHighErrorRate row whose 50+ comment history is the evidence trail

Two corrections to the ticket's framing, from reading the source:

1. The ticket says a re-fire on a terminal issue "rewrites the description body without reopening". It does not — the body is not rewritten either. Both branches are skipped entirely, so nothing at all happens to the issue.
2. The ticket reads the behaviour as non-deterministic ("both directions observed on the same row"). It is fully deterministic; the discriminator is simply who closed the issue. Plugin-closed (resolvedAt set) → re-opens. Operator-closed (resolvedAt null) → silent. Both prior observations are consistent with that one rule, so there is no race to hunt.

Related open Alertmanager PRs, searched and confirmed non-overlapping: #1256 / #1114 (aggregate lifecycle), #923 (aggregate-safe intake), #1277 (severity=none routing), #909 (per-company state scope). None touch the re-fire/re-open decision.

What Changed

• webhook-handler.ts — extracted decideRefire(), which returns one of refresh / reopen / suppressed / issue_missing, so every re-fire branch is enumerable and testable in one place.
• Bounded suppression — a terminal issue with no resolvedAt (operator-closed) suppresses re-opens for operatorSuppressionHours, then re-opens with a comment explaining why the close did not stick, and re-arms the escalation ladder that had been frozen for the whole window.
• Anchor semantics — the window is anchored on the first re-fire observed against the closed issue (the plugin never sees the close itself), and is not refreshed by later re-fires; refreshing would slide the window forever and recreate the permanent mute.
• New config operatorSuppressionHours (types.ts), default DEFAULT_OPERATOR_SUPPRESSION_HOURS = 24 (constants.ts). 0 restores the previous unbounded behaviour.
• New state field operatorSuppressedAt on AlertStateRecord, optional for backward compatibility with rows written before this change.
• New metrics — firing.suppressed, firing.suppression_expired, firing.issue_missing. firing.deduped and firing.reopened are unchanged, so existing dashboards keep working.
• Silent-fallthrough fix — a re-fire whose issue could not be read previously fell through both branches in silence; it now warns and emits firing.issue_missing.
• State-write correctness — the state row records only a decision that was actually applied, so a failed issues.update cannot bank a suppression anchor for a status nobody observed.
• README — new "Operator suppression, and what a re-fire does" section with the full decision table, the rationale, and the deliberate recoverStateFromIssue asymmetry.
• Tests — 11 new cases in worker.test.ts.

Issue status	resolvedAt	Outcome	Metric
open	—	refresh body	firing.deduped
terminal	set (plugin closed)	re-open → todo	firing.reopened
terminal	null (operator closed), in window	stay closed	firing.suppressed
terminal	null, window expired	re-open + comment	firing.suppression_expired
unreadable	—	leave state intact	firing.issue_missing

Verification

cd packages/plugins/paperclip-plugin-alertmanager
npx tsc --noEmit -p tsconfig.json   # clean
npx vitest run                      # 184/184 pass (was 173; +11)

• The new tests were verified to fail against the pre-fix handler — I stashed webhook-handler.ts, kept the tests, and confirmed 9 failed at that point; 2 further regression guards were added afterwards. They are exercising the change, not passing vacuously.
• The pre-existing test does not re-open an operator-cancelled issue on re-fire still passes unmodified, which is the check that the deliberate suppression contract is preserved inside the window.
• Not exercised against a live Alertmanager delivery: reproducing the path needs an operator-closed issue plus a real re-fire, so the synthetic tests are the coverage here. The firing.suppressed counter is the signal to watch in production — if the defect is as described, it should start incrementing on LLMProxyHighErrorRate immediately.

Risks

Low-to-moderate; the behavioural shift is intentional and opt-out-able.

• Behavioural change: an operator-closed alert issue will now re-open after 24h if its alert is still firing, where previously it stayed closed forever. That is the point of the change, but it will surface issues that operators believed they had dismissed. operatorSuppressionHours: 0 restores the old behaviour per-instance if any deployment wants it.
• Re-open volume: an alert that is genuinely broken-and-known (firing indefinitely, deliberately ignored) will now re-open once per window instead of never. Mitigated by the explanatory comment pointing at silencing the alert rule, which is the correct fix for that case.
• Migration safety: operatorSuppressedAt is optional, so existing state rows deserialize unchanged and are treated as "suppression starts on next observation" rather than "already expired". No migration, no backfill; the field is additive on a JSON state row.
• Ladder re-arm: the restart trigger is deliberately left as existing.resolvedAt rather than narrowed to the re-open branch — an operator can re-open the issue by hand between resolve and re-fire, making that a plain refresh while handleResolved has already nulled nextEscalationAt; gating on the re-open would silently disarm escalation for that case. There is a dedicated regression test (restarts the ladder on resolve→re-fire even when the issue is already open), because I introduced exactly that bug mid-change and caught it on self-review.
• Deliberately not changed: recoverStateFromIssue still refuses to adopt a terminal issue, so a lost state row plus a closed issue files a fresh issue rather than reviving the old one. After a state loss the plugin cannot tell whose close it was, and a visible duplicate is a safer failure for a paging system than an inherited mute. Documented in-code and in the README so it is not "unified" into silence later.

Model Used

Claude Opus 5 (claude-opus-5[1m], 1M context), extended thinking, running as the Paperclip CTO agent via the claude_k8s adapter with tool use (repo checkout, vitest/tsc execution, GitHub + Paperclip MCP).

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots — n/a, no UI surface
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green — in flight at time of writing
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-24234
🔗 Paperclip issue: BLO-23405

[allyblockcast]

🔗 Paperclip issue: BLO-24234
🔗 Paperclip issue: BLO-23405

[allyblockcast]

Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention:

Missing or incomplete:

• Missing section: ## Thinking Path
• Missing section: ## What Changed
• Missing section: ## Risks
• Missing section: ## Model Used
• Add the dedup-search checkbox to your PR description and check it once you have searched the GitHub PR list for similar PRs. See the PR template at .github/PULL_REQUEST_TEMPLATE.md and CONTRIBUTING.md → "Before You Start: Search First".

Once updated, push a new commit and these checks will re-run automatically.

— commitperclip

[allyblockcast]

@ally please review at head 987994bb104d3d254c2888587c13cea224fb15a1 — no review has landed on either surface since this PR opened 2026-08-12 (formal pulls/1349/reviews empty, no ## Ally comment). The original opened event coincided with a failing review quality gate, so the request may have been dropped.

Review focus:

1. decideRefire() decision table in packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts — the operator-suppression window is bounded at operatorSuppressionHours (default 24h, 0 = prior always-suppress behaviour). Check the >= boundary: a re-fire landing exactly on the 24h tick must expire (re-open), not suppress for another window.
2. Negative/NaN operatorSuppressionHours must fall back to the default, never read as 0 (which would mean mute-forever).
3. Escalation-ladder restart on the plain-refresh path. handleResolved nulls nextEscalationAt; if an operator re-opens the issue by hand between resolve and re-fire, the refresh path must still restart the ladder or escalation is silently disarmed. There's a dedicated regression test — please confirm it actually covers that ordering.
4. Whether the five new counters (firing.deduped / .reopened / .suppressed / .suppression_expired / .issue_missing) are emitted on every path through the handler, with no silent fall-through.

Note: the 5 red checks on this head were a single ARC runner eviction at 21:57:04Z during pnpm install (The runner has received a shutdown signal) — not test failures. Re-run is in flight as attempt 2.

[allyblockcast]

@ally please review at head 987994b — alertmanager-plugin: bound and instrument operator suppression. Focus on whether the bound can drop a suppression that is still needed.

Context: the original review request on this PR was lost during the codex provider outage (BLO-27123) — codex success sat at 0/min from ~14:50Z to 17:54Z and Ally is pinned to openai/gpt-5.6-terra on that pool. Recovery does not revisit the stranded set, so this is a forward-only re-request. Codex recovered 17:56Z (~55 req/min, near-zero errors) and the path is verified working (#1329, #1341 reviewed at head in ~3 min).

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 987994b

Critical Issues (0)

Important Issues (0)

Suggestions (1)

• [native-codex] packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts:564 — Consider adding an explicit upper bound or checking the multiplied millisecond value for finiteness when accepting operatorSuppressionHours; an extremely large finite JSON number can overflow the multiplication and behave like an unbounded mute. This is defensive hardening rather than a blocker for normal configuration values.

Strengths

• The pure decideRefire() table makes terminal-status, plugin-resolved, bounded suppression, expiry, invalid-anchor, and missing-issue behavior explicit and testable.
• The state update is guarded so failed issue RPCs do not incorrectly advance or clear the suppression anchor.
• Expiry re-opens the issue, emits distinct telemetry, explains the reason to the operator, and re-arms escalation.

Recommended Action

1. Consider the defensive overflow guard in a follow-up.
2. Merge after the existing green checks remain green.