From e8bcfdd21d3de23818f61314aa525275866fa99d Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Tue, 14 Apr 2026 11:31:09 -0700 Subject: [PATCH 1/7] add additional heartbeat tests --- .../heartbeatJobDownDetection.test.ts | 26 ++++++++++ .../heartbeatJobThresholdBreach.test.ts | 51 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/server/test/integration/heartbeatJobDownDetection.test.ts b/server/test/integration/heartbeatJobDownDetection.test.ts index abf928cb1..71628aabe 100644 --- a/server/test/integration/heartbeatJobDownDetection.test.ts +++ b/server/test/integration/heartbeatJobDownDetection.test.ts @@ -72,5 +72,31 @@ describe("Heartbeat job: down detection", () => { const storedMonitor = await h.monitorsRepo.findById("mon-1", "team-1"); expect(storedMonitor.status).toBe("up"); expect(h.incidentsRepo.getAll()).toHaveLength(0); + // Regression guard for #3438: a sub-threshold blip must produce NO notification + // dispatches. Before PR #3505, a failing check below threshold silently wrote + // status="down" and the next passing check fired a spurious "Recovered" event. + expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); + }); + + it("fires zero notifications on a sub-threshold blip (regression #3438)", async () => { + // Dedicated end-to-end guard for the #3438 bug. Distinct from "stays up when + // failures are below threshold" because it pins the notification-dispatch + // contract independently of status/incident assertions, so a future refactor + // that reintroduces the silent-down-write can't sneak past by happening to + // leave status and incidents in the right shape. + const monitor = makeMonitor(); + h.monitorsRepo.seed(monitor); + + // Failing check below threshold (window [t,t,t,t,f], 20%) + h.setNextResponse(false, 503); + await h.heartbeatJob(monitor); + + // Passing check — pre-fix, this is where the spurious "Recovered" fired + h.setNextResponse(true, 200); + await h.heartbeatJob(monitor); + + expect((await h.monitorsRepo.findById("mon-1", "team-1")).status).toBe("up"); + expect(h.incidentsRepo.getAll()).toHaveLength(0); + expect(h.notificationsService.handleNotifications).not.toHaveBeenCalled(); }); }); diff --git a/server/test/integration/heartbeatJobThresholdBreach.test.ts b/server/test/integration/heartbeatJobThresholdBreach.test.ts index edaf63173..ccb4065fa 100644 --- a/server/test/integration/heartbeatJobThresholdBreach.test.ts +++ b/server/test/integration/heartbeatJobThresholdBreach.test.ts @@ -123,6 +123,57 @@ describe("Heartbeat job: hardware threshold breach", () => { expect(incidents[0].resolutionType).toBe("automatic"); }); + it("transitions breached hardware monitor to down when reachability also fails", async () => { + // A hardware monitor currently in "breached" state starts failing its reachability + // checks enough to cross the statusWindow threshold. "down" must take precedence + // over "breached" — when both conditions are true, the user wants the escalated + // alert. Unit-covered in statusService.test.ts via computeReachability; this is + // the end-to-end counterpart that exercises the full heartbeat path including + // incident-lifecycle handling. + const monitor = makeMonitor({ + type: "hardware", + status: "breached", + cpuAlertThreshold: 80, + cpuAlertCounter: 0, + memoryAlertThreshold: 100, + memoryAlertCounter: 5, + diskAlertThreshold: 100, + diskAlertCounter: 5, + tempAlertThreshold: 100, + tempAlertCounter: 5, + selectedDisks: [], + // Window is fully healthy — the monitor is breached from hardware metrics, + // not from reachability. The test drives the window from clean → failing. + statusWindow: [true, true, true, true, true], + }); + h.monitorsRepo.seed(monitor); + + // Seed an active breached incident matching the initial state. + await h.incidentsRepo.create({ + monitorId: "mon-1", + teamId: "team-1", + status: true, + statusCode: 9999, + message: "CPU: 90% (threshold: 80%)", + }); + + // Drive reachability failure: 3 consecutive failing heartbeats. + // Window evolves: [t,t,t,t,t] → [t,t,t,t,f] (20%) → [t,t,t,f,f] (40%) → [t,t,f,f,f] (60%) + // At 60% >= threshold, and current status "breached" !== "down", reachability fires. + // Reachability sends HTTP-shaped responses with no hardware payload, so the + // hardware block is correctly skipped for these checks. + h.setNextResponse(false, 503); + for (let i = 0; i < 3; i++) { + await h.heartbeatJob(monitor); + } + + const stored = await h.monitorsRepo.findById("mon-1", "team-1"); + expect(stored.status).toBe("down"); + + // Notification dispatch must have fired for the transition. + expect(h.notificationsService.handleNotifications).toHaveBeenCalled(); + }); + it("does not create duplicate incidents while remaining breached", async () => { const monitor = makeMonitor({ type: "hardware", From 6f7fba5ae51e80c36b27ee8afa3203b3f5f7c8e9 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 10:54:43 -0700 Subject: [PATCH 2/7] fixes a bug where state was not correctly calcluated for monitors in initializing state --- server/src/service/infrastructure/statusService.ts | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/server/src/service/infrastructure/statusService.ts b/server/src/service/infrastructure/statusService.ts index 00fd3b3b4..fa57117dc 100755 --- a/server/src/service/infrastructure/statusService.ts +++ b/server/src/service/infrastructure/statusService.ts @@ -257,12 +257,18 @@ export class StatusService implements IStatusService { }; } - // With a full window, a single raw check must not change monitor.status; only the sliding-window threshold can trigger a transition. - let newStatus = monitor.status; + // With a full window, a single raw check must not change UNLESS we are initializing. Otherwise, only the sliding-window threshold can trigger a transition. + let newStatus: MonitorStatus; + if (monitor.status === "initializing") { + newStatus = status === true ? "up" : "down"; + } else { + newStatus = monitor.status; + } + let statusChanged = false; // First evaluate reachability-based status changes, which apply to all monitor types and take precedence over hardware breaches. - const reachabilityResult = this.computeReachability(monitor.status, monitor.statusWindow, monitor.statusWindowThreshold); + const reachabilityResult = this.computeReachability(newStatus, monitor.statusWindow, monitor.statusWindowThreshold); if (reachabilityResult.transitioned) { newStatus = reachabilityResult.nextStatus; statusChanged = true; @@ -273,7 +279,7 @@ export class StatusService implements IStatusService { const hardwarePayload = statusResponse.payload as HardwareStatusPayload | undefined; if (monitor.type === "hardware" && hardwarePayload?.data) { const hardware = this.computeHardwareStatus({ - currentStatus: monitor.status, + currentStatus: newStatus, reachabilityDown: newStatus === "down", metrics: hardwarePayload.data, thresholds: { From 564b356115ba935e982d52b44301cfef1355d1e0 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 11:06:30 -0700 Subject: [PATCH 3/7] tighten streak time coniditional --- client/src/Components/monitors/MonitorStatBoxes.tsx | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/client/src/Components/monitors/MonitorStatBoxes.tsx b/client/src/Components/monitors/MonitorStatBoxes.tsx index 044bb71ab..1d4dc5b32 100644 --- a/client/src/Components/monitors/MonitorStatBoxes.tsx +++ b/client/src/Components/monitors/MonitorStatBoxes.tsx @@ -25,10 +25,7 @@ export const MonitorStatBoxes = ({ } const timeOfLastFailure = monitorStats?.timeOfLastFailure || 0; - const timeSinceLastFailure = - timeOfLastFailure > 0 - ? Date.now() - timeOfLastFailure - : Date.now() - new Date(monitorStats?.createdAt || 0).getTime(); + const timeSinceLastFailure = timeOfLastFailure > 0 ? Date.now() - timeOfLastFailure : 0; // Determine time since last check const timeOfLastCheck = monitorStats?.lastCheckTimestamp || 0; From be462811ac34db54632a556bbb8624f45d1c01ed Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 11:08:40 -0700 Subject: [PATCH 4/7] test initializing state --- .../test/unit/services/statusService.test.ts | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/server/test/unit/services/statusService.test.ts b/server/test/unit/services/statusService.test.ts index ae8db0924..4e6ceb323 100644 --- a/server/test/unit/services/statusService.test.ts +++ b/server/test/unit/services/statusService.test.ts @@ -418,6 +418,45 @@ describe("StatusService", () => { expect(result.monitor.status).toBe("down"); }); + it("resumes from 'initializing' to 'up' on a passing check with a full window (regression: pause/resume)", async () => { + // When a monitor is paused and resumed, togglePauseById sets status='initializing' + // but does NOT reset statusWindow. On the next check the window is already full, + // so the warmup early-return is skipped. Pre-fix, newStatus was seeded from + // monitor.status ('initializing') and computeReachability never transitioned + // out of 'initializing' on a passing window, so the monitor was stuck forever. + const monitor = makeMonitor({ + statusWindow: [true, true, true, true, true], + statusWindowSize: 5, + statusWindowThreshold: 80, + status: "initializing", + }); + const { service, monitorsRepository } = createService(); + (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); + (monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m)); + + const result = await service.updateMonitorStatus(makeStatusResponse({ status: true }), makeCheck({ status: true })); + + expect(result.monitor.status).toBe("up"); + }); + + it("resumes from 'initializing' to 'down' on a failing check with a full window (regression: pause/resume mirror)", async () => { + // Mirror of the pause/resume regression with a failing check: should flip to 'down' + // immediately rather than stay stuck in 'initializing'. + const monitor = makeMonitor({ + statusWindow: [false, false, false, false, false], + statusWindowSize: 5, + statusWindowThreshold: 80, + status: "initializing", + }); + const { service, monitorsRepository } = createService(); + (monitorsRepository.findById as jest.Mock).mockResolvedValue(monitor); + (monitorsRepository.updateById as jest.Mock).mockImplementation((_id: unknown, _tid: unknown, m: unknown) => Promise.resolve(m)); + + const result = await service.updateMonitorStatus(makeStatusResponse({ status: false }), makeCheck({ status: false })); + + expect(result.monitor.status).toBe("down"); + }); + it("during warmup (window not full) the stored status tracks the raw check", async () => { // Early-return branch: monitor.status is written to reflect the current check and // no status-changed notification is emitted. This is the service's documented warmup behavior. From 9e24cc655c1dee33be8c814bd1d232469176a745 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 13:21:14 -0700 Subject: [PATCH 5/7] remove notifications from monitors before deleting notifications --- server/src/service/infrastructure/notificationsService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/service/infrastructure/notificationsService.ts b/server/src/service/infrastructure/notificationsService.ts index b131c4ff9..a96ccecb6 100644 --- a/server/src/service/infrastructure/notificationsService.ts +++ b/server/src/service/infrastructure/notificationsService.ts @@ -200,8 +200,8 @@ export class NotificationsService implements INotificationsService { }; deleteById = async (id: string, teamId: string): Promise => { - const deleted = await this.notificationsRepository.deleteById(id, teamId); await this.monitorsRepository.removeNotificationFromMonitors(id); + const deleted = await this.notificationsRepository.deleteById(id, teamId); return deleted; }; } From 69bec42a66a84953a2546644234a789479cc47d8 Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 13:53:55 -0700 Subject: [PATCH 6/7] restrict editing user routes to regular users --- server/src/routes/authRoute.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/routes/authRoute.ts b/server/src/routes/authRoute.ts index fd951c6b1..af6fd4266 100755 --- a/server/src/routes/authRoute.ts +++ b/server/src/routes/authRoute.ts @@ -32,8 +32,8 @@ class AuthRoutes { this.router.patch("/users/:userId/password", verifyJWT, isAllowed(["superadmin"]), this.authController.editUserPasswordById); this.router.delete("/users/:userId", verifyJWT, isAllowed(["admin", "superadmin"]), this.authController.deleteUserById); - this.router.patch("/user", verifyJWT, upload.single("profileImage"), this.authController.editUser); - this.router.delete("/user", verifyJWT, this.authController.deleteUser); + this.router.patch("/user", verifyJWT, upload.single("profileImage"), isAllowed(["admin", "superadmin", "user"]), this.authController.editUser); + this.router.delete("/user", verifyJWT, isAllowed(["admin", "superadmin", "user"]), this.authController.deleteUser); } getRouter() { From be3b7c2e86276a9292849dc223bb7009e80b04da Mon Sep 17 00:00:00 2001 From: Alex Holliday Date: Wed, 15 Apr 2026 13:54:05 -0700 Subject: [PATCH 7/7] add demo role to list of roles --- client/src/Pages/Account/components/AddTeamMemberDialog.tsx | 1 + client/src/Pages/Account/components/InviteTeamMemberDialog.tsx | 1 + 2 files changed, 2 insertions(+) diff --git a/client/src/Pages/Account/components/AddTeamMemberDialog.tsx b/client/src/Pages/Account/components/AddTeamMemberDialog.tsx index 14765eb79..e584d3235 100644 --- a/client/src/Pages/Account/components/AddTeamMemberDialog.tsx +++ b/client/src/Pages/Account/components/AddTeamMemberDialog.tsx @@ -43,6 +43,7 @@ export const AddTeamMemberDialog = ({ const roleOptions: { value: UserRole; label: string }[] = [ { value: "admin", label: t("common.auth.roles.admin") }, { value: "user", label: t("common.auth.roles.user") }, + { value: "demo", label: t("common.auth.roles.demo") }, ]; const handleClose = () => { diff --git a/client/src/Pages/Account/components/InviteTeamMemberDialog.tsx b/client/src/Pages/Account/components/InviteTeamMemberDialog.tsx index e7c42a1ac..cf72e3d24 100644 --- a/client/src/Pages/Account/components/InviteTeamMemberDialog.tsx +++ b/client/src/Pages/Account/components/InviteTeamMemberDialog.tsx @@ -48,6 +48,7 @@ export const InviteTeamMemberDialog = ({ const roleOptions: { value: UserRole; label: string }[] = [ { value: "admin", label: t("common.auth.roles.admin") }, { value: "user", label: t("common.auth.roles.user") }, + { value: "demo", label: t("common.auth.roles.demo") }, ]; const handleGenerateToken = async (data: InviteFormData) => {