fix: alarm channels (#3491)

config.template.jsonc

@@ -292,8 +292,12 @@
             // Optional; defaults to the channel the webhook was created for.
             "channel": "#alerts",
             "username": "puter-alarms",
-            // Lowest severity posted. Default "info" (everything).
+            // Severity window posted to Slack. The ceiling defaults to
+            // "info" when PagerDuty is configured — what pages belongs in
+            // the pager, not in chat — and to "critical" when Slack is the
+            // only transport.
             "minSeverity": "info",
+            "maxSeverity": "info",
             // Don't repost the same alarm id within this window. The first
             // occurrence always posts; the next post that gets through
             // reports how many occurrences piled up. 0 disables throttling.

doc/alarms.md

@@ -15,15 +15,17 @@ this.clients.alarm.create(
 
 ## Severity is the routing decision
 
-| Severity   | Meaning                                                | Goes to            |
-| ---------- | ------------------------------------------------------ | ------------------ |
-| `critical` | An unhandled server error. Someone gets woken up.      | Pager + chat       |
-| `error`    | Same urgency as critical; prefer one of the other two. | Pager + chat       |
-| `warning`  | Worth a look today. Nobody is paged.                   | Pager (low) + chat |
-| `info`     | A record of something expected-but-notable.            | Chat only          |
+| Severity   | Meaning                                                | Goes to     |
+| ---------- | ------------------------------------------------------ | ----------- |
+| `critical` | An unhandled server error. Someone gets woken up.      | Pager       |
+| `error`    | Same urgency as critical; prefer one of the other two. | Pager       |
+| `warning`  | Worth a look today. Nobody is paged.                   | Pager (low) |
+| `info`     | A record of something expected-but-notable.            | Chat        |
 
-Each transport declares the lowest severity it accepts, so the value a call
-site passes is what decides where the alarm lands. The bar for `critical` is
+Each transport declares the severity window it accepts, so the value a call
+site passes is what decides where the alarm lands. The two windows don't
+overlap by default: anything that pages lives in the paging system, and chat
+is the record of what didn't. The bar for `critical` is
 deliberately high: an unhandled 5xx out of the HTTP error handler is the main
 thing that still pages. Anything a human can look at tomorrow is `warning`,
 and anything that's just worth recording is `info`.
@@ -42,6 +44,29 @@ An extension whose signals are all one tier can default its own local
 `raiseAlarm` helper to that tier instead of repeating it at every call site —
 see [extensions/cronMonitor](../../../extensions/cronMonitor/index.js).
 
+## One incident per occurrence, unless you say otherwise
+
+Alarms always de-dupe *in process* — repeats of an id bump its occurrence
+count rather than creating a second alarm. What that means for the pager is a
+separate decision, and by default every occurrence opens its own PagerDuty
+incident: two failed scans an hour apart are two things that happened, and
+closing one shouldn't hide the other.
+
+Pass `{ dedup: true }` as a fifth argument when repeats of the id really are
+one recurring fault, and they collapse onto a single incident carrying the
+occurrence count:
+
+```ts
+this.clients.alarm.create(alarmId, message, fields, 'critical', {
+    dedup: true,
+});
+```
+
+The HTTP error handler uses it: its id is route + error signature, so a hot
+loop of the same crash is one incident with N occurrences instead of N pages.
+Reach for it anywhere else only when the id is that specific — otherwise a
+per-request alarm can flood the pager.
+
 ## Configuration
 
 Everything lives under `pager` in config (see
@@ -59,11 +84,16 @@ alarms to the console.
         "webhookUrl": "…",
         "channel": "#alerts",
         "minSeverity": "info",
+        "maxSeverity": "info",
         "repeatThrottleMs": 900000,
     },
 }
 ```
 
+Slack's `maxSeverity` defaults to `info` whenever PagerDuty is configured, and
+to `critical` when it isn't — a node with only a webhook still sees
+everything. Raise it to have chat mirror the paging tiers as well.
+
 ### Retiering without a deploy
 
 `severityOverrides` is the escape hatch for an alarm that turns out to be

src/backend/clients/alarm/AlarmClient.test.ts

@@ -13,13 +13,14 @@ const makeClient = (pager: IPagerConfig = {}) =>
 const capture = (
     client: AlarmClient,
     minSeverity?: 'critical' | 'error' | 'warning' | 'info',
+    maxSeverity?: 'critical' | 'error' | 'warning' | 'info',
 ) => {
     const seen: AlertPayload[] = [];
     client.addAlertHandler(
         async (alert) => {
             seen.push(alert);
         },
-        { name: 'capture', minSeverity },
+        { name: 'capture', minSeverity, maxSeverity },
     );
     return seen;
 };
@@ -62,6 +63,16 @@ describe('AlarmClient severity routing', () => {
         expect(chat).toHaveLength(1);
     });
 
+    it('skips handlers whose ceiling the alarm exceeds', () => {
+        const client = makeClient();
+        const chat = capture(client, 'info', 'info');
+
+        client.create('rate-limit', 'a user hit a limit', {}, 'info');
+        client.create('outage', 'everything is on fire', {}, 'critical');
+
+        expect(chat.map((alert) => alert.id)).toEqual(['rate-limit']);
+    });
+
     it('retiers an alarm from config', () => {
         const client = makeClient({
             severityOverrides: { 'noisy:*': 'info' },
@@ -181,7 +192,7 @@ describe('AlarmClient transport registration', () => {
         pdEvent.mockClear();
     });
 
-    it('keeps info alarms out of PagerDuty but sends them to Slack', async () => {
+    it('splits info to Slack and everything above it to PagerDuty', async () => {
         const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }));
         vi.stubGlobal('fetch', fetchMock);
 
@@ -197,11 +208,93 @@ describe('AlarmClient transport registration', () => {
 
         client.create('loud:thing', 'paging', {}, 'critical');
         await vi.waitFor(() => expect(pdEvent).toHaveBeenCalledTimes(1));
-        expect(fetchMock).toHaveBeenCalledTimes(2);
+        // The pager has it; chat doesn't repeat it.
+        expect(fetchMock).toHaveBeenCalledTimes(1);
 
         vi.unstubAllGlobals();
     });
 
+    it('sends every severity to Slack when there is no pager', async () => {
+        const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }));
+        vi.stubGlobal('fetch', fetchMock);
+
+        const client = makeClient({
+            slack: { enabled: true, webhookUrl: 'https://hooks.example/abc' },
+        });
+        await client.onServerStart();
+
+        client.create(
+            'loud:thing',
+            'nowhere else to send this',
+            {},
+            'critical',
+        );
+        await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
+
+        vi.unstubAllGlobals();
+    });
+
+    it('lets config widen the Slack ceiling back out', async () => {
+        const fetchMock = vi.fn(async () => ({ ok: true, status: 200 }));
+        vi.stubGlobal('fetch', fetchMock);
+
+        const client = makeClient({
+            pagerduty: { enabled: true, routingKey: 'rk' },
+            slack: {
+                enabled: true,
+                webhookUrl: 'https://hooks.example/abc',
+                maxSeverity: 'critical',
+            },
+        });
+        await client.onServerStart();
+
+        client.create('loud:thing', 'paging', {}, 'critical');
+        await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
+
+        vi.unstubAllGlobals();
+    });
+
+    it('gives each occurrence its own PagerDuty incident by default', async () => {
+        const client = makeClient({
+            pagerduty: { enabled: true, routingKey: 'rk' },
+        });
+        await client.onServerStart();
+
+        client.create('scan:failed', 'first', {}, 'warning');
+        client.create('scan:failed', 'second', {}, 'warning');
+        await vi.waitFor(() => expect(pdEvent).toHaveBeenCalledTimes(2));
+
+        const keys = pdEvent.mock.calls.map(
+            ([arg]: [{ data: { dedup_key: string } }]) => arg.data.dedup_key,
+        );
+        expect(keys[0]).not.toBe(keys[1]);
+    });
+
+    it('collapses repeats of a dedup alarm onto one incident', async () => {
+        const client = makeClient({
+            pagerduty: { enabled: true, routingKey: 'rk' },
+        });
+        await client.onServerStart();
+
+        const raise = () =>
+            client.create(
+                'http_500:POST:/notif/mark-ack:deadlock',
+                'HTTP 500 on POST /notif/mark-ack: deadlock',
+                {},
+                'critical',
+                { dedup: true },
+            );
+        raise();
+        raise();
+        await vi.waitFor(() => expect(pdEvent).toHaveBeenCalledTimes(2));
+
+        const keys = pdEvent.mock.calls.map(
+            ([arg]: [{ data: { dedup_key: string } }]) => arg.data.dedup_key,
+        );
+        expect(keys[0]).toBe('http_500:POST:/notif/mark-ack:deadlock');
+        expect(keys[1]).toBe(keys[0]);
+    });
+
     it('skips transports that are enabled but not configured', async () => {
         const client = makeClient({
             pagerduty: { enabled: true },

src/backend/clients/alarm/AlarmClient.ts

@@ -22,11 +22,16 @@ import { inspect } from 'node:util';
 import { createHash } from 'node:crypto';
 import type { IConfig, PagerSeverity, SeverityRule } from '../../types';
 import { PuterClient } from '../types';
-import { meetsMinSeverity, resolveSeverityOverride } from './severity';
+import {
+    meetsMinSeverity,
+    resolveSeverityOverride,
+    withinMaxSeverity,
+} from './severity';
 import { createSlackAlertHandler } from './slack';
 import type {
     Alarm,
     AlarmFields,
+    AlarmOptions,
     AlertHandler,
     AlertPayload,
     KnownErrorRule,
@@ -35,6 +40,7 @@ import type {
 export type {
     Alarm,
     AlarmFields,
+    AlarmOptions,
     AlertHandler,
     AlertPayload,
     KnownErrorRule,
@@ -47,6 +53,8 @@ interface RegisteredHandler {
     name: string;
     /** Lowest severity this transport accepts. */
     minSeverity: PagerSeverity;
+    /** Highest severity this transport accepts. */
+    maxSeverity: PagerSeverity;
     handler: AlertHandler;
 }
 
@@ -54,6 +62,8 @@ interface RegisteredHandler {
 const FALLBACK_SEVERITY: PagerSeverity = 'critical';
 /** Keeps `info` alarms out of the paging system unless config says otherwise. */
 const DEFAULT_PAGERDUTY_MIN_SEVERITY: PagerSeverity = 'warning';
+/** Slack's ceiling once a pager exists: chat gets what doesn't page. */
+const DEFAULT_SLACK_MAX_SEVERITY_WITH_PAGER: PagerSeverity = 'info';
 
 // -- Helpers ----------------------------------------------------------
 
@@ -153,7 +163,7 @@ function cleanFields(fields: AlarmFields): Record<string, string> {
 /**
  * Manages system alarms and routes them to alert transports by severity.
  *
- * Severity is the routing decision: each transport declares the lowest severity
+ * Severity is the routing decision: each transport declares the severity window
  * it accepts, so `critical` pages on-call while `info` only reaches the chat
  * channel. Config can retier or mute any alarm id after the fact — see
  * `pager.severityOverrides` in {@link IConfig}.
@@ -173,8 +183,8 @@ export class AlarmClient extends PuterClient {
     // -- Lifecycle ----------------------------------------------------
 
     override async onServerStart(): Promise<void> {
-        this.registerPagerDuty();
-        this.registerSlack();
+        const paging = this.registerPagerDuty();
+        this.registerSlack({ paging });
     }
 
     override onServerPrepareShutdown(): void {
@@ -183,16 +193,17 @@ export class AlarmClient extends PuterClient {
         console.log('[alarm] entering drain mode — suppressing new alarms');
     }
 
-    private registerPagerDuty(): void {
+    /** @returns Whether a working PagerDuty transport was registered. */
+    private registerPagerDuty(): boolean {
         const pagerDutyConf = this.config.pager?.pagerduty;
-        if (!pagerDutyConf?.enabled) return;
+        if (!pagerDutyConf?.enabled) return false;
 
         const routingKey = pagerDutyConf.routingKey;
         if (!routingKey) {
             console.warn(
                 '[alarm] PagerDuty enabled but no routingKey configured',
             );
-            return;
+            return false;
         }
 
         const serverId = this.config.serverId;
@@ -205,7 +216,7 @@ export class AlarmClient extends PuterClient {
                     data: {
                         routing_key: routingKey,
                         event_action: 'trigger',
-                        dedup_key: alert.id,
+                        dedup_key: alert.dedupKey,
                         payload: {
                             summary: alert.message,
                             source: alert.source,
@@ -224,9 +235,10 @@ export class AlarmClient extends PuterClient {
         console.log(
             `[alarm] PagerDuty handler registered (min severity: ${minSeverity})`,
         );
+        return true;
     }
 
-    private registerSlack(): void {
+    private registerSlack({ paging }: { paging: boolean }): void {
         const slackConf = this.config.pager?.slack;
         if (!slackConf?.enabled) return;
 
@@ -236,16 +248,23 @@ export class AlarmClient extends PuterClient {
         }
 
         const minSeverity = slackConf.minSeverity ?? 'info';
+        // With a pager taking everything from `warning` up, chat is where the
+        // rest is recorded — reposting the paging tiers there only trains
+        // people to skim the channel. Without one, Slack is the only place an
+        // alarm can land, so it takes all of them.
+        const maxSeverity =
+            slackConf.maxSeverity ??
+            (paging ? DEFAULT_SLACK_MAX_SEVERITY_WITH_PAGER : 'critical');
 
         this.addAlertHandler(
             createSlackAlertHandler(slackConf, {
                 serverId: this.config.serverId,
             }),
-            { name: 'slack', minSeverity },
+            { name: 'slack', minSeverity, maxSeverity },
         );
 
         console.log(
-            `[alarm] Slack handler registered (min severity: ${minSeverity})`,
+            `[alarm] Slack handler registered (severity ${minSeverity}..${maxSeverity})`,
         );
     }
 
@@ -265,12 +284,16 @@ export class AlarmClient extends PuterClient {
      * Omit it to take `pager.defaultSeverity` (itself defaulting to
      * 'critical'). Operators can retier or mute any alarm id from config
      * afterwards, so the value here is the starting point, not the last word.
+     *
+     * Pass `{ dedup: true }` when repeats of this id are one recurring fault
+     * that should collapse into a single incident — see {@link AlarmOptions}.
      */
     create(
         id: string,
         message: string,
         fields: AlarmFields = {},
         severity?: PagerSeverity,
+        opts: AlarmOptions = {},
     ): void {
         if (this.draining) {
             if (!this.drainLogged) {
@@ -294,6 +317,7 @@ export class AlarmClient extends PuterClient {
             message,
             fields,
             severity,
+            dedup: opts.dedup,
             started: Date.now(),
             // `recordOccurrence` below stamps the first occurrence; seeding one
             // here too would report every alarm as one occurrence ahead.
@@ -325,16 +349,22 @@ export class AlarmClient extends PuterClient {
 
     /**
      * Register an additional alert handler. Handlers are called for every alarm
-     * that isn't suppressed by a known-error rule or muted by config, and that
-     * meets the handler's own `minSeverity` (default: everything).
+     * that isn't suppressed by a known-error rule or muted by config, and whose
+     * severity falls inside the handler's own `minSeverity`..`maxSeverity`
+     * window (default: everything).
      */
     addAlertHandler(
         handler: AlertHandler,
-        opts: { name?: string; minSeverity?: PagerSeverity } = {},
+        opts: {
+            name?: string;
+            minSeverity?: PagerSeverity;
+            maxSeverity?: PagerSeverity;
+        } = {},
     ): void {
         this.alertHandlers.push({
             name: opts.name ?? `handler-${this.alertHandlers.length}`,
             minSeverity: opts.minSeverity ?? 'info',
+            maxSeverity: opts.maxSeverity ?? 'critical',
             handler,
         });
     }
@@ -448,8 +478,17 @@ export class AlarmClient extends PuterClient {
         const fieldsClean = cleanFields(alarm.fields);
         const repeatCount = alarm.timestamps.length;
 
+        const id = alarm.id || 'something-bad';
+
         const payload: AlertPayload = {
-            id: alarm.id || 'something-bad',
+            id,
+            // A de-duplicating transport should fold repeats of a recurring
+            // fault into one incident, but everything else is a fresh event
+            // each time it fires — the start time keeps occurrence numbering
+            // from colliding with an incident left open by an earlier boot.
+            dedupKey: alarm.dedup
+                ? id
+                : `${id}#${alarm.started}.${repeatCount}`,
             shortId: alarm.shortId,
             message: alarm.message || alarm.id || 'something bad happened',
             source: 'alarm',
@@ -465,8 +504,10 @@ export class AlarmClient extends PuterClient {
             },
         };
 
