diff --git a/server/src/domain/incidents/incident.service.ts b/server/src/domain/incidents/incident.service.ts index 84ef104bd..1cc941ead 100644 --- a/server/src/domain/incidents/incident.service.ts +++ b/server/src/domain/incidents/incident.service.ts @@ -54,45 +54,35 @@ export class IncidentService implements IIncidentService { } handleIncident = async (monitor: Monitor, decision: MonitorActionDecision, check: Check): Promise => { - if (!decision.shouldCreateIncident && !decision.shouldResolveIncident) { - return null; - } + const { transition } = decision; + if (transition === null) return null; const activeIncident = await this.incidentsRepository.findActiveByMonitorId(monitor.id, monitor.teamId); - if (decision.shouldCreateIncident) { - if (activeIncident) { - return activeIncident; - } else { - let statusCode = check.statusCode; - let message: string | undefined; - - // For threshold breaches, use 9999 status code and build descriptive message - if (decision.incidentReason === "threshold_breach") { - statusCode = 9999; - message = this.notificationMessageBuilder.buildThresholdBreachMessage(monitor, check, decision.thresholdBreaches); - } - - const incident = { + switch (transition) { + case "status_down": + case "threshold_breach": { + if (activeIncident) return activeIncident; + // A threshold breach has no HTTP status + const isBreach = transition === "threshold_breach"; + return await this.incidentsRepository.create({ monitorId: monitor.id, teamId: monitor.teamId, startTime: Date.now().toString(), status: true, - statusCode, - message, - }; - return await this.incidentsRepository.create(incident); + statusCode: isBreach ? 9999 : check.statusCode, + message: isBreach ? this.notificationMessageBuilder.buildThresholdBreachMessage(monitor, check, decision.thresholdBreaches) : undefined, + }); + } + case "status_up": + case "threshold_resolved": { + if (!activeIncident) return null; + activeIncident.status = false; + activeIncident.endTime = Date.now().toString(); + activeIncident.resolutionType = "automatic"; + return await this.incidentsRepository.updateById(activeIncident.id, activeIncident.teamId, activeIncident); } } - - if (!decision.shouldResolveIncident || !activeIncident) { - return null; - } - - activeIncident.status = false; - activeIncident.endTime = Date.now().toString(); - activeIncident.resolutionType = "automatic"; - return await this.incidentsRepository.updateById(activeIncident.id, activeIncident.teamId, activeIncident); }; resolveIncident = async (incidentId: string, userId: string, teamId: string, comment?: string, userEmail?: string) => { diff --git a/server/src/domain/notifications/notification.message-builder.ts b/server/src/domain/notifications/notification.message-builder.ts index 605868ddb..ea6b63ef7 100644 --- a/server/src/domain/notifications/notification.message-builder.ts +++ b/server/src/domain/notifications/notification.message-builder.ts @@ -39,24 +39,21 @@ export class NotificationMessageBuilder implements INotificationMessageBuilder { clientHost, metadata: { teamId: monitor.teamId, - notificationReason: decision.notificationReason, }, }; } private determineNotificationType(decision: MonitorActionDecision, monitor: Monitor): NotificationType { - // Down status has highest priority (critical) - if (monitor.status === "down") { - return "monitor_down"; - } - const isDocker = monitor.type === "docker"; - switch (decision.notificationReason) { + switch (decision.transition) { + case "status_down": + return "monitor_down"; case "threshold_breach": return isDocker ? "container_breach" : "threshold_breach"; case "threshold_resolved": return isDocker ? "container_resolved" : "threshold_resolved"; - default: + case "status_up": + case null: return "monitor_up"; } } diff --git a/server/src/domain/notifications/notification.service.ts b/server/src/domain/notifications/notification.service.ts index b782b7e0a..b9cb1a791 100644 --- a/server/src/domain/notifications/notification.service.ts +++ b/server/src/domain/notifications/notification.service.ts @@ -115,9 +115,7 @@ export class NotificationsService implements INotificationsService { }; handleNotifications = async (monitor: Monitor, check: Check, decision: MonitorActionDecision) => { - if (!decision.shouldSendNotification) { - return false; - } + if (decision.transition === null) return false; // Send notifications based on decision return await this.sendNotifications(monitor, check, decision); diff --git a/server/src/domain/notifications/notification.type.ts b/server/src/domain/notifications/notification.type.ts index 34899386d..29f2a7acb 100644 --- a/server/src/domain/notifications/notification.type.ts +++ b/server/src/domain/notifications/notification.type.ts @@ -1,5 +1,4 @@ import type { HardwareMetricKey } from "@/domain/monitors/monitor.type.js"; -import { MonitorActionDecision } from "@/worker/worker.interface.js"; export const NotificationChannels = [ "email", @@ -120,6 +119,5 @@ export interface NotificationMessage { clientHost: string; metadata: { teamId: string; - notificationReason: MonitorActionDecision["notificationReason"]; }; } diff --git a/server/src/worker/worker.interface.ts b/server/src/worker/worker.interface.ts index 6e9ec3d49..e51b3eff4 100644 --- a/server/src/worker/worker.interface.ts +++ b/server/src/worker/worker.interface.ts @@ -6,12 +6,12 @@ import type { QueueMode } from "@/domain/app-settings/app-settings.type.js"; export type JobHandler = (job: Job) => Promise; export type JobHandlers = Record; + +export const MonitorTransitions = ["status_down", "status_up", "threshold_breach", "threshold_resolved"] as const; +export type MonitorTransition = (typeof MonitorTransitions)[number]; + export interface MonitorActionDecision { - shouldCreateIncident: boolean; - shouldResolveIncident: boolean; - shouldSendNotification: boolean; - incidentReason: "status_down" | "threshold_breach" | null; - notificationReason: "status_change" | "threshold_breach" | "threshold_resolved" | null; + transition: MonitorTransition | null; thresholdBreaches?: HardwareBreaches; } diff --git a/server/src/worker/worker.pipeline.ts b/server/src/worker/worker.pipeline.ts index 3d703d3eb..ef5949229 100644 --- a/server/src/worker/worker.pipeline.ts +++ b/server/src/worker/worker.pipeline.ts @@ -20,13 +20,7 @@ import { IReactorDispatcher } from "@/worker/reactors/reactor.dispatcher.js"; import { JobHandler, MonitorActionDecision, MonitorEvaluation } from "@/worker/worker.interface.js"; const SERVICE_NAME = "WorkerPipeline"; -const NO_ACTION: MonitorActionDecision = { - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, -}; + // The three entry points: the queue calls the two handlers, the buffer calls ingestChecks. // produce and evaluateCheck stay public on the class so tests can drive one stage without the queue or the buffer. export interface IWorkerPipeline { @@ -272,7 +266,7 @@ export class WorkerPipeline implements IWorkerPipeline { return { monitor, check, - decision: { ...NO_ACTION }, + decision: { transition: null }, }; }; @@ -288,30 +282,21 @@ export class WorkerPipeline implements IWorkerPipeline { }; private decide = (statusChange: StatusChangeResult): MonitorActionDecision => { - const { monitor, statusChanged, prevStatus } = statusChange; - const decision: MonitorActionDecision = { ...NO_ACTION }; - if (!statusChanged) return decision; + const { monitor, statusChanged, prevStatus, thresholdBreaches } = statusChange; + if (!statusChanged) return { transition: null }; - if (monitor.status === "down") { - decision.shouldCreateIncident = true; - decision.shouldSendNotification = true; - decision.incidentReason = "status_down"; - decision.notificationReason = "status_change"; - } else if (monitor.status === "breached") { - if (statusChange.thresholdBreaches) decision.thresholdBreaches = statusChange.thresholdBreaches; - decision.shouldCreateIncident = true; - decision.shouldSendNotification = true; - decision.incidentReason = "threshold_breach"; - decision.notificationReason = "threshold_breach"; - } else if (monitor.status === "up" && prevStatus === "breached") { - decision.shouldResolveIncident = true; - decision.shouldSendNotification = true; - decision.notificationReason = "threshold_resolved"; - } else if (monitor.status === "up" && prevStatus === "down") { - decision.shouldResolveIncident = true; - decision.shouldSendNotification = true; - decision.notificationReason = "status_change"; + if (monitor.status === "down") return { transition: "status_down" }; + + if (monitor.status === "breached") { + return thresholdBreaches + ? // Hardware + { transition: "threshold_breach", thresholdBreaches } + : // Docker + { transition: "threshold_breach" }; } - return decision; + + if (monitor.status === "up" && prevStatus === "breached") return { transition: "threshold_resolved" }; + if (monitor.status === "up" && prevStatus === "down") return { transition: "status_up" }; + return { transition: null }; }; } diff --git a/server/test/helpers/heartbeatTestHarness.ts b/server/test/helpers/heartbeatTestHarness.ts index c0f7f1529..f5b9ab9fa 100644 --- a/server/test/helpers/heartbeatTestHarness.ts +++ b/server/test/helpers/heartbeatTestHarness.ts @@ -119,11 +119,11 @@ export function createHeartbeatTestHarness(): HeartbeatTestHarness { nextEgressStatus = status; }; - // The real NotificationsService ignores decisions with shouldSendNotification false; the stub is reached only for - // the ones it would act on, so tests can assert on handleNotifications calls as "notifications sent". + // The real NotificationsService ignores decisions with no transition; the stub is reached only for the ones it + // would act on, so tests can assert on handleNotifications calls as "notifications sent". const guardedNotificationsService = { handleNotifications: (monitor: Monitor, check: Check, decision: MonitorActionDecision) => - decision.shouldSendNotification ? notificationsService.handleNotifications(monitor, check, decision) : Promise.resolve(false), + decision.transition !== null ? notificationsService.handleNotifications(monitor, check, decision) : Promise.resolve(false), }; const notificationReactor = new NotificationReactor(guardedNotificationsService as any); const incidentReactor = new IncidentReactor(incidentService as any); diff --git a/server/test/helpers/notificationMessage.ts b/server/test/helpers/notificationMessage.ts index f73ce67e7..9f8d6c699 100644 --- a/server/test/helpers/notificationMessage.ts +++ b/server/test/helpers/notificationMessage.ts @@ -21,7 +21,7 @@ export const makeMessage = (overrides?: Partial): Notificat timestamp: new Date("2025-01-01T00:00:00Z"), }, clientHost: "https://app.example.com", - metadata: { teamId: "team-1", notificationReason: "status_change" }, + metadata: { teamId: "team-1" }, ...overrides, }); diff --git a/server/test/helpers/notificationProviderContract.ts b/server/test/helpers/notificationProviderContract.ts index 3f3c5b80b..64ea9487c 100644 --- a/server/test/helpers/notificationProviderContract.ts +++ b/server/test/helpers/notificationProviderContract.ts @@ -14,7 +14,7 @@ const makeMessage = (): NotificationMessage => ({ timestamp: new Date("2025-01-01T00:00:00Z"), }, clientHost: "https://app.example.com", - metadata: { teamId: "team-1", notificationReason: "status_change" }, + metadata: { teamId: "team-1" }, }); export const testNotificationProviderContract = ( diff --git a/server/test/integration/incidentLifecycle.test.ts b/server/test/integration/incidentLifecycle.test.ts index 64e723183..534a790ff 100644 --- a/server/test/integration/incidentLifecycle.test.ts +++ b/server/test/integration/incidentLifecycle.test.ts @@ -23,11 +23,7 @@ const makeMonitor = (overrides?: Partial): Monitor => }) as Monitor; const makeDecision = (overrides?: Partial): MonitorActionDecision => ({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, + transition: null, ...overrides, }); @@ -63,7 +59,7 @@ describe("Incident lifecycle (integration)", () => { it("creates an incident when monitor goes down", async () => { const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const decision = makeDecision({ transition: "status_down" }); const incident = await service.handleIncident(monitor, decision, makeCheck(503)); @@ -84,7 +80,7 @@ describe("Incident lifecycle (integration)", () => { it("does not create a duplicate incident for the same monitor", async () => { const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const decision = makeDecision({ transition: "status_down" }); const first = await service.handleIncident(monitor, decision, makeCheck(503)); const second = await service.handleIncident(monitor, decision, makeCheck(503)); @@ -97,10 +93,10 @@ describe("Incident lifecycle (integration)", () => { it("auto-resolves an incident when monitor recovers", async () => { const monitor = makeMonitor({ status: "down" }); - const createDecision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const createDecision = makeDecision({ transition: "status_down" }); const created = await service.handleIncident(monitor, createDecision, makeCheck(503)); - const resolveDecision = makeDecision({ shouldResolveIncident: true }); + const resolveDecision = makeDecision({ transition: "status_up" }); const resolved = await service.handleIncident(monitor, resolveDecision, makeCheck(200)); expect(resolved).not.toBeNull(); @@ -114,7 +110,7 @@ describe("Incident lifecycle (integration)", () => { it("returns null when resolving with no active incident", async () => { const monitor = makeMonitor({ status: "up" }); - const decision = makeDecision({ shouldResolveIncident: true }); + const decision = makeDecision({ transition: "status_up" }); const result = await service.handleIncident(monitor, decision, makeCheck(200)); @@ -125,7 +121,7 @@ describe("Incident lifecycle (integration)", () => { it("manually resolves an active incident with comment", async () => { const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const decision = makeDecision({ transition: "status_down" }); const created = await service.handleIncident(monitor, decision, makeCheck(500)); const resolved = await service.resolveIncident(created!.id, "user-1", "team-1", "Root cause identified", "user@test.com"); @@ -140,7 +136,7 @@ describe("Incident lifecycle (integration)", () => { it("throws when manually resolving an already-resolved incident", async () => { const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const decision = makeDecision({ transition: "status_down" }); const created = await service.handleIncident(monitor, decision, makeCheck(500)); await service.resolveIncident(created!.id, "user-1", "team-1"); @@ -153,7 +149,7 @@ describe("Incident lifecycle (integration)", () => { it("creates a threshold breach incident with statusCode 9999 and descriptive message", async () => { const monitor = makeMonitor({ status: "breached", type: "hardware" }); (messageBuilder.buildThresholdBreachMessage as jest.Mock).mockReturnValue("CPU: 92% (threshold: 80%), MEMORY: 88% (threshold: 85%)"); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "threshold_breach" }); + const decision = makeDecision({ transition: "threshold_breach" }); const incident = await service.handleIncident(monitor, decision, makeCheck(200)); @@ -179,19 +175,19 @@ describe("Incident lifecycle (integration)", () => { const monitor = makeMonitor({ status: "down" }); // First outage - const first = await service.handleIncident(monitor, makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }), makeCheck(503)); + const first = await service.handleIncident(monitor, makeDecision({ transition: "status_down" }), makeCheck(503)); expect(first!.status).toBe(true); // Manually resolved await service.resolveIncident(first!.id, "user-1", "team-1", "Restarted server"); // Second outage — new incident since previous was resolved - const second = await service.handleIncident(monitor, makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }), makeCheck(502)); + const second = await service.handleIncident(monitor, makeDecision({ transition: "status_down" }), makeCheck(502)); expect(second!.id).not.toBe(first!.id); expect(second!.statusCode).toBe(502); // Auto-resolve - const resolved = await service.handleIncident(monitor, makeDecision({ shouldResolveIncident: true }), makeCheck(200)); + const resolved = await service.handleIncident(monitor, makeDecision({ transition: "status_up" }), makeCheck(200)); expect(resolved!.id).toBe(second!.id); expect(resolved!.status).toBe(false); expect(resolved!.resolutionType).toBe("automatic"); @@ -207,7 +203,7 @@ describe("Incident lifecycle (integration)", () => { it("incidents for different monitors do not interfere", async () => { const monitorA = makeMonitor({ id: "mon-a", status: "down" }); const monitorB = makeMonitor({ id: "mon-b", status: "down" }); - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }); + const decision = makeDecision({ transition: "status_down" }); const incidentA = await service.handleIncident(monitorA, decision, makeCheck(500)); const incidentB = await service.handleIncident(monitorB, decision, makeCheck(502)); @@ -216,7 +212,7 @@ describe("Incident lifecycle (integration)", () => { expect(repo.getAll()).toHaveLength(2); // Resolving monitor A does not affect monitor B - const resolved = await service.handleIncident(monitorA, makeDecision({ shouldResolveIncident: true }), makeCheck(200)); + const resolved = await service.handleIncident(monitorA, makeDecision({ transition: "status_up" }), makeCheck(200)); expect(resolved!.id).toBe(incidentA!.id); const bStillActive = await repo.findActiveByMonitorId("mon-b", "team-1"); diff --git a/server/test/unit/providers/notifications/teamsProvider.test.ts b/server/test/unit/providers/notifications/teamsProvider.test.ts index 514190fe5..8bd296206 100644 --- a/server/test/unit/providers/notifications/teamsProvider.test.ts +++ b/server/test/unit/providers/notifications/teamsProvider.test.ts @@ -48,7 +48,6 @@ const createMessage = (overrides?: Partial): NotificationMe clientHost: "https://app.example.com", metadata: { teamId: "team-1", - notificationReason: "monitor_down", }, ...overrides, }); diff --git a/server/test/unit/services/incidentService.test.ts b/server/test/unit/services/incidentService.test.ts index 8639542c4..1a01cd57f 100644 --- a/server/test/unit/services/incidentService.test.ts +++ b/server/test/unit/services/incidentService.test.ts @@ -94,11 +94,7 @@ const makeCheck = (overrides?: Partial): Check => }) as Check; const makeDecision = (overrides?: Partial): MonitorActionDecision => ({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, + transition: null, ...overrides, }); @@ -114,32 +110,24 @@ describe("IncidentService", () => { expect(result).toBeNull(); }); - it("returns existing active incident when shouldCreateIncident and one already exists", async () => { + it("returns the existing active incident on a down transition when one is already open", async () => { const existing = makeIncident(); const { service, incidentsRepository } = createService(); (incidentsRepository.findActiveByMonitorId as jest.Mock).mockResolvedValue(existing); - const result = await service.handleIncident( - makeMonitor(), - makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }), - makeCheck({ statusCode: 500 }) - ); + const result = await service.handleIncident(makeMonitor(), makeDecision({ transition: "status_down" }), makeCheck({ statusCode: 500 })); expect(result).toBe(existing); expect(incidentsRepository.create).not.toHaveBeenCalled(); }); - it("creates a new incident when shouldCreateIncident and no active incident exists", async () => { + it("creates a new incident on a down transition when none is open", async () => { const created = makeIncident(); const { service, incidentsRepository, notificationMessageBuilder } = createService(); (incidentsRepository.findActiveByMonitorId as jest.Mock).mockResolvedValue(null); (incidentsRepository.create as jest.Mock).mockResolvedValue(created); - const result = await service.handleIncident( - makeMonitor(), - makeDecision({ shouldCreateIncident: true, incidentReason: "status_down" }), - makeCheck({ statusCode: 500 }) - ); + const result = await service.handleIncident(makeMonitor(), makeDecision({ transition: "status_down" }), makeCheck({ statusCode: 500 })); expect(result).toBe(created); expect(incidentsRepository.create).toHaveBeenCalledWith( @@ -163,7 +151,7 @@ describe("IncidentService", () => { const monitor = makeMonitor({ type: "hardware" }); const check = makeCheck({ metadata: { monitorId: "mon-1", teamId: "team-1", type: "hardware" }, cpu: { usage_percent: 0.95 } }); const thresholdBreaches = { cpu: true, memory: false, disk: false, temp: false }; - const decision = makeDecision({ shouldCreateIncident: true, incidentReason: "threshold_breach", thresholdBreaches }); + const decision = makeDecision({ transition: "threshold_breach", thresholdBreaches }); await service.handleIncident(monitor, decision, check); expect(notificationMessageBuilder.buildThresholdBreachMessage).toHaveBeenCalledWith(monitor, check, thresholdBreaches); @@ -175,14 +163,14 @@ describe("IncidentService", () => { ); }); - it("resolves active incident when shouldResolveIncident", async () => { + it("resolves the active incident on an up transition", async () => { const active = makeIncident(); const resolved = makeIncident({ status: false, endTime: "123", resolutionType: "automatic" }); const { service, incidentsRepository } = createService(); (incidentsRepository.findActiveByMonitorId as jest.Mock).mockResolvedValue(active); (incidentsRepository.updateById as jest.Mock).mockResolvedValue(resolved); - const result = await service.handleIncident(makeMonitor(), makeDecision({ shouldResolveIncident: true }), makeCheck()); + const result = await service.handleIncident(makeMonitor(), makeDecision({ transition: "status_up" }), makeCheck()); expect(result).toBe(resolved); expect(incidentsRepository.updateById).toHaveBeenCalledWith( @@ -192,11 +180,28 @@ describe("IncidentService", () => { ); }); - it("returns null when shouldResolveIncident but no active incident exists", async () => { + it("resolves the active incident on a threshold_resolved transition", async () => { + const active = makeIncident(); + const resolved = makeIncident({ status: false, endTime: "123", resolutionType: "automatic" }); + const { service, incidentsRepository } = createService(); + (incidentsRepository.findActiveByMonitorId as jest.Mock).mockResolvedValue(active); + (incidentsRepository.updateById as jest.Mock).mockResolvedValue(resolved); + + const result = await service.handleIncident(makeMonitor(), makeDecision({ transition: "threshold_resolved" }), makeCheck()); + + expect(result).toBe(resolved); + expect(incidentsRepository.updateById).toHaveBeenCalledWith( + "inc-1", + "team-1", + expect.objectContaining({ status: false, resolutionType: "automatic" }) + ); + }); + + it("returns null on an up transition when no incident is open", async () => { const { service, incidentsRepository } = createService(); (incidentsRepository.findActiveByMonitorId as jest.Mock).mockResolvedValue(null); - const result = await service.handleIncident(makeMonitor(), makeDecision({ shouldResolveIncident: true }), makeCheck()); + const result = await service.handleIncident(makeMonitor(), makeDecision({ transition: "status_up" }), makeCheck()); expect(result).toBeNull(); }); diff --git a/server/test/unit/services/notificationMessageBuilder.test.ts b/server/test/unit/services/notificationMessageBuilder.test.ts index 7560b1168..7297f1b19 100644 --- a/server/test/unit/services/notificationMessageBuilder.test.ts +++ b/server/test/unit/services/notificationMessageBuilder.test.ts @@ -23,11 +23,7 @@ const makeMonitor = (overrides?: Partial): Monitor => }) as Monitor; const makeDecision = (overrides?: Partial): MonitorActionDecision => ({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: true, - incidentReason: null, - notificationReason: "status_change", + transition: "status_down", ...overrides, }); @@ -100,15 +96,12 @@ describe("NotificationMessageBuilder", () => { }); expect(msg.content.title).toBe("Monitor Down: Test Monitor"); expect(msg.clientHost).toBe("https://app.example.com"); - expect(msg.metadata).toEqual({ - teamId: "team-1", - notificationReason: "status_change", - }); + expect(msg.metadata).toEqual({ teamId: "team-1" }); }); it("builds a monitor_up message", () => { const monitor = makeMonitor({ status: "up", type: "http" }); - const decision = makeDecision({ notificationReason: "status_change" }); + const decision = makeDecision({ transition: "status_up" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, "https://app.example.com"); @@ -120,7 +113,7 @@ describe("NotificationMessageBuilder", () => { it("builds a threshold_breach message", () => { const monitor = makeMonitor({ status: "up", type: "hardware", cpuAlertThreshold: 80 }); const decision = makeDecision({ - notificationReason: "threshold_breach", + transition: "threshold_breach", thresholdBreaches: { cpu: true, memory: false, disk: false, temp: false }, }); const check = makeCheck({ cpu: { usage_percent: 0.9, temperature: [50] } }); @@ -136,7 +129,7 @@ describe("NotificationMessageBuilder", () => { it("builds a threshold_resolved message for hardware monitor recovering", () => { const monitor = makeMonitor({ status: "up", type: "hardware" }); - const decision = makeDecision({ notificationReason: "threshold_resolved" }); + const decision = makeDecision({ transition: "threshold_resolved" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, "https://app.example.com"); @@ -149,18 +142,9 @@ describe("NotificationMessageBuilder", () => { // ── determineNotificationType (via buildMessage) ───────────────────── describe("determineNotificationType", () => { - it("returns monitor_down when status is down, even if notificationReason is threshold_breach", () => { - const monitor = makeMonitor({ status: "down" }); - const decision = makeDecision({ notificationReason: "threshold_breach" }); - - const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); - - expect(msg.type).toBe("monitor_down"); - }); - - it("returns threshold_breach when reason is threshold_breach and status is not down", () => { + it("returns threshold_breach for a hardware monitor on a threshold_breach transition", () => { const monitor = makeMonitor({ status: "up", type: "hardware" }); - const decision = makeDecision({ notificationReason: "threshold_breach" }); + const decision = makeDecision({ transition: "threshold_breach" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); @@ -169,16 +153,16 @@ describe("NotificationMessageBuilder", () => { it("returns threshold_resolved for hardware monitor with threshold_resolved reason", () => { const monitor = makeMonitor({ status: "up", type: "hardware" }); - const decision = makeDecision({ notificationReason: "threshold_resolved" }); + const decision = makeDecision({ transition: "threshold_resolved" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); expect(msg.type).toBe("threshold_resolved"); }); - it("returns monitor_up for hardware monitor recovering from down with status_change", () => { + it("returns monitor_up for a hardware monitor on a status_up transition", () => { const monitor = makeMonitor({ status: "up", type: "hardware" }); - const decision = makeDecision({ notificationReason: "status_change" }); + const decision = makeDecision({ transition: "status_up" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); @@ -186,7 +170,7 @@ describe("NotificationMessageBuilder", () => { }); it("returns container_breach for docker monitor with threshold_breach reason", () => { - const decision = makeDecision({ notificationReason: "threshold_breach" }); + const decision = makeDecision({ transition: "threshold_breach" }); const msg = builder.buildMessage(makeDockerMonitor(), makeCheck(), decision, ""); @@ -194,15 +178,15 @@ describe("NotificationMessageBuilder", () => { }); it("returns container_resolved for docker monitor with threshold_resolved reason", () => { - const decision = makeDecision({ notificationReason: "threshold_resolved" }); + const decision = makeDecision({ transition: "threshold_resolved" }); const msg = builder.buildMessage(makeDockerMonitor({ status: "up" }), makeCheck(), decision, ""); expect(msg.type).toBe("container_resolved"); }); - it("returns monitor_down for docker monitor that is down regardless of reason", () => { - const decision = makeDecision({ notificationReason: "threshold_breach" }); + it("returns monitor_down for a docker monitor on a status_down transition", () => { + const decision = makeDecision({ transition: "status_down" }); const msg = builder.buildMessage(makeDockerMonitor({ status: "down" }), makeCheck(), decision, ""); @@ -211,16 +195,16 @@ describe("NotificationMessageBuilder", () => { it("returns monitor_up for non-hardware monitor with status up", () => { const monitor = makeMonitor({ status: "up", type: "http" }); - const decision = makeDecision({ notificationReason: "status_change" }); + const decision = makeDecision({ transition: "status_up" }); const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); expect(msg.type).toBe("monitor_up"); }); - it("returns monitor_up as default for unrecognized status", () => { + it("returns monitor_up when there is no transition", () => { const monitor = makeMonitor({ status: "unknown" as any }); - const decision = makeDecision({ notificationReason: null }); + const decision = makeDecision({ transition: null }); const msg = builder.buildMessage(monitor, makeCheck(), decision, ""); @@ -240,19 +224,14 @@ describe("NotificationMessageBuilder", () => { const msg = builder.buildMessage( makeMonitor({ status: "up", type: "hardware" }), makeCheck(), - makeDecision({ notificationReason: "threshold_breach" }), + makeDecision({ transition: "threshold_breach" }), "" ); expect(msg.severity).toBe("warning"); }); it("returns success for monitor_up", () => { - const msg = builder.buildMessage( - makeMonitor({ status: "up", type: "http" }), - makeCheck(), - makeDecision({ notificationReason: "status_change" }), - "" - ); + const msg = builder.buildMessage(makeMonitor({ status: "up", type: "http" }), makeCheck(), makeDecision({ transition: "status_up" }), ""); expect(msg.severity).toBe("success"); }); @@ -260,24 +239,19 @@ describe("NotificationMessageBuilder", () => { const msg = builder.buildMessage( makeMonitor({ status: "up", type: "hardware" }), makeCheck(), - makeDecision({ notificationReason: "threshold_resolved" }), + makeDecision({ transition: "threshold_resolved" }), "" ); expect(msg.severity).toBe("success"); }); it("returns warning for container_breach", () => { - const msg = builder.buildMessage(makeDockerMonitor(), makeCheck(), makeDecision({ notificationReason: "threshold_breach" }), ""); + const msg = builder.buildMessage(makeDockerMonitor(), makeCheck(), makeDecision({ transition: "threshold_breach" }), ""); expect(msg.severity).toBe("warning"); }); it("returns success for container_resolved", () => { - const msg = builder.buildMessage( - makeDockerMonitor({ status: "up" }), - makeCheck(), - makeDecision({ notificationReason: "threshold_resolved" }), - "" - ); + const msg = builder.buildMessage(makeDockerMonitor({ status: "up" }), makeCheck(), makeDecision({ transition: "threshold_resolved" }), ""); expect(msg.severity).toBe("success"); }); }); @@ -331,7 +305,7 @@ describe("NotificationMessageBuilder", () => { describe("monitor_up", () => { it("includes recovery details", () => { const monitor = makeMonitor({ status: "up", type: "http" }); - const msg = builder.buildMessage(monitor, makeCheck(), makeDecision(), ""); + const msg = builder.buildMessage(monitor, makeCheck(), makeDecision({ transition: "status_up" }), ""); expect(msg.content.title).toBe("Monitor Recovered: Test Monitor"); expect(msg.content.summary).toBe('Monitor "Test Monitor" is back up and operational.'); @@ -344,7 +318,7 @@ describe("NotificationMessageBuilder", () => { const monitor = makeMonitor({ status: "up", type: "hardware", cpuAlertThreshold: 80 }); const check = makeCheck({ cpu: { usage_percent: 0.9, temperature: [50] } }); const decision = makeDecision({ - notificationReason: "threshold_breach", + transition: "threshold_breach", thresholdBreaches: { cpu: true, memory: false, disk: false, temp: false }, }); @@ -357,7 +331,7 @@ describe("NotificationMessageBuilder", () => { const monitor = makeMonitor({ status: "up", type: "hardware", cpuAlertThreshold: 80 }); const check = makeCheck({ cpu: { usage_percent: 0.9, temperature: [50] } }); - const msg = builder.buildMessage(monitor, check, makeDecision({ notificationReason: "threshold_breach" }), ""); + const msg = builder.buildMessage(monitor, check, makeDecision({ transition: "threshold_breach" }), ""); expect(msg.content.thresholds).toEqual([]); }); @@ -366,7 +340,7 @@ describe("NotificationMessageBuilder", () => { describe("threshold_resolved", () => { it("includes resolved details", () => { const monitor = makeMonitor({ status: "up", type: "hardware" }); - const msg = builder.buildMessage(monitor, makeCheck(), makeDecision({ notificationReason: "threshold_resolved" }), ""); + const msg = builder.buildMessage(monitor, makeCheck(), makeDecision({ transition: "threshold_resolved" }), ""); expect(msg.content.title).toBe("Thresholds Resolved: Test Monitor"); expect(msg.content.summary).toBe('Monitor "Test Monitor" thresholds have returned to normal.'); @@ -381,7 +355,7 @@ describe("NotificationMessageBuilder", () => { containers: makeContainers([{ name: "db", state: "exited", exitCode: 1 }, { name: "api", health: "unhealthy" }, { name: "web" }]), }); - const msg = builder.buildMessage(makeDockerMonitor(), check, makeDecision({ notificationReason: "threshold_breach" }), ""); + const msg = builder.buildMessage(makeDockerMonitor(), check, makeDecision({ transition: "threshold_breach" }), ""); expect(msg.content.title).toBe("Container Alert: Test Monitor"); expect(msg.content.summary).toBe('2 container(s) on "Test Monitor" need attention.'); @@ -399,7 +373,7 @@ describe("NotificationMessageBuilder", () => { }); const monitor = makeDockerMonitor({ dockerAlertOnStopped: false }); - const msg = builder.buildMessage(monitor, check, makeDecision({ notificationReason: "threshold_breach" }), ""); + const msg = builder.buildMessage(monitor, check, makeDecision({ transition: "threshold_breach" }), ""); expect(msg.content.details).toEqual(["URL: unix:///var/run/docker.sock", "Type: docker", "api: unhealthy"]); }); @@ -407,7 +381,7 @@ describe("NotificationMessageBuilder", () => { it("lists no containers when the check carries no container data", () => { const check = makeCheck({ status: false, message: "connect ECONNREFUSED" }); - const msg = builder.buildMessage(makeDockerMonitor(), check, makeDecision({ notificationReason: "threshold_breach" }), ""); + const msg = builder.buildMessage(makeDockerMonitor(), check, makeDecision({ transition: "threshold_breach" }), ""); expect(msg.content.summary).toBe('0 container(s) on "Test Monitor" need attention.'); expect(msg.content.details).toEqual(["URL: unix:///var/run/docker.sock", "Type: docker"]); @@ -416,12 +390,7 @@ describe("NotificationMessageBuilder", () => { describe("container_resolved", () => { it("includes recovered details", () => { - const msg = builder.buildMessage( - makeDockerMonitor({ status: "up" }), - makeCheck(), - makeDecision({ notificationReason: "threshold_resolved" }), - "" - ); + const msg = builder.buildMessage(makeDockerMonitor({ status: "up" }), makeCheck(), makeDecision({ transition: "threshold_resolved" }), ""); expect(msg.content.title).toBe("Containers Recovered: Test Monitor"); expect(msg.content.summary).toBe('All containers on "Test Monitor" are back to normal.'); @@ -430,18 +399,10 @@ describe("NotificationMessageBuilder", () => { }); describe("default content", () => { - it("builds default content for unhandled notification type", () => { - // Force default by creating a scenario where type falls through - // We can test buildContent default by subclassing or testing indirectly - // The default case in buildContent handles "test" type among others - // Since determineNotificationType never returns "test", we test via - // a monitor with unrecognized status that maps to monitor_up - // Instead, let's verify the default case exists by checking that - // non-standard types would get default content + it("builds monitor_up content on a status_up transition", () => { const monitor = makeMonitor({ status: "up", type: "http" }); - const msg = builder.buildMessage(monitor, makeCheck(), makeDecision(), ""); + const msg = builder.buildMessage(monitor, makeCheck(), makeDecision({ transition: "status_up" }), ""); - // monitor_up is handled, so content should be monitor_up specific expect(msg.content.title).toBe("Monitor Recovered: Test Monitor"); }); }); diff --git a/server/test/unit/services/notificationsService.test.ts b/server/test/unit/services/notificationsService.test.ts index c621fe0e4..e100a76ec 100644 --- a/server/test/unit/services/notificationsService.test.ts +++ b/server/test/unit/services/notificationsService.test.ts @@ -122,11 +122,7 @@ const makeMonitor = (overrides?: Partial): Monitor => }) as Monitor; const makeDecision = (overrides?: Partial): MonitorActionDecision => ({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: true, - incidentReason: null, - notificationReason: "status_change", + transition: "status_down", ...overrides, }); @@ -148,9 +144,9 @@ describe("NotificationsService", () => { // ── handleNotifications ─────────────────────────────────────────────────── describe("handleNotifications", () => { - it("returns false when shouldSendNotification is false", async () => { + it("returns false when there is no transition", async () => { const { service } = createService(); - const result = await service.handleNotifications(makeMonitor(), makeCheck(), makeDecision({ shouldSendNotification: false })); + const result = await service.handleNotifications(makeMonitor(), makeCheck(), makeDecision({ transition: null })); expect(result).toBe(false); }); diff --git a/server/test/unit/services/workerPipeline.test.ts b/server/test/unit/services/workerPipeline.test.ts index fbaf2eb73..7410c04bc 100644 --- a/server/test/unit/services/workerPipeline.test.ts +++ b/server/test/unit/services/workerPipeline.test.ts @@ -714,11 +714,7 @@ describe("WorkerPipeline", () => { expect(result).toMatchObject({ monitor: statusChange.monitor, check, - decision: expect.objectContaining({ - shouldCreateIncident: true, - shouldSendNotification: true, - incidentReason: "status_down", - }), + decision: { transition: "status_down" }, }); }); @@ -730,7 +726,7 @@ describe("WorkerPipeline", () => { const result = await pipeline.evaluateCheck(makeCheck({ status: true, statusCode: 200, message: "OK" }), makeMonitor({ status: "down" })); - expect(result.decision).toMatchObject({ shouldResolveIncident: true, shouldSendNotification: true }); + expect(result.decision).toEqual({ transition: "status_up" }); }); it("produces a no-op decision when status did not change", async () => { @@ -738,11 +734,7 @@ describe("WorkerPipeline", () => { const result = await pipeline.evaluateCheck(makeCheck({ status: true, statusCode: 200, message: "OK" }), makeMonitor()); - expect(result.decision).toMatchObject({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - }); + expect(result.decision).toEqual({ transition: null }); }); // ── degraded egress ─────────────────────────────────────────────────── @@ -757,13 +749,7 @@ describe("WorkerPipeline", () => { expect(deps.statusService.updateMonitorStatus).not.toHaveBeenCalled(); expect(result.monitor).toBe(monitor); expect(result).not.toHaveProperty("statusChange"); - expect(result.decision).toEqual({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, - }); + expect(result.decision).toEqual({ transition: null }); expect(deps.logger.debug).toHaveBeenCalledWith(expect.objectContaining({ message: expect.stringContaining("egress was degraded") })); }); @@ -790,64 +776,38 @@ describe("WorkerPipeline", () => { it("does nothing when statusChanged is false", async () => { const decision = await decide(makeStatusChange({ status: "down", statusChanged: false, prevStatus: "up" })); - expect(decision).toEqual({ - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, - }); + expect(decision).toEqual({ transition: null }); }); - it("creates incident and notifies when monitor goes down", async () => { + it("reports status_down when the monitor goes down", async () => { const decision = await decide(makeStatusChange({ status: "down", statusChanged: true, prevStatus: "up" })); - expect(decision).toMatchObject({ - shouldCreateIncident: true, - shouldSendNotification: true, - incidentReason: "status_down", - notificationReason: "status_change", - }); + expect(decision).toEqual({ transition: "status_down" }); }); - it("creates incident and notifies when monitor is breached", async () => { + it("reports threshold_breach when the monitor is breached", async () => { const decision = await decide(makeStatusChange({ status: "breached", statusChanged: true, prevStatus: "up" })); - expect(decision).toMatchObject({ - shouldCreateIncident: true, - shouldSendNotification: true, - incidentReason: "threshold_breach", - notificationReason: "threshold_breach", - }); + expect(decision).toEqual({ transition: "threshold_breach" }); }); - it("resolves incident when monitor recovers from down", async () => { + it("reports status_up when the monitor recovers from down", async () => { const decision = await decide(makeStatusChange({ status: "up", statusChanged: true, prevStatus: "down" })); - expect(decision).toMatchObject({ - shouldResolveIncident: true, - shouldSendNotification: true, - shouldCreateIncident: false, - notificationReason: "status_change", - }); + expect(decision).toEqual({ transition: "status_up" }); }); - it("resolves incident with threshold_resolved when monitor recovers from breached", async () => { + it("reports threshold_resolved when the monitor recovers from breached", async () => { const decision = await decide(makeStatusChange({ status: "up", statusChanged: true, prevStatus: "breached" })); - expect(decision).toMatchObject({ - shouldResolveIncident: true, - shouldSendNotification: true, - shouldCreateIncident: false, - notificationReason: "threshold_resolved", - }); + expect(decision).toEqual({ transition: "threshold_resolved" }); }); - it("does not create or resolve for unhandled status transitions", async () => { + it("reports no transition for unhandled status changes", async () => { const decision = await decide(makeStatusChange({ status: "paused", statusChanged: true, prevStatus: "up" })); - expect(decision).toMatchObject({ shouldCreateIncident: false, shouldResolveIncident: false, shouldSendNotification: false }); + expect(decision).toEqual({ transition: null }); }); it("carries the threshold breaches onto a breached decision", async () => { const thresholdBreaches = { cpu: true, memory: false, disk: true, temp: false }; const decision = await decide(makeStatusChange({ status: "breached", statusChanged: true, prevStatus: "up", thresholdBreaches })); - expect(decision.thresholdBreaches).toEqual(thresholdBreaches); + expect(decision).toEqual({ transition: "threshold_breach", thresholdBreaches }); }); it("leaves threshold breaches off a down decision even when the status change carries them", async () => { @@ -865,12 +825,11 @@ describe("WorkerPipeline", () => { // StatusService only sets thresholdBreaches for hardware monitors; a docker breach must not leave the key behind holding undefined. it("leaves threshold breaches off a breached decision when the status change carries none", async () => { const decision = await decide(makeStatusChange({ status: "breached", statusChanged: true, prevStatus: "up" })); - expect(decision).toMatchObject({ shouldCreateIncident: true, incidentReason: "threshold_breach" }); - expect(decision).not.toHaveProperty("thresholdBreaches"); + expect(decision).toEqual({ transition: "threshold_breach" }); }); - // Each decision starts from a shared template. A transition must not leak its flags into the next decision - // the same pipeline makes, whether that is a no-op evaluation or a degraded-egress short-circuit. + // A transition must not leak into the next decision the same pipeline makes, whether that is a no-op + // evaluation or a degraded-egress short-circuit. it("starts every decision clean after a transition on the same pipeline", async () => { const updateMonitorStatus = jest .fn() @@ -883,14 +842,8 @@ describe("WorkerPipeline", () => { const second = await pipeline.evaluateCheck(failing, makeMonitor({ status: "down" })); const degraded = await pipeline.evaluateCheck(makeCheck({ ...failing, egressStatus: "degraded" }), makeMonitor({ status: "down" })); - expect(first.decision.shouldCreateIncident).toBe(true); - const clean = { - shouldCreateIncident: false, - shouldResolveIncident: false, - shouldSendNotification: false, - incidentReason: null, - notificationReason: null, - }; + expect(first.decision).toEqual({ transition: "status_down" }); + const clean = { transition: null }; expect(second.decision).toEqual(clean); expect(degraded.decision).toEqual(clean); });