From 51fbc63d759bcc75bfc0502aa2a9424fd593f56e Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 30 Jul 2026 16:57:09 +0800 Subject: [PATCH] fix: update monitor.status for private location monitors (#2496) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: update monitor.status for private location monitors Private location monitors were not updating the monitor.status field when checks failed/recovered, causing the dashboard to always show them as 'active' even when failing. The cloud checker updates monitor.status on status transitions, but the private location handler (updateStatusPrivate) only updated the privateLocationMonitorStatus table without touching monitor.status. This fix adds the same monitor.status update logic to all three status transition cases (error, degraded, active) to match the cloud checker behavior. Fixes dashboard status display and 'Failing'/'Normal' counts for private location monitors. * fix: add missing inArray import from drizzle-orm * ci: apply automated fixes * fix: only update monitor status from private locations for private-only monitors * ci: apply automated fixes * feat: add incident creation and resolution for private location monitors * feat: add multi-location threshold logic to private location status updates - Query all private location statuses for a monitor - Require >=50% agreement OR single location before updating monitor.status - Matches cloud checker threshold behavior (affectedCount >= total/2 || total === 1) - Fix redundant null check in hasCloudRegions Addresses Claude review feedback: prevents single location from overwriting monitor.status when multiple locations are attached to a monitor. * test: add coverage for multi-location threshold logic - Add private-only monitor fixture (no cloud regions) - Add 3 private locations for threshold testing - Test 1: Threshold met (2/3 locations agree) → status updates - Test 2: Threshold not met (1/3 location) → status unchanged - Test 3: Cloud+private monitors → status unchanged by private reports Addresses Claude review feedback: ensures new code path has regression tests. * ci: apply automated fixes * fix: use total attached locations for threshold check - Query privateLocationToMonitors to get total ATTACHED locations - Use attachedLocations.length instead of reportedLocations.length - This ensures threshold uses 3 attached locations, not just 1 reported - Also removed invalid autoResolved property from audit log metadata Fixes test failure and TypeScript error. * fix: gate notifications with threshold check - Only trigger notifications when threshold is met - Prevents multiple alerts when locations report below threshold - Applies to alert, degraded, and recovery notifications Fixes test failure: assertSpyCalls expected 1 but got 2 * fix: preserve notification behavior for cloud+private monitors - Changed gate from AND to OR: hasCloudRegions || shouldUpdateMonitorStatus - Cloud+private monitors: always trigger notifications (original behavior) - Private-only monitors: only trigger when threshold met (new behavior) Fixes 4 failing tests that use monitors with cloud regions. * fix: address 5 violations in private location handling V1 (P1): Filter status query to only attached locations - Prevents detached location stale status from counting toward threshold - Added inArray filter on attachedLocationIds V2 (P1): Gate incident creation with threshold check - Only create incidents when threshold met for private-only monitors - Prevents premature incident creation (1/3 errors) V3 (P2): Pass incident ID to notifications - Capture incident ID in error and recovery cases - Pass to triggerNotifications for provider context V4 (P1): Gate incident resolution with threshold check - Only resolve when threshold met and monitor recovered - Prevents premature incident closure (1/3 recovery) Test (P2): Add PRIVATE_ONLY_MONITOR_ID cleanup to afterEach - Prevents test data leakage between tests * fix: resolve compilation errors in private location handler - Move incidentId declaration outside switch to fix redeclaration error - Filter null values from attachedLocationIds for inArray type safety - Fixes TS2451 (redeclaration) and TS2769 (inArray type mismatch) * ci: apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../src/checker/private-location.test.ts | 221 +++++++++++++++++- .../workflows/src/checker/private-location.ts | 117 +++++++--- 2 files changed, 307 insertions(+), 31 deletions(-) diff --git a/apps/workflows/src/checker/private-location.test.ts b/apps/workflows/src/checker/private-location.test.ts index 527cefcbf..8da20e5fb 100644 --- a/apps/workflows/src/checker/private-location.test.ts +++ b/apps/workflows/src/checker/private-location.test.ts @@ -1,4 +1,4 @@ -import { and, db, eq } from "@openstatus/db"; +import { and, db, eq, or } from "@openstatus/db"; import { monitor, notification, @@ -39,6 +39,9 @@ const INACTIVE_MONITOR_ID = 9102; const TEST_NOTIFICATION_ID = 9101; const TEST_LOCATION_ID = 9001; const UNATTACHED_LOCATION_ID = 9002; +const PRIVATE_ONLY_MONITOR_ID = 9103; +const PRIVATE_LOCATION_2_ID = 9003; +const PRIVATE_LOCATION_3_ID = 9004; const cronSecret = env().CRON_SECRET; @@ -105,6 +108,15 @@ describe("updateStatusPrivate", () => { periodicity: "1m", regions: "ams", }, + { + id: PRIVATE_ONLY_MONITOR_ID, + workspaceId, + active: true, + url: "https://private-only.test", + name: "Private-Only Monitor (No Cloud Regions)", + periodicity: "1m", + regions: "", + }, ]) .onConflictDoNothing() .run(); @@ -147,6 +159,61 @@ describe("updateStatusPrivate", () => { }) .onConflictDoNothing() .run(); + + // Additional private locations for multi-location tests + await db + .insert(privateLocation) + .values([ + { + id: PRIVATE_LOCATION_2_ID, + name: "Test Office 2", + token: "test-private-location-token-2", + workspaceId, + createdAt: new Date(), + }, + { + id: PRIVATE_LOCATION_3_ID, + name: "Test Office 3", + token: "test-private-location-token-3", + workspaceId, + createdAt: new Date(), + }, + ]) + .onConflictDoNothing() + .run(); + + // Link all three locations to private-only monitor + await db + .insert(privateLocationToMonitors) + .values([ + { + privateLocationId: TEST_LOCATION_ID, + monitorId: PRIVATE_ONLY_MONITOR_ID, + createdAt: new Date(), + }, + { + privateLocationId: PRIVATE_LOCATION_2_ID, + monitorId: PRIVATE_ONLY_MONITOR_ID, + createdAt: new Date(), + }, + { + privateLocationId: PRIVATE_LOCATION_3_ID, + monitorId: PRIVATE_ONLY_MONITOR_ID, + createdAt: new Date(), + }, + ]) + .onConflictDoNothing() + .run(); + + // Link first location to private-only monitor for notifications + await db + .insert(notificationsToMonitors) + .values({ + monitorId: PRIVATE_ONLY_MONITOR_ID, + notificationId: TEST_NOTIFICATION_ID, + }) + .onConflictDoNothing() + .run(); }); afterAll(async () => { @@ -174,8 +241,52 @@ describe("updateStatusPrivate", () => { .delete(notification) .where(eq(notification.id, TEST_NOTIFICATION_ID)) .run(); + // Cleanup private-only monitor fixtures + await db + .delete(privateLocationMonitorStatus) + .where( + eq(privateLocationMonitorStatus.monitorId, PRIVATE_ONLY_MONITOR_ID), + ) + .run(); + await db + .delete(notificationTrigger) + .where(eq(notificationTrigger.monitorId, PRIVATE_ONLY_MONITOR_ID)) + .run(); + await db + .delete(notificationsToMonitors) + .where(eq(notificationsToMonitors.monitorId, PRIVATE_ONLY_MONITOR_ID)) + .run(); + await db + .delete(privateLocationToMonitors) + .where( + or( + eq(privateLocationToMonitors.privateLocationId, TEST_LOCATION_ID), + eq( + privateLocationToMonitors.privateLocationId, + PRIVATE_LOCATION_2_ID, + ), + eq( + privateLocationToMonitors.privateLocationId, + PRIVATE_LOCATION_3_ID, + ), + ), + ) + .run(); + await db + .delete(privateLocation) + .where( + or( + eq(privateLocation.id, PRIVATE_LOCATION_2_ID), + eq(privateLocation.id, PRIVATE_LOCATION_3_ID), + ), + ) + .run(); await db.delete(monitor).where(eq(monitor.id, TEST_MONITOR_ID)).run(); await db.delete(monitor).where(eq(monitor.id, INACTIVE_MONITOR_ID)).run(); + await db + .delete(monitor) + .where(eq(monitor.id, PRIVATE_ONLY_MONITOR_ID)) + .run(); }); beforeEach(() => { @@ -208,10 +319,20 @@ describe("updateStatusPrivate", () => { .delete(privateLocationMonitorStatus) .where(eq(privateLocationMonitorStatus.monitorId, TEST_MONITOR_ID)) .run(); + await db + .delete(privateLocationMonitorStatus) + .where( + eq(privateLocationMonitorStatus.monitorId, PRIVATE_ONLY_MONITOR_ID), + ) + .run(); await db .delete(notificationTrigger) .where(eq(notificationTrigger.monitorId, TEST_MONITOR_ID)) .run(); + await db + .delete(notificationTrigger) + .where(eq(notificationTrigger.monitorId, PRIVATE_ONLY_MONITOR_ID)) + .run(); }); test("rejects a wrong CRON_SECRET with 401", async () => { @@ -362,4 +483,102 @@ describe("updateStatusPrivate", () => { expect(res.status).toBe(200); assertSpyCalls(mockEmailSendAlert, 0); }); + + test("private-only monitor: updates status when threshold met (2/3 locations agree)", async () => { + // First location reports error → 1/3 < 50% → should NOT update monitor status + const res1 = await post({ + monitorId: String(PRIVATE_ONLY_MONITOR_ID), + privateLocationId: String(TEST_LOCATION_ID), + status: "error", + cronTimestamp: 9310010, + statusCode: 500, + message: "down", + }); + expect(res1.status).toBe(200); + + const monitorAfter1 = await db + .select() + .from(monitor) + .where(eq(monitor.id, PRIVATE_ONLY_MONITOR_ID)) + .get(); + expect(monitorAfter1?.status).toBe("active"); // Threshold not met + + // Second location reports error → 2/3 >= 50% → SHOULD update monitor status + const res2 = await post({ + monitorId: String(PRIVATE_ONLY_MONITOR_ID), + privateLocationId: String(PRIVATE_LOCATION_2_ID), + status: "error", + cronTimestamp: 9310020, + statusCode: 500, + message: "down", + }); + expect(res2.status).toBe(200); + + const monitorAfter2 = await db + .select() + .from(monitor) + .where(eq(monitor.id, PRIVATE_ONLY_MONITOR_ID)) + .get(); + expect(monitorAfter2?.status).toBe("error"); + assertSpyCalls(mockEmailSendAlert, 1); + }); + + test("does not change monitor.status from error when 1/3 locations report degraded (threshold not met)", async () => { + // First seed the monitor to "error" state via 2/3 locations agreeing + await post({ + monitorId: String(PRIVATE_ONLY_MONITOR_ID), + privateLocationId: String(TEST_LOCATION_ID), + status: "error", + cronTimestamp: 9320000, + statusCode: 500, + message: "down", + }); + await post({ + monitorId: String(PRIVATE_ONLY_MONITOR_ID), + privateLocationId: String(PRIVATE_LOCATION_2_ID), + status: "error", + cronTimestamp: 9320005, + statusCode: 500, + message: "down", + }); + + // Now try degraded with only 1/3 → should NOT change + const res = await post({ + monitorId: String(PRIVATE_ONLY_MONITOR_ID), + privateLocationId: String(PRIVATE_LOCATION_3_ID), + status: "degraded", + cronTimestamp: 9320010, + statusCode: 200, + }); + expect(res.status).toBe(200); + + const monitorAfter = await db + .select() + .from(monitor) + .where(eq(monitor.id, PRIVATE_ONLY_MONITOR_ID)) + .get(); + // Monitor should remain "error" because 1/3 < 50% threshold + expect(monitorAfter?.status).toBe("error"); + }); + + test("does not update monitor.status for monitors with cloud regions", async () => { + // TEST_MONITOR_ID has regions: "ams" (cloud region) + const res = await post({ + monitorId: String(TEST_MONITOR_ID), + privateLocationId: String(TEST_LOCATION_ID), + status: "error", + cronTimestamp: 9330010, + statusCode: 500, + message: "down", + }); + expect(res.status).toBe(200); + + const monitorAfter = await db + .select() + .from(monitor) + .where(eq(monitor.id, TEST_MONITOR_ID)) + .get(); + // Monitor has cloud regions, so private status updates should not affect it + expect(monitorAfter?.status).toBe("active"); + }); }); diff --git a/apps/workflows/src/checker/private-location.ts b/apps/workflows/src/checker/private-location.ts index 2907b8d6a..f4653d543 100644 --- a/apps/workflows/src/checker/private-location.ts +++ b/apps/workflows/src/checker/private-location.ts @@ -235,6 +235,22 @@ export async function updateStatusPrivate(c: Context) { switch (status) { case "error": + // Update monitor status for private-only monitors + if ( + !hasCloudRegions && + shouldTriggerIncident && + monitor.status !== "error" + ) { + logger.info("Monitor status changed to error", { + monitor_id: monitor.id, + workspace_id: monitor.workspaceId, + }); + await db + .update(schema.monitor) + .set({ status: "error" }) + .where(eq(schema.monitor.id, monitorIdNumber)); + } + // Create incident only if private-only monitor AND threshold met if (shouldTriggerIncident) { try { @@ -310,18 +326,37 @@ export async function updateStatusPrivate(c: Context) { latency, }, }); - triggeredNotifications = await triggerNotifications({ - monitorId, - statusCode, - message, - notifType: "alert", - cronTimestamp, - regions, - latency, - incidentId: incident?.id, - }); + // Trigger notifications for cloud monitors always, private-only when threshold met + if (hasCloudRegions || shouldTriggerIncident) { + triggeredNotifications = await triggerNotifications({ + monitorId, + statusCode, + message, + notifType: "alert", + cronTimestamp, + regions, + latency, + incidentId: incident?.id, + }); + } break; case "degraded": + // Update monitor status for private-only monitors + if ( + !hasCloudRegions && + shouldTriggerIncident && + monitor.status !== "degraded" + ) { + logger.info("Monitor status changed to degraded", { + monitor_id: monitor.id, + workspace_id: monitor.workspaceId, + }); + await db + .update(schema.monitor) + .set({ status: "degraded" }) + .where(eq(schema.monitor.id, monitorIdNumber)); + } + // Resolve incident if private-only monitor AND threshold met if (shouldTriggerIncident) { try { @@ -354,18 +389,37 @@ export async function updateStatusPrivate(c: Context) { latency, }, }); - triggeredNotifications = await triggerNotifications({ - monitorId, - statusCode, - message, - notifType: "degraded", - cronTimestamp, - regions, - latency, - incidentId: incident?.id, - }); + // Trigger notifications for cloud monitors always, private-only when threshold met + if (hasCloudRegions || shouldTriggerIncident) { + triggeredNotifications = await triggerNotifications({ + monitorId, + statusCode, + message, + notifType: "degraded", + cronTimestamp, + regions, + latency, + incidentId: incident?.id, + }); + } break; case "active": + // Update monitor status for private-only monitors + if ( + !hasCloudRegions && + shouldTriggerIncident && + monitor.status !== "active" + ) { + logger.info("Monitor status changed to active", { + monitor_id: monitor.id, + workspace_id: monitor.workspaceId, + }); + await db + .update(schema.monitor) + .set({ status: "active" }) + .where(eq(schema.monitor.id, monitorIdNumber)); + } + // Resolve incident if private-only monitor AND threshold met if (shouldTriggerIncident) { try { @@ -395,16 +449,19 @@ export async function updateStatusPrivate(c: Context) { latency, }, }); - triggeredNotifications = await triggerNotifications({ - monitorId, - statusCode, - message, - notifType: "recovery", - cronTimestamp, - regions, - latency, - incidentId: incident?.id, - }); + // Trigger notifications for cloud monitors always, private-only when threshold met + if (hasCloudRegions || shouldTriggerIncident) { + triggeredNotifications = await triggerNotifications({ + monitorId, + statusCode, + message, + notifType: "recovery", + cronTimestamp, + regions, + latency, + incidentId: incident?.id, + }); + } break; } -- 2.51.2