feat: uptime alarms with deduplication and non-blocking evaluation#332
feat: uptime alarms with deduplication and non-blocking evaluation#332maoshuorz wants to merge 4 commits intodatabuddy-analytics:mainfrom
Conversation
- Add alarm_triggers table to track alarm trigger history (down/recovery events) - Add alarm evaluation service in uptime worker that fires notifications on site down and sends recovery notifications when site comes back up - Prevent duplicate notifications by checking last trigger state - Add alarms.listByWebsite and alarms.listTriggers API endpoints - Add UptimeAlarms component to pulse page with: - List of assigned alarms with enable/disable toggles - Quick-create sheet for new uptime alarms - Trigger history view showing down/recovery events - Alarm count badge in the pulse page header - Add @databuddy/notifications dependency to uptime app
…statement, sort imports
|
|
|
@maoshuorz is attempting to deploy a commit to the Databuddy OSS Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
recheck |
Greptile SummaryThis PR adds a complete uptime alarm system: a new Key issues found:
Confidence Score: 2/5
Last reviewed commit: 138bb1a |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| case "email": { | ||
| // Email requires server-side provider; skip in uptime worker | ||
| results.push({ | ||
| channel: "email", | ||
| success: false, | ||
| error: "Email not available in uptime worker", | ||
| }); |
There was a problem hiding this comment.
Email channel silently fails for all users
The email case always records success: false with the error "Email not available in uptime worker", yet the UI (uptime-alarms.tsx line 88 and alarm-sheet.tsx line 70) shows Email as a fully selectable notification channel. Any user who configures an email notification will silently receive no alerts with no indication in the UI that the channel is broken.
Either the email channel should be removed from the uptime alarm UI, or a prominent warning should be displayed when it's selected, or email dispatch should be implemented here (e.g., via a queue).
packages/rpc/src/routers/alarms.ts
Outdated
| context.headers.forEach((value, key) => { | ||
| headersObj[key] = value; | ||
| }); | ||
| const perm = permission === "read" ? "read" : "create"; |
There was a problem hiding this comment.
"delete" permission mapped to "create" in authorizeScope
Inside authorizeScope, any permission that isn't "read" (including "delete") is mapped to "create" before calling websitesApi.hasPermission:
const perm = permission === "read" ? "read" : "create";This means a user who only has "create" permission on the organization is allowed to delete alarms — a more destructive operation. The delete handler calls authorizeScope(context, undefined, alarm.organizationId, "delete") expecting stricter enforcement, but the check collapses to a "create" permission test. Consider using a dedicated "delete" or "admin" scope here, or documenting clearly that "create" covers both create and delete.
| interface Alarm { | ||
| id: string; | ||
| name: string; | ||
| description: string | null; | ||
| enabled: boolean; | ||
| notificationChannels: unknown; | ||
| slackWebhookUrl: string | null; | ||
| discordWebhookUrl: string | null; | ||
| webhookUrl: string | null; | ||
| webhookHeaders: unknown; | ||
| emailAddresses: unknown; | ||
| triggerType: string; | ||
| triggerConditions: unknown; | ||
| websiteId: string | null; | ||
| organizationId: string | null; | ||
| userId: string | null; | ||
| createdBy: string; | ||
| createdAt: Date | string; | ||
| updatedAt: Date | string; | ||
| deletedAt: Date | string | null; | ||
| } |
There was a problem hiding this comment.
Alarm interface duplicated across two files
A nearly identical Alarm interface is defined here and again in apps/dashboard/app/(main)/settings/notifications/_components/types.ts. The two definitions can drift independently. Consider importing the shared type from types.ts:
import type { Alarm } from "@/app/(main)/settings/notifications/_components/types";or, better, co-locating it in a shared module (e.g., @databuddy/shared).
- Fix permission mapping in authorizeScope: properly map "delete" permission instead of collapsing it to "create", preventing privilege escalation where create-only users could delete alarms - Remove email from selectable notification channels in UI since email sending is not yet implemented in the uptime worker - Add clear warning log when email channel is attempted - Consolidate duplicated Alarm interface — import from shared types
|
Thanks for the review @greptile-apps! All issues addressed:
|
|
All three fixes look great! Permission mapping ( Email channel is removed from the Alarm interface properly imports from Everything checks out. Nice work addressing the security issue and preventing the email misconfiguration footgun! 🎯 |
2 participants
Summary
Adds uptime alarm evaluation to the monitoring pipeline:
.catch(), never slows uptime checksalarmTriggerstable tracks trigger history (down/recovery events with HTTP codes)listByWebsiteandlistTriggersfor website-scoped alarm accessChanges
apps/uptime/src/lib/alarm-trigger.ts— Core alarm evaluation service with dedup logicapps/uptime/src/index.ts— Non-blocking alarm evaluation after each uptime checkapps/dashboard/app/(main)/websites/[id]/pulse/_components/uptime-alarms.tsx— Full alarm management UIapps/dashboard/app/(main)/websites/[id]/pulse/page.tsx— Integrated UptimeAlarms componentpackages/db/src/drizzle/schema.ts— NewalarmTriggerstablepackages/db/src/drizzle/relations.ts— Trigger relationspackages/rpc/src/routers/alarms.ts— AddedlistByWebsite+listTriggersendpointsResolves #268
/claim