refactor(alerts): add a notification dispatch seam

[jordan-simonovski]

Why

Alert notifications are delivered inline, on the evaluation tick. That is the right default, but it hardwires one delivery strategy into the render path: anything wanting to queue deliveries, bound concurrency, or survive across ticks has to edit renderAlertTemplate itself.

This adds the seam so delivery becomes swappable. No behaviour changes — the inline dispatcher is the default and does exactly what the code did before.

The contract worth reading

dispatch() has two legitimate implementations with different timing, and the difference matters to callers:

• inline resolves after delivery, so errors propagate to the caller and keep landing in executionErrors.
• queueing resolves after enqueue; its errors surface in its own logs and metrics, not to the caller.

Anything depending on synchronous error propagation from dispatch() is depending on the inline implementation specifically. That is documented on the interface.

Naming

NotificationJob.populatedChannel is deliberately not called channel. A job may carry both a serializable channel reference and the resolved document; reusing one name for both makes the two indistinguishable. Only the resolved form exists here today — the name is chosen so it stays correct when the other appears.

Not a behaviour change

checkAlerts.int.test.ts (277) and renderAlertTemplate.int.test.ts (74) pass unmodified — that is the gate. The dispatcher parameter is optional and defaults to inline, so no existing caller changed. eventId is byte-identical: same hash, same inputs, same place.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 1cd58ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
hyperdx-oss	Ready	Preview	Aug 19, 2026 12:56pm
hyperdx-storybook	Ready	Preview	Aug 19, 2026 12:56pm

[greptile-apps]

Greptile Summary

The PR extracts alert delivery behind an injectable notification dispatcher while preserving synchronous inline delivery as the default.

• Adds the NotificationJob and NotificationDispatcher contracts.
• Adds and tests an inline dispatcher that preserves delivery timing and error propagation.
• Routes rendered alert notifications through the dispatcher seam.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/api/src/tasks/checkAlerts/notifications.ts	Introduces the notification job and dispatcher contracts plus a synchronous default implementation that delegates to the existing transport.
packages/api/src/tasks/checkAlerts/template.ts	Constructs notification jobs and dispatches them through an optional dispatcher while retaining the inline implementation by default.
packages/api/src/tasks/checkAlerts/tests/notifications.test.ts	Verifies that inline dispatch waits for delivery, propagates delivery errors, and has a no-op shutdown.

Sequence Diagram

sequenceDiagram
  participant Alert as Alert evaluation
  participant Template as renderAlertTemplate
  participant Dispatcher as NotificationDispatcher
  participant Transport as deliverToChannel
  Alert->>Template: Render notification
  Template->>Dispatcher: dispatch(job)
  alt Default inline dispatcher
    Dispatcher->>Transport: Deliver message
    Transport-->>Dispatcher: Success or error
    Dispatcher-->>Template: Resolve after delivery
  else Future queueing dispatcher
    Dispatcher-->>Template: Resolve after enqueue
  end

Loading

_{Reviews (2): Last reviewed commit: "refactor(alerts): add a notification dis..." | Re-trigger Greptile}

[github-actions]

Deep Review

Refactor introducing a notification dispatch seam in the alerts task: a new NotificationDispatcher interface, NotificationJob type, and default InlineNotificationDispatcher, with renderAlertTemplate gaining an optional dispatcher parameter. Correctness and adversarial reviewers independently confirmed the inline path is behavior-preserving — arguments map 1:1 to the prior deliverToChannel call, eventId is computed from an unchanged objectHash input, and delivery rejections still propagate through the promised-handlebars helper into executionErrors.

✅ No critical issues found.

🟡 P2 -- recommended

• packages/api/src/tasks/checkAlerts/template.ts:309 -- The new injectable dispatcher parameter has no test that wires a custom dispatcher through renderAlertTemplate, so the swappability this change adds is unverified and no test pins the shape of the NotificationJob handed to dispatch().
  • Fix: Add a test that calls renderAlertTemplate with a fake NotificationDispatcher and asserts dispatch receives a job with the expected eventId, alertId, group, populatedChannel, and message fields.
  • _{testing, api-contract, kieran-typescript}

🔵 P3 nitpicks (4)

• packages/api/src/tasks/checkAlerts/notifications.ts:18 -- NotificationJob.teamId is declared optional but never populated at the sole construction site, leaving a field that always reads undefined.
  • Fix: Populate teamId from the caller's team scope in template.ts, or drop the field until a consumer reads it.
  • _{maintainability, kieran-typescript, correctness}
• packages/api/src/tasks/checkAlerts/notifications.ts:1 -- PopulatedAlertChannel is imported from @/tasks/checkAlerts/transports here while template.ts imports the same type from @/tasks/checkAlerts/providers, contradicting the re-export's stated single-source intent.
  • Fix: Standardize both sibling files on one canonical import path for the type.
  • _{maintainability, kieran-typescript}
• packages/api/src/tasks/checkAlerts/notifications.ts:63 -- shutdown(deadlineMs) is a no-op with an unused parameter and no caller in any process-lifecycle path, so a later queueing dispatcher could ship without a flush hook and silently drop buffered notifications on exit.
  • Fix: Either wire a module-level dispatcher.shutdown into the worker graceful-shutdown path, or drop the method until an implementation needs it.
  • _{maintainability, adversarial, reliability}
• packages/api/src/tasks/checkAlerts/notifications.ts:57 -- The dispatcher interface plus deliver-fn injection currently has one implementation and no overriding caller, adding indirection ahead of a consumer.
  • Fix: Consider collapsing to a direct deliverNotification call and reintroducing the interface when a second dispatcher actually lands.
  • _{maintainability}

Pre-existing (not counted toward verdict)

• packages/api/src/tasks/checkAlerts/template.ts:1 -- template.ts was already 540 lines before this change (over the project's 300-line guideline) and grows to ~548; the diff enlarges rather than causes the violation.
  • Fix: Split the notify-helper/delivery logic out of template.ts in a follow-up.

---

Reviewers (9): correctness, adversarial, testing, maintainability, project-standards, api-contract, reliability, kieran-typescript, agent-native.

Testing gaps:

• deliverNotification (the default deliver fn wired to deliverToChannel) has no direct unit test; coverage is only transitive through the existing integration suite.
• No regression test asserts that a dispatch() rejection still surfaces into executionErrors after the seam was introduced.
• No test pins the "eventId byte-identical" claim across the refactor (the objectHash input shape, including the grouped-alert branch).

Coverage: Two low-confidence single-reviewer findings were suppressed below the confidence gate — eventId being duplicated at the NotificationJob top level and inside message (currently set from one variable, so it cannot diverge today), and the test's opaque fakeJob: any fixture (project-standards flagged this as an accepted convention already pervasive in the directory). A residual note: no changeset was added for this packages/api change; the author should confirm it qualifies for the internal-refactor exemption.