Fix/webhook cascade

[Aryainguz]

Summary

Deleting a webhook left every alert whose channel.webhookId pointed at it broken indefinitely silently logging WEBHOOK_ERROR on every evaluation tick without any user-visible notice.

A user can delete a webhook (to rotate credentials, rename it, etc.) not realising that every alert attached to it just died silently.

This fix nulls the channel field on all referencing alerts before the webhook is removed, so they continue to evaluate but no longer attempt delivery to the deleted endpoint. The alert data, thresholds, and history are fully preserved.

Dashboard deletion cascades via deleteDashboardAlerts; SavedSearch deletion via deleteSavedSearchAlerts. Webhook deletion had no equivalent this PR closes that gap.

Fixes #2590

Changed file: packages/api/src/routers/api/webhooks.ts

+import Alert, { AlertState } from '@/models/alert';
 import Webhook, { WebhookService } from '@/models/webhook';

 router.delete('/:id', ..., async (req, res, next) => {
   const teamId = req.user?.team;
   if (teamId == null) return res.sendStatus(403);

+  // Null the channel on referencing alerts so they don't fire WEBHOOK_ERROR on every tick.
+  await Alert.updateMany(
+    { 'channel.webhookId': req.params.id, team: teamId },
+    { $set: { channel: { type: null } } },
+  );
+
   await Webhook.findOneAndDelete({ _id: req.params.id, team: teamId });
   res.json({});
 });

Changes:

• packages/api/src/routers/api/webhooks.ts: Added Alert.updateMany inside the DELETE /:id handler to cascade-null the channel of all attached alerts.
• packages/api/src/routers/api/__tests__/webhooks.test.ts: Added an integration test verifying the cascade correctly nulls the alert channel in the database when the webhook is deleted.
• /.changeset/fix-webhook-cascade-delete.md

Local Validations

[changeset-bot]

🦋 Changeset detected

Latest commit: 02258cc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

@Aryainguz is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[greptile-apps]

Greptile Summary

This PR closes a silent failure mode where deleting a webhook left every alert whose channel.webhookId pointed at it logging WEBHOOK_ERROR on every evaluation tick with no user-visible feedback. The fix adds an Alert.updateMany call in the DELETE /:id handler that nulls the channel field on all referencing alerts (scoped to the same team) before the webhook document is removed. The alert evaluation path handles channel.type === null gracefully — getDefaultExternalAction returns null, no {{notify}} action is appended to the template, and no delivery is attempted.

• webhooks.ts: Alert.updateMany with { 'channel.webhookId': id, team: teamId } and { $set: { channel: { type: null } } } is inserted before Webhook.findOneAndDelete; the filter correctly applies team isolation to both operations.
• webhooks.test.ts: Integration test creates a webhook and a referencing alert, deletes the webhook, and asserts the alert's channel.type is null; the previously-flagged missing threshold/thresholdType fields are now present.
• .changeset: Patch-level changeset entry added for the fix.

Confidence Score: 5/5

Safe to merge — the cascade correctly nulls referencing alert channels within the same team before removing the webhook, and the alert evaluation path handles a null channel type gracefully without throwing errors.

Both the updateMany and findOneAndDelete operations filter by team, preserving cross-team isolation. The alert runtime (renderAlertTemplate) already handles channel.type === null by returning null from getDefaultExternalAction and skipping any delivery — no new error surface is introduced. The only open item is a minor test fixture inconsistency (source: 'logs') that has no runtime impact.

No files require special attention. The test fixture uses a non-enum source value but this does not affect correctness.

Important Files Changed

Filename	Overview
packages/api/src/routers/api/webhooks.ts	Adds Alert.updateMany before webhook deletion to null referencing alert channels; team isolation is correctly applied to both operations.
packages/api/src/routers/api/tests/webhooks.test.ts	New cascade test correctly provides threshold/thresholdType, but uses source: 'logs' which is not a valid AlertSource enum value (though Mongoose accepts it due to no schema enum constraint).
.changeset/fix-webhook-cascade-delete.md	Adds a patch-level changeset entry for the webhook cascade fix.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant WebhookRouter as DELETE /webhooks/:id
    participant AlertModel as Alert.updateMany
    participant WebhookModel as Webhook.findOneAndDelete

    Client->>WebhookRouter: DELETE /webhooks/:id
    WebhookRouter->>WebhookRouter: Verify teamId (403 if null)
    WebhookRouter->>AlertModel: "updateMany({ channel.webhookId: id, team: teamId }, { $set: { channel: { type: null } } })"
    AlertModel-->>WebhookRouter: alerts updated (channel nulled)
    WebhookRouter->>WebhookModel: "findOneAndDelete({ _id: id, team: teamId })"
    WebhookModel-->>WebhookRouter: webhook removed
    WebhookRouter-->>Client: "200 {}"

    Note over AlertModel,WebhookModel: Two separate operations — not wrapped in a transaction

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant WebhookRouter as DELETE /webhooks/:id
    participant AlertModel as Alert.updateMany
    participant WebhookModel as Webhook.findOneAndDelete

    Client->>WebhookRouter: DELETE /webhooks/:id
    WebhookRouter->>WebhookRouter: Verify teamId (403 if null)
    WebhookRouter->>AlertModel: "updateMany({ channel.webhookId: id, team: teamId }, { $set: { channel: { type: null } } })"
    AlertModel-->>WebhookRouter: alerts updated (channel nulled)
    WebhookRouter->>WebhookModel: "findOneAndDelete({ _id: id, team: teamId })"
    WebhookModel-->>WebhookRouter: webhook removed
    WebhookRouter-->>Client: "200 {}"

    Note over AlertModel,WebhookModel: Two separate operations — not wrapped in a transaction

