From a9053d34559429ea5bb183a89c844fa9621fdeb5 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Mon, 14 Sep 2026 15:20:50 +1000 Subject: [PATCH 1/3] feat(api): accept an AI agent as an alert notification channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An alert's notification channel becomes a discriminated union: a webhook, or an agent to hand the investigation to. The write path is shared, so the internal API, the external API v2 and the MCP saveAlert tool all accept the new shape and all enforce the same rules — the agent must belong to the team, and HDX_MANAGED_AGENTS_ENABLED must be on, so a channel cannot be saved on a deployment where dispatch would do nothing. Existing alerts are untouched. Channels are persisted as Mixed and read back permissively, so the union gates only what can be written; the previous schema was already a strict single-branch object, so nothing that used to validate stops validating. Alerts can now store an agent channel, but nothing dispatches one yet — that lands next. Until then an agent channel is reachable only through the API with the flag deliberately enabled, since the alert editor does not offer one. --- packages/api/openapi.json | 41 +++++++-- packages/api/src/controllers/alerts.ts | 24 +++++ .../tools/alerts/__tests__/schemas.test.ts | 37 ++++++++ .../api/src/mcp/tools/alerts/saveAlert.ts | 30 +++++-- packages/api/src/mcp/tools/alerts/schemas.ts | 21 ++++- packages/api/src/models/alert.ts | 6 ++ .../__tests__/agentChannelFlagOff.int.test.ts | 81 +++++++++++++++++ .../routers/api/__tests__/alerts.int.test.ts | 89 +++++++++++++++++++ packages/api/src/routers/api/alerts.ts | 12 +-- .../api/src/routers/external-api/v2/alerts.ts | 28 ++++-- packages/app/src/utils/alerts.ts | 1 + packages/common-utils/src/types.ts | 19 +++- 12 files changed, 359 insertions(+), 30 deletions(-) create mode 100644 packages/api/src/routers/api/__tests__/agentChannelFlagOff.int.test.ts diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 0a7b87fb39..0c065bc6be 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -173,13 +173,6 @@ ], "description": "Current alert state." }, - "AlertChannelType": { - "type": "string", - "enum": [ - "webhook" - ], - "description": "Channel type." - }, "AlertErrorType": { "type": "string", "enum": [ @@ -250,8 +243,12 @@ ], "properties": { "type": { - "$ref": "#/components/schemas/AlertChannelType", - "description": "Channel type. Must be \"webhook\" for webhook alerts." + "type": "string", + "enum": [ + "webhook" + ], + "description": "Channel type. Must be \"webhook\" for webhook alerts.", + "example": "webhook" }, "webhookId": { "type": "string", @@ -260,10 +257,36 @@ } } }, + "AlertChannelAgent": { + "type": "object", + "description": "AI agent notification target. A firing alert starts an investigation session on the referenced managed agent. Only available on deployments with managed agents enabled.", + "required": [ + "type", + "agentId" + ], + "properties": { + "type": { + "type": "string", + "enum": [ + "agent" + ], + "description": "Channel type. Must be \"agent\" for agent alerts.", + "example": "agent" + }, + "agentId": { + "type": "string", + "description": "Managed agent ID.", + "example": "65f5e4a3b9e77c001a789014" + } + } + }, "AlertChannel": { "oneOf": [ { "$ref": "#/components/schemas/AlertChannelWebhook" + }, + { + "$ref": "#/components/schemas/AlertChannelAgent" } ], "discriminator": { diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 59cbc183a8..1cc920c791 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -10,6 +10,7 @@ import { groupBy } from 'lodash'; import { Types } from 'mongoose'; import { z } from 'zod'; +import { IS_MANAGED_AGENTS_ENABLED } from '@/config'; import type { ObjectId } from '@/models'; import Alert, { AlertChannel, @@ -19,6 +20,7 @@ import Alert, { } from '@/models/alert'; import Connection from '@/models/connection'; import Dashboard, { IDashboard } from '@/models/dashboard'; +import ManagedAgent from '@/models/managedAgent'; import { ISavedSearch, SavedSearch } from '@/models/savedSearch'; import { Source } from '@/models/source'; import { IUser } from '@/models/user'; @@ -232,6 +234,28 @@ export const validateAlertInput = async ( throw new Api400Error('Webhook not found'); } + const agentIds = channels.filter(c => c.type === 'agent').map(c => c.agentId); + if (agentIds.length > 0) { + // The alert editor is flag-gated too; this rejects an agent channel saved + // via the API on a deployment where dispatch would silently do nothing. + if (!IS_MANAGED_AGENTS_ENABLED) { + throw new Api400Error( + 'AI agent notification channels are not enabled on this deployment (set HDX_MANAGED_AGENTS_ENABLED)', + ); + } + for (const agentId of agentIds) { + validateObjectId(agentId, 'Invalid agent ID'); + } + const uniqueAgentIds = [...new Set(agentIds)]; + const foundAgents = await ManagedAgent.countDocuments({ + _id: { $in: uniqueAgentIds }, + team: teamId, + }); + if (foundAgents !== uniqueAgentIds.length) { + throw new Api400Error('Agent not found'); + } + } + return refs; }; diff --git a/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts b/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts index ab280b7f69..931742d63a 100644 --- a/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts +++ b/packages/api/src/mcp/tools/alerts/__tests__/schemas.test.ts @@ -209,3 +209,40 @@ describe('mcpSaveAlertSchema isDelta dialect bridge', () => { }, ); }); + +describe('validateSaveAlertInput per-type channel id pairing', () => { + const ag = (id: string) => ({ type: 'agent' as const, agentId: id }); + + it('accepts a webhook channel with a webhookId and an agent channel with an agentId', () => { + expect( + validateSaveAlertInput({ ...baseInput, channels: [wh('a'), ag('b')] }), + ).toBeNull(); + }); + + it('rejects a webhook channel missing webhookId', () => { + expect( + validateSaveAlertInput({ + ...baseInput, + channel: foreignChannel({ type: 'webhook' }), + }), + ).toBe('webhookId is required for a webhook channel'); + }); + + it('rejects an agent channel missing agentId', () => { + expect( + validateSaveAlertInput({ + ...baseInput, + channel: foreignChannel({ type: 'agent' }), + }), + ).toBe('agentId is required for an agent channel'); + }); + + it('rejects when any entry in channels misses its per-type id', () => { + expect( + validateSaveAlertInput({ + ...baseInput, + channels: [wh('a'), foreignChannel({ type: 'agent' })], + }), + ).toBe('agentId is required for an agent channel'); + }); +}); diff --git a/packages/api/src/mcp/tools/alerts/saveAlert.ts b/packages/api/src/mcp/tools/alerts/saveAlert.ts index 9ed8c882a2..a006c46bc1 100644 --- a/packages/api/src/mcp/tools/alerts/saveAlert.ts +++ b/packages/api/src/mcp/tools/alerts/saveAlert.ts @@ -15,7 +15,7 @@ import { mcpUserError, validateObjectId, } from '@/mcp/utils/errors'; -import { AlertSource } from '@/models/alert'; +import { AlertChannel, AlertSource } from '@/models/alert'; import { convertExternalAlertChartConfigToInternal, translateAlertDocumentToExternalAlertWithChartConfig, @@ -48,11 +48,13 @@ export function registerSaveAlert({ 'Alerts monitor a saved search, a dashboard tile, or an inline chart ' + 'config (source "inline" + chartConfig — no saved search or dashboard ' + 'needed) and fire when the metric crosses a threshold. At least one ' + - 'webhook notification channel ' + - 'is required: pass "channels" for 1-10 targets, or the legacy singular ' + - '"channel" for one. Updates replace the alert configuration rather than ' + - 'merging it, so read the alert first and resend its full "channels" ' + - 'array to avoid dropping channels you did not mean to remove.', + 'notification channel is required — a webhook ' + + '({type:"webhook", webhookId}) or, on deployments with managed agents ' + + 'enabled, an AI agent investigation ({type:"agent", agentId}). Pass ' + + '"channels" for 1-10 targets, or the legacy singular "channel" for ' + + 'one. Updates replace the alert configuration rather than merging it, ' + + 'so read the alert first and resend its full "channels" array to ' + + 'avoid dropping channels you did not mean to remove.', inputSchema: mcpSaveAlertSchema, }, async input => { @@ -95,10 +97,24 @@ export function registerSaveAlert({ // Build the alert input matching the shape expected by controllers. const source = MCP_SOURCE_TO_ALERT_SOURCE[input.source]; + // The MCP schema keeps webhookId/agentId both optional on one object + // shape (the per-type pairing is enforced in validateSaveAlertInput), so + // narrow into the discriminated AlertChannel here. + const toAlertChannel = (c: { + type: 'webhook' | 'agent'; + webhookId?: string; + agentId?: string; + }): AlertChannel => + c.type === 'agent' + ? { type: 'agent', agentId: c.agentId ?? '' } + : { type: 'webhook', webhookId: c.webhookId ?? '' }; + const alertInput: AlertInput = { source, // `channel` is omitted; makeAlert mirrors it from channels[0]. - channels: input.channels ?? (input.channel ? [input.channel] : []), + channels: ( + input.channels ?? (input.channel ? [input.channel] : []) + ).map(toAlertChannel), interval: input.interval, threshold: input.threshold, thresholdType: input.thresholdType as AlertThresholdType, diff --git a/packages/api/src/mcp/tools/alerts/schemas.ts b/packages/api/src/mcp/tools/alerts/schemas.ts index a1258ff50b..b9c4b76550 100644 --- a/packages/api/src/mcp/tools/alerts/schemas.ts +++ b/packages/api/src/mcp/tools/alerts/schemas.ts @@ -106,11 +106,18 @@ const mcpAlertChartConfigSchema = z const mcpAlertChannelSchema = z .object({ type: z - .literal('webhook') + .enum(['webhook', 'agent']) .describe('Channel type for alert notifications.'), webhookId: z .string() + .optional() .describe('Webhook destination ID (required for webhook channel).'), + agentId: z + .string() + .optional() + .describe( + 'Managed agent ID (required for agent channel; a firing alert starts an AI investigation on this agent).', + ), }) .describe('Alert notification channel configuration.'); @@ -245,6 +252,18 @@ export function validateSaveAlertInput(data: McpSaveAlertInput): string | null { } } + // Per-type required channel id (the Zod schema keeps both ids optional so + // one object shape serves both types; enforce the pairing here). + for (const channel of [data.channel, ...(data.channels ?? [])]) { + if (!channel) continue; + if (channel.type === 'webhook' && !channel.webhookId) { + return 'webhookId is required for a webhook channel'; + } + if (channel.type === 'agent' && !channel.agentId) { + return 'agentId is required for an agent channel'; + } + } + // Source-specific required fields if (data.source === 'tile') { if (!data.dashboardId) { diff --git a/packages/api/src/models/alert.ts b/packages/api/src/models/alert.ts index 62c52eb3df..335ffc57cc 100644 --- a/packages/api/src/models/alert.ts +++ b/packages/api/src/models/alert.ts @@ -47,6 +47,12 @@ export type AlertChannel = type: 'webhook'; webhookId: string; } + | { + // A firing alert starts an investigation session on the referenced + // managed agent (see models/managedAgent.ts) instead of POSTing a payload. + type: 'agent'; + agentId: string; + } | { type: null; }; diff --git a/packages/api/src/routers/api/__tests__/agentChannelFlagOff.int.test.ts b/packages/api/src/routers/api/__tests__/agentChannelFlagOff.int.test.ts new file mode 100644 index 0000000000..620a7d818d --- /dev/null +++ b/packages/api/src/routers/api/__tests__/agentChannelFlagOff.int.test.ts @@ -0,0 +1,81 @@ +import mongoose from 'mongoose'; + +import { + getLoggedInAgent, + getServer, + makeAlertInput, + makeTile, +} from '@/fixtures'; +import ManagedAgent from '@/models/managedAgent'; + +// The rest of the agent-channel suites run with the flag on (.env.test); this +// file mocks it off to pin the two flag-off behaviours: alert saves carrying an +// agent channel are rejected, and the managed-agents surface 404s. +jest.mock('@/config', () => ({ + ...jest.requireActual('@/config'), + IS_MANAGED_AGENTS_ENABLED: false, +})); + +const MOCK_TILES = [makeTile()]; +const MOCK_DASHBOARD = { + id: new mongoose.Types.ObjectId().toString(), + name: 'Test Dashboard', + tiles: MOCK_TILES, + tags: ['test'], +}; + +describe('managed agents disabled', () => { + const server = getServer(); + let agent: Awaited>['agent']; + let team: Awaited>['team']; + + beforeAll(async () => { + await server.start(); + }); + + beforeEach(async () => { + const result = await getLoggedInAgent(server); + agent = result.agent; + team = result.team; + }); + + afterEach(async () => { + await server.clearDBs(); + }); + + afterAll(async () => { + await server.stop(); + }); + + it('rejects saving an alert with an agent channel', async () => { + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + const managedAgent = await ManagedAgent.create({ + team: team._id, + name: 'SRE Responder', + model: 'claude-opus-4-8', + anthropicAgentId: 'agent_1', + vaultId: 'vlt_1', + environmentId: 'env_1', + mcpServerUrl: 'https://mcp.example.test/api/mcp', + }); + + const resp = await agent + .post('/alerts') + .send({ + ...makeAlertInput({ + dashboardId: dashboard.body.id, + tileId: dashboard.body.tiles[0].id, + }), + channel: { type: 'agent', agentId: managedAgent._id.toString() }, + }) + .expect(400); + expect(resp.text).toContain('not enabled'); + }); + + it('404s the managed-agents surface', async () => { + await agent.get('/managed-agents').expect(404); + }); +}); diff --git a/packages/api/src/routers/api/__tests__/alerts.int.test.ts b/packages/api/src/routers/api/__tests__/alerts.int.test.ts index a8da5fa1de..aff853a2d9 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -23,6 +23,7 @@ import { import Alert, { AlertSource, AlertState } from '@/models/alert'; import AlertHistory from '@/models/alertHistory'; import Connection from '@/models/connection'; +import ManagedAgent from '@/models/managedAgent'; import { SavedSearch } from '@/models/savedSearch'; import { Source } from '@/models/source'; import Webhook, { WebhookDocument, WebhookService } from '@/models/webhook'; @@ -2190,4 +2191,92 @@ describe('alerts router', () => { }); }); }); + + describe('agent notification channels', () => { + const seedManagedAgent = (teamId: mongoose.Types.ObjectId) => + ManagedAgent.create({ + team: teamId, + name: 'SRE Responder', + model: 'claude-opus-4-8', + anthropicAgentId: 'agent_1', + vaultId: 'vlt_1', + environmentId: 'env_1', + mcpServerUrl: 'https://mcp.example.test/api/mcp', + }); + + const agentAlertInput = ( + dashboardId: string, + tileId: string, + agentId: string, + ) => ({ + ...makeAlertInput({ dashboardId, tileId }), + channel: { type: 'agent' as const, agentId }, + }); + + it('accepts an agent channel referencing a team agent and round-trips agentId', async () => { + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + const managedAgent = await seedManagedAgent(team._id); + + await agent + .post('/alerts') + .send( + agentAlertInput( + dashboard.body.id, + dashboard.body.tiles[0].id, + managedAgent._id.toString(), + ), + ) + .expect(200); + + const alerts = await agent.get('/alerts').expect(200); + expect(alerts.body.data[0].channel).toEqual({ + type: 'agent', + agentId: managedAgent._id.toString(), + }); + }); + + it("rejects an agent channel referencing another team's agent", async () => { + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + const foreignAgent = await seedManagedAgent( + new mongoose.Types.ObjectId(), + ); + + const resp = await agent + .post('/alerts') + .send( + agentAlertInput( + dashboard.body.id, + dashboard.body.tiles[0].id, + foreignAgent._id.toString(), + ), + ) + .expect(400); + expect(resp.text).toContain('Agent not found'); + }); + + it('rejects a malformed agent id', async () => { + const dashboard = await agent + .post('/dashboards') + .send(MOCK_DASHBOARD) + .expect(200); + + const resp = await agent + .post('/alerts') + .send( + agentAlertInput( + dashboard.body.id, + dashboard.body.tiles[0].id, + 'not-an-object-id', + ), + ) + .expect(400); + expect(resp.text).toContain('Invalid agent ID'); + }); + }); }); diff --git a/packages/api/src/routers/api/alerts.ts b/packages/api/src/routers/api/alerts.ts index 4beb502fe1..cf01be68f7 100644 --- a/packages/api/src/routers/api/alerts.ts +++ b/packages/api/src/routers/api/alerts.ts @@ -60,11 +60,13 @@ const formatAlertResponse = ( createdBy: alert.createdBy ? pick(alert.createdBy, ['email', 'name']) : undefined, - // webhookId is included so edit surfaces (e.g. the alert detail page) can - // prefill the notification channel; webhook ids are already visible to - // team members via GET /webhooks. - channel: pick(alert.channel, ['type', 'webhookId']), - channels: getAlertChannels(alert).map(c => pick(c, ['type', 'webhookId'])), + // webhookId/agentId are included so edit surfaces (e.g. the alert detail + // page) can prefill the notification channel; both id kinds are already + // visible to team members via GET /webhooks and GET /managed-agents. + channel: pick(alert.channel, ['type', 'webhookId', 'agentId']), + channels: getAlertChannels(alert).map(c => + pick(c, ['type', 'webhookId', 'agentId']), + ), ...(alert.dashboard && { dashboardId: alert.dashboard._id, dashboard: { diff --git a/packages/api/src/routers/external-api/v2/alerts.ts b/packages/api/src/routers/external-api/v2/alerts.ts index aed17a11d2..1e8d4798bd 100644 --- a/packages/api/src/routers/external-api/v2/alerts.ts +++ b/packages/api/src/routers/external-api/v2/alerts.ts @@ -132,10 +132,6 @@ function toInternalAlertInput(body: ExternalAlertInput): InternalAlertInput { * type: string * enum: [ALERT, OK, INSUFFICIENT_DATA, DISABLED, PENDING] * description: Current alert state. - * AlertChannelType: - * type: string - * enum: [webhook] - * description: Channel type. * AlertErrorType: * type: string * enum: [QUERY_ERROR, QUERY_TIMEOUT, WEBHOOK_ERROR, INVALID_ALERT, UNKNOWN] @@ -187,15 +183,37 @@ function toInternalAlertInput(body: ExternalAlertInput): InternalAlertInput { * - webhookId * properties: * type: - * $ref: '#/components/schemas/AlertChannelType' + * type: string + * enum: [webhook] * description: Channel type. Must be "webhook" for webhook alerts. + * example: "webhook" * webhookId: * type: string * description: Webhook destination ID. * example: "65f5e4a3b9e77c001a789012" + * AlertChannelAgent: + * type: object + * description: >- + * AI agent notification target. A firing alert starts an + * investigation session on the referenced managed agent. Only + * available on deployments with managed agents enabled. + * required: + * - type + * - agentId + * properties: + * type: + * type: string + * enum: [agent] + * description: Channel type. Must be "agent" for agent alerts. + * example: "agent" + * agentId: + * type: string + * description: Managed agent ID. + * example: "65f5e4a3b9e77c001a789014" * AlertChannel: * oneOf: * - $ref: '#/components/schemas/AlertChannelWebhook' + * - $ref: '#/components/schemas/AlertChannelAgent' * discriminator: * propertyName: type * AlertChannels: diff --git a/packages/app/src/utils/alerts.ts b/packages/app/src/utils/alerts.ts index c56075387e..00e88ed68d 100644 --- a/packages/app/src/utils/alerts.ts +++ b/packages/app/src/utils/alerts.ts @@ -133,6 +133,7 @@ export const TILE_ALERT_INTERVAL_OPTIONS = _.pick(ALERT_INTERVAL_OPTIONS, [ export const ALERT_CHANNEL_OPTIONS: Record = { webhook: 'Webhook', + agent: 'AI agent', }; const EMPTY_ALERT_CHANNEL = { type: 'webhook', webhookId: '' } as const; diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 443852d6c7..3810d046d8 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -747,15 +747,27 @@ export const ALERT_INTERVAL_TO_MINUTES: Record = { '1d': 1440, }; -export const zAlertChannelType = z.literal('webhook'); +export const zAlertChannelType = z.enum(['webhook', 'agent']); export type AlertChannelType = z.infer; -export const zAlertChannel = z.object({ - type: zAlertChannelType, +export const zWebhookAlertChannel = z.object({ + type: z.literal('webhook'), webhookId: z.string().nonempty("Webhook ID can't be empty"), }); +// An AI agent notification target: a firing alert starts an investigation +// session on the referenced managed agent instead of POSTing a payload. +export const zAgentAlertChannel = z.object({ + type: z.literal('agent'), + agentId: z.string().nonempty("Agent ID can't be empty"), +}); + +export const zAlertChannel = z.discriminatedUnion('type', [ + zWebhookAlertChannel, + zAgentAlertChannel, +]); + export const MAX_ALERT_CHANNELS = 10; export const zAlertChannels = z @@ -2631,6 +2643,7 @@ export type AssistantResponseConfigSchema = z.infer< const alertsPageItemChannelSchema = z.object({ type: z.string().optional().nullable(), webhookId: z.string().optional(), + agentId: z.string().optional(), }); export const AlertsPageItemSchema = z.object({ From b5042e40a709f45e75c022c0cde6103569096669 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Mon, 14 Sep 2026 15:57:14 +1000 Subject: [PATCH 2/3] fix(api): surface an undispatchable channel instead of dropping it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on the agent channel schema. A channel this build cannot dispatch resolved to zero notification jobs and recorded nothing, so an alert whose only target was one would fire, notify nobody, and leave no trace. It is now reported as a failed target, which also covers the downstream channel kinds the surrounding comment already anticipates. Widening the channel-type union added an "AI agent" entry to the type picker in three alert editors, none of which can collect an agentId — so the option was selectable and unsaveable. That map now lists only the types the picker can actually produce, and no longer tracks the union. Also gives the AlertChannel discriminator an explicit mapping, so a generated client resolves `type: "webhook"` to a schema rather than guessing at a name that does not exist, and folds the webhook and agent reference checks into one pass — they were the same five steps with the model and the noun swapped. --- packages/api/openapi.json | 6 +- packages/api/src/controllers/alerts.ts | 55 ++++++++++--------- .../api/src/routers/external-api/v2/alerts.ts | 3 + .../__tests__/renderAlertTemplate.int.test.ts | 38 ++++++++++++- .../api/src/tasks/checkAlerts/template.ts | 13 +++++ packages/app/src/utils/alerts.ts | 9 ++- 6 files changed, 94 insertions(+), 30 deletions(-) diff --git a/packages/api/openapi.json b/packages/api/openapi.json index 0c065bc6be..af33c4f713 100644 --- a/packages/api/openapi.json +++ b/packages/api/openapi.json @@ -290,7 +290,11 @@ } ], "discriminator": { - "propertyName": "type" + "propertyName": "type", + "mapping": { + "webhook": "#/components/schemas/AlertChannelWebhook", + "agent": "#/components/schemas/AlertChannelAgent" + } } }, "AlertChannels": { diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 1cc920c791..13b1524dd6 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -219,20 +219,35 @@ export const validateAlertInput = async ( throw new Api400Error('At least one notification channel is required'); } - const webhookIds = channels - .filter(c => c.type === 'webhook') - .map(c => c.webhookId); - for (const webhookId of webhookIds) { - validateObjectId(webhookId, 'Invalid webhook ID'); - } - const uniqueIds = [...new Set(webhookIds)]; - const found = await Webhook.countDocuments({ - _id: { $in: uniqueIds }, - team: teamId, - }); - if (found !== uniqueIds.length) { - throw new Api400Error('Webhook not found'); - } + // Each channel kind names a row in a different collection; the check is the + // same either way — the ids parse, and every distinct one belongs to this + // team. Kept as one pass so a third channel kind cannot arrive with a + // subtly different rule. + const assertChannelRefsExist = async ( + ids: string[], + model: { countDocuments: (f: object) => PromiseLike }, + label: string, + ) => { + for (const id of ids) { + validateObjectId(id, `Invalid ${label} ID`); + } + const unique = [...new Set(ids)]; + const found = await model.countDocuments({ + _id: { $in: unique }, + team: teamId, + }); + if (found !== unique.length) { + throw new Api400Error( + `${label[0].toUpperCase()}${label.slice(1)} not found`, + ); + } + }; + + await assertChannelRefsExist( + channels.filter(c => c.type === 'webhook').map(c => c.webhookId), + Webhook, + 'webhook', + ); const agentIds = channels.filter(c => c.type === 'agent').map(c => c.agentId); if (agentIds.length > 0) { @@ -243,17 +258,7 @@ export const validateAlertInput = async ( 'AI agent notification channels are not enabled on this deployment (set HDX_MANAGED_AGENTS_ENABLED)', ); } - for (const agentId of agentIds) { - validateObjectId(agentId, 'Invalid agent ID'); - } - const uniqueAgentIds = [...new Set(agentIds)]; - const foundAgents = await ManagedAgent.countDocuments({ - _id: { $in: uniqueAgentIds }, - team: teamId, - }); - if (foundAgents !== uniqueAgentIds.length) { - throw new Api400Error('Agent not found'); - } + await assertChannelRefsExist(agentIds, ManagedAgent, 'agent'); } return refs; diff --git a/packages/api/src/routers/external-api/v2/alerts.ts b/packages/api/src/routers/external-api/v2/alerts.ts index 1e8d4798bd..fd8052ef25 100644 --- a/packages/api/src/routers/external-api/v2/alerts.ts +++ b/packages/api/src/routers/external-api/v2/alerts.ts @@ -216,6 +216,9 @@ function toInternalAlertInput(body: ExternalAlertInput): InternalAlertInput { * - $ref: '#/components/schemas/AlertChannelAgent' * discriminator: * propertyName: type + * mapping: + * webhook: '#/components/schemas/AlertChannelWebhook' + * agent: '#/components/schemas/AlertChannelAgent' * AlertChannels: * type: array * description: > diff --git a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts index d7ffdcdf10..3dcd6e2129 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts @@ -14,7 +14,7 @@ import { makeTile, RAW_SQL_ALERT_TEMPLATE, } from '@/fixtures'; -import { AlertSource } from '@/models/alert'; +import { AlertChannel, AlertSource } from '@/models/alert'; import type { IWebhook } from '@/models/webhook'; import { NotificationDispatcher, @@ -1197,6 +1197,42 @@ describe('notification targets are resolved once per webhook', () => { expect(failures).toHaveLength(0); }); + // A channel kind this build has no transport for — the downstream 'email' + // case the transport table anticipates. Typed loosely on purpose: the point + // is a value the union does not describe. + const foreignChannel = (value: any): AlertChannel => value; + + // An unsupported channel type must cost the alert a visible failure, not + // vanish. An alert whose only target is one would otherwise fire and notify + // nobody, with nothing recorded to notice. + it('records a channel this build cannot dispatch as a failed target', async () => { + const { dispatcher, dispatched } = makeRecordingDispatcher(); + + const { failures } = await renderAlertTemplate({ + alertProvider, + clickhouseClient: mockClickhouseClient, + metadata: mockMetadata, + state: AlertState.ALERT, + template: null, + title: 'Test Alert Title', + view: { + ...makeSearchView(), + alert: { + ...makeSearchView().alert, + channels: [foreignChannel({ type: 'email', address: 'a@b.c' })], + }, + }, + teamId: TEST_TEAM_ID, + teamWebhooksById: new Map(), + dispatcher, + }); + + expect(dispatched).toHaveLength(0); + expect(failures).toHaveLength(1); + expect(failures[0].type).toBe('email'); + expect(String(failures[0].error)).toContain('cannot notify'); + }); + it('keeps firing configured channels when the message has a plain @mention', async () => { const webhook = makeWebhook('ok-hook'); const id = webhook._id.toString(); diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index a2850c6207..405b643b10 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -761,7 +761,20 @@ export const renderAlertTemplate = async ({ const resolveConfiguredChannel = ( channel: AlertChannel, ): PopulatedAlertChannel[] => { + // Anything this build cannot dispatch is reported as a failed target + // rather than dropped: an alert whose only channel is unsupported would + // otherwise fire, notify nobody, and record nothing. if (channel.type !== 'webhook') { + // A pre-multi-channel row can carry a null type; label it rather than + // rendering "null" at the user. + const kind = channel.type ?? 'unknown'; + recordPreFailure( + kind, + kind, + new Error( + `This deployment cannot notify a "${kind}" channel. Update the alert's notification channel.`, + ), + ); return []; } const webhook = teamWebhooksById.get(channel.webhookId); diff --git a/packages/app/src/utils/alerts.ts b/packages/app/src/utils/alerts.ts index 00e88ed68d..75268eb40f 100644 --- a/packages/app/src/utils/alerts.ts +++ b/packages/app/src/utils/alerts.ts @@ -12,7 +12,6 @@ import { isPromqlSavedChartConfig } from '@hyperdx/common-utils/dist/guards'; import { type Alert, ALERT_INTERVAL_TO_MINUTES, - AlertChannelType, AlertChartConfig, AlertInterval, AlertSource, @@ -131,9 +130,13 @@ export const TILE_ALERT_INTERVAL_OPTIONS = _.pick(ALERT_INTERVAL_OPTIONS, [ '1d', ]); -export const ALERT_CHANNEL_OPTIONS: Record = { +// Only the channel types this control can actually produce. An agent channel +// needs an agentId, which a bare type picker cannot collect, so agents are +// offered by the notification target picker instead — not here. Deliberately +// not keyed on AlertChannelType: widening that union must not silently add an +// unfillable option to the editors that read this. +export const ALERT_CHANNEL_OPTIONS: Record<'webhook', string> = { webhook: 'Webhook', - agent: 'AI agent', }; const EMPTY_ALERT_CHANNEL = { type: 'webhook', webhookId: '' } as const; From 609b29ad4a5a0c70679f4e22451b8747492f1b94 Mon Sep 17 00:00:00 2001 From: Jordan Simonovski Date: Mon, 14 Sep 2026 23:24:23 +1000 Subject: [PATCH 3/3] fix(alerts): let the undispatchable-channel message reach the operator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recording the failure wasn't enough: makeNotificationAlertError only keeps the message for a handful of known error classes, so a bare Error fell through to the generic branch and the authored "this deployment cannot notify a … channel" never surfaced. It gets its own class and branch, like WebhookNotFoundError beside it, and the target is the channel's own id so two unsupported channels are told apart rather than reported identically. --- .../__tests__/renderAlertTemplate.int.test.ts | 4 ++++ packages/api/src/tasks/checkAlerts/errors.ts | 13 +++++++++++++ packages/api/src/tasks/checkAlerts/index.ts | 8 ++++++++ packages/api/src/tasks/checkAlerts/template.ts | 13 +++++-------- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts index 3dcd6e2129..cfea4f2ecc 100644 --- a/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts +++ b/packages/api/src/tasks/checkAlerts/__tests__/renderAlertTemplate.int.test.ts @@ -16,6 +16,7 @@ import { } from '@/fixtures'; import { AlertChannel, AlertSource } from '@/models/alert'; import type { IWebhook } from '@/models/webhook'; +import { UnsupportedChannelError } from '@/tasks/checkAlerts/errors'; import { NotificationDispatcher, NotificationJob, @@ -1230,6 +1231,9 @@ describe('notification targets are resolved once per webhook', () => { expect(dispatched).toHaveLength(0); expect(failures).toHaveLength(1); expect(failures[0].type).toBe('email'); + // Its own class, so makeNotificationAlertError keeps the message instead + // of rewriting it as a generic webhook failure the operator can't act on. + expect(failures[0].error).toBeInstanceOf(UnsupportedChannelError); expect(String(failures[0].error)).toContain('cannot notify'); }); diff --git a/packages/api/src/tasks/checkAlerts/errors.ts b/packages/api/src/tasks/checkAlerts/errors.ts index 1774851099..eb8260d5f6 100644 --- a/packages/api/src/tasks/checkAlerts/errors.ts +++ b/packages/api/src/tasks/checkAlerts/errors.ts @@ -19,6 +19,19 @@ export class UnsupportedMentionError extends Error { } } +// A channel kind this build has no transport for — a downstream channel type, +// or one whose dispatch lands in a later change. Its own class so the message +// survives makeNotificationAlertError instead of being rewritten as a generic +// webhook failure. +export class UnsupportedChannelError extends Error { + constructor(kind: string) { + super( + `This deployment cannot notify a "${kind}" channel. Update the alert's notification channel.`, + ); + this.name = 'UnsupportedChannelError'; + } +} + // MAX_NOTIFICATIONS_PER_EVENT was reached; this target was dropped before // dispatch rather than silently skipped, so it's still visible as an error. export class NotificationCapExceededError extends Error { diff --git a/packages/api/src/tasks/checkAlerts/index.ts b/packages/api/src/tasks/checkAlerts/index.ts index 6ad0d89489..060337c217 100644 --- a/packages/api/src/tasks/checkAlerts/index.ts +++ b/packages/api/src/tasks/checkAlerts/index.ts @@ -67,6 +67,7 @@ import { isClientTimeoutOrAbortError, isQueryTimeoutError, NotificationCapExceededError, + UnsupportedChannelError, UnsupportedMentionError, WEBHOOK_REDIRECT_ERROR_MESSAGE, WebhookNotFoundError, @@ -306,6 +307,13 @@ const makeNotificationAlertError = ( message: `${failure.error.message} (${target})`.slice(0, 10000), }; } + if (failure.error instanceof UnsupportedChannelError) { + return { + timestamp, + type: AlertErrorType.WEBHOOK_ERROR, + message: `${failure.error.message} (${target})`.slice(0, 10000), + }; + } if (failure.error instanceof WebhookNotFoundError) { // Name the target like every other branch — with several channels, "a // webhook was deleted" is useless unless it says which one. diff --git a/packages/api/src/tasks/checkAlerts/template.ts b/packages/api/src/tasks/checkAlerts/template.ts index 405b643b10..bf7491e5da 100644 --- a/packages/api/src/tasks/checkAlerts/template.ts +++ b/packages/api/src/tasks/checkAlerts/template.ts @@ -41,6 +41,7 @@ import { } from '@/tasks/checkAlerts'; import { NotificationCapExceededError, + UnsupportedChannelError, UnsupportedMentionError, WebhookNotFoundError, } from '@/tasks/checkAlerts/errors'; @@ -766,15 +767,11 @@ export const renderAlertTemplate = async ({ // otherwise fire, notify nobody, and record nothing. if (channel.type !== 'webhook') { // A pre-multi-channel row can carry a null type; label it rather than - // rendering "null" at the user. + // rendering "null" at the user. The kind is the whole target here — a + // channel this build has no transport for has no id shape we can read + // without inventing one. const kind = channel.type ?? 'unknown'; - recordPreFailure( - kind, - kind, - new Error( - `This deployment cannot notify a "${kind}" channel. Update the alert's notification channel.`, - ), - ); + recordPreFailure(kind, kind, new UnsupportedChannelError(kind)); return []; } const webhook = teamWebhooksById.get(channel.webhookId);