-        for (const { name, minSeverity, handler } of this.alertHandlers) {
+        for (const { name, minSeverity, maxSeverity, handler } of this
+            .alertHandlers) {
             if (!meetsMinSeverity(resolved, minSeverity)) continue;
+            if (!withinMaxSeverity(resolved, maxSeverity)) continue;
             handler(payload).catch((err) => {
                 console.error(
                     `[alarm] ${name} alert handler failed: ${err?.message}`,

src/backend/clients/alarm/severity.ts

@@ -41,6 +41,18 @@ export function meetsMinSeverity(
     return RANK[severity] >= RANK[min];
 }
 
+/**
+ * True when `severity` is quiet enough for a transport whose ceiling is `max`.
+ * A ceiling is what keeps a chat transport from repeating everything the pager
+ * already delivered.
+ */
+export function withinMaxSeverity(
+    severity: PagerSeverity,
+    max: PagerSeverity,
+): boolean {
+    return RANK[severity] <= RANK[max];
+}
+
 /**
  * Look up the operator override for an alarm id. Exact ids win over prefix
  * patterns (`cronMonitor:*`); among patterns the longest prefix wins, so a

src/backend/clients/alarm/slack.test.ts

@@ -4,6 +4,7 @@ import type { AlertPayload } from './types';
 
 const alert = (over: Partial<AlertPayload> = {}): AlertPayload => ({
     id: 'cronMonitor:high_aiLogEntries',
+    dedupKey: 'cronMonitor:high_aiLogEntries#1.1',
     shortId: 'amber-delta-fox',
     message: 'High AI log entries: 1200 in the last 10 minutes',
     source: 'alarm',

src/backend/clients/alarm/types.ts

@@ -30,7 +30,18 @@ export interface AlarmOccurrence {
     timestamp: number;
 }
 
-export interface Alarm {
+export interface AlarmOptions {
+    /**
+     * Collapse repeats of this alarm id into a single incident on transports
+     * that de-duplicate (PagerDuty). Only right when the id already pins down
+     * one recurring fault — an uncaught error on a route, say — so that N
+     * occurrences really are one thing to fix. Off by default: every other
+     * alarm raises its own incident per occurrence.
+     */
+    dedup?: boolean;
+}
+
+export interface Alarm extends AlarmOptions {
     id: string;
     shortId: string;
     message: string;
@@ -47,6 +58,11 @@ export interface Alarm {
 
 export interface AlertPayload {
     id: string;
+    /**
+     * Key a de-duplicating transport groups by. Equal to `id` for alarms raised
+     * with `dedup`, and unique per occurrence for everything else.
+     */
+    dedupKey: string;
     /** Readable slug for the same alarm, for humans quoting it back. */
     shortId: string;
     message: string;

src/backend/server.ts

@@ -781,6 +781,9 @@ export class PuterServer {
                         // An unhandled server error is the one thing that
                         // still pages on-call.
                         forcedSeverity ?? 'critical',
+                        // The id pins route + error signature, so repeats are
+                        // the same fault and belong on one incident.
+                        { dedup: true },
                     );
                 },
             }),

src/backend/types.ts

@@ -97,8 +97,14 @@ export interface ISlackAlertConfig {
     channel?: string;
     /** Bot display name on the posted message. */
     username?: string;
-    /** Lowest severity posted to Slack. Default `info` (everything). */
+    /** Lowest severity posted to Slack. Default `info`. */
     minSeverity?: PagerSeverity;
+    /**
+     * Highest severity posted to Slack. Defaults to `info` when PagerDuty is
+     * configured — anything that pages belongs in the paging system, not in
+     * chat — and to `critical` (everything) when Slack is the only transport.
+     */
+    maxSeverity?: PagerSeverity;
     /**
      * Don't repost the same alarm id within this window. The first occurrence
      * always posts; repeats inside the window only bump the occurrence count