Loading

_{Reviews (2): Last reviewed commit: "test: fix missing required threshold fie..." | Re-trigger Greptile}

[github-actions]

Deep Review

This PR nulls the channel field on alerts referencing a webhook before the webhook is deleted, so attached alerts keep evaluating but stop delivering to the dead endpoint. The auth/tenant-scoping is correct (team: teamId on both the updateMany and the findOneAndDelete), req.params.id is a zod-validated ObjectId used as a query value (no NoSQL-operator injection), and { $set: { channel: { type: null } } } correctly matches the AlertChannel { type: null } union variant so downstream getDefaultExternalAction returns null and no WEBHOOK_ERROR is recorded. No ship-blocker issues.

✅ No critical issues found.

🟡 P2 — recommended

• packages/api/src/routers/api/webhooks.ts:359 — The cascade only rewrites alerts matched by channel.webhookId, but an alert can also route to a webhook via an @webhook-<id> token in alert.message; renderAlertTemplate runs translateExternalActionsToInternal(alert.message) unconditionally, so getPopulatedChannel still resolves the now-deleted id and throws Webhook not found, recording WEBHOOK_ERROR on every tick for those alerts.
  • Fix: Also scan and rewrite @webhook-<deletedId> tokens in alert.message, or make getPopulatedChannel treat a missing webhook as a no-op instead of throwing.
  • _{adversarial, correctness}
• packages/api/src/routers/api/__tests__/webhooks.test.ts:190 — The new test creates a single referencing alert and asserts only channel.type is null, leaving the tenant-scoping, multi-document, and selectivity guarantees of the updateMany untested; a regression dropping the team: teamId filter or widening the query would still pass.
  • Fix: Add cases asserting an alert in another team with the same webhookId is untouched, that multiple referencing alerts are all nulled, and that alerts referencing a different webhook are unchanged.
  • _{testing, security, kieran-typescript, project-standards}
• packages/api/src/routers/api/webhooks.ts:361 — After nulling, a channel-only alert with no @webhook token in its message evaluates silently: notifyChannel is never reached and no executionError is recorded, so a single webhook deletion can silently darken many alerts with no operator-visible signal.
  • Fix: Record a distinct channel removed execution error or expose a broken/no-channel alert state so the degraded alerts are discoverable.
  • _adversarial

🔵 P3 nitpicks (5)

• packages/api/src/routers/api/webhooks.ts:359 — The updateMany and findOneAndDelete are not wrapped in a transaction, so a delete failure after the update leaves alerts channel-nulled while the webhook persists; ordering is the safe direction and the writes are idempotent, but the pair is non-atomic.
  • Fix: Wrap both writes in a mongoose session.withTransaction so the channel-null and webhook delete commit atomically.
  • _{reliability, correctness, maintainability}
• packages/api/src/routers/api/__tests__/webhooks.test.ts:207 — The fixture sets source: 'logs', which is not a member of AlertSource (saved_search | tile) and persists only because the schema field is an unconstrained String, so the test does not reflect a production-shaped alert.
  • Fix: Use AlertSource.SAVED_SEARCH (with groupBy/savedSearch) or AlertSource.TILE.
  • _{correctness, testing, kieran-typescript}
• packages/api/src/routers/api/webhooks.ts:359 — The cascade is inlined in the router while the sibling alert cascades (deleteDashboardAlerts, deleteSavedSearchAlerts) live as named helpers in controllers/alerts.ts, coupling the router to the AlertChannel null-shape literal.
  • Fix: Extract a deleteWebhookAlerts(webhookId, teamId) helper in controllers/alerts.ts and call it from the handler.
  • _{maintainability, project-standards}
• packages/api/src/routers/api/webhooks.ts:359 — A create-alert request that validates the webhook exists and then persists after the delete's updateMany runs can produce an orphaned alert the cascade never touches, re-introducing the WEBHOOK_ERROR state the fix targets.
  • Fix: Re-check webhook existence at alert save time or perform the deletion transactionally so concurrent creates cannot orphan an alert.
• packages/api/src/routers/api/__tests__/webhooks.test.ts:219 — The single channel.type assertion does not verify the PR's stated preservation guarantees; an accidental top-level $set wiping sibling fields would not be caught.
  • Fix: Assert threshold, thresholdType, and state are unchanged after the cascade.
  • _{testing, correctness}

---

Reviewers (10): correctness, testing, maintainability, project-standards, security, reliability, kieran-typescript, adversarial, agent-native, learnings.

Testing gaps: No cross-team isolation test for the cascade; updateMany multi-document behavior exercised with only one alert; no assertion that a nulled-channel alert still evaluates without emitting WEBHOOK_ERROR.

[pulpdrew]

Thanks for taking a look at this @Aryainguz

Deleting a webhook left every alert whose channel.webhookId pointed at it broken indefinitely silently logging WEBHOOK_ERROR on every evaluation tick without any user-visible notice.

A user can delete a webhook (to rotate credentials, rename it, etc.) not realising that every alert attached to it just died silently.

This is indeed a problem, but IMO this change makes it slightly worse than before because the user can still silently kill many alerts, but now the alert history UI does not indicate any problem to the user.

As the alert task is currently implemented, alerts with a broken Webhook reference will still be evaluated and save their state to the AlertHistory. So the alerts haven't died, their state is just shadowed in the AlertHistory UX with the webhook error. So I think that a better approach here might be to either:

1. Error on webhook deletion, encouraging the user to migrate the alerts before deleting the webhook; OR
2. Update the alert history UX to indicate both the WEBHOOK_ERROR and the actual alert evaluation state