Repository navigation
feat(api): accept an AI agent as an alert notification channel #3121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a9053d3
b5042e4
609b29a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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'; | ||
|
|
@@ -217,19 +219,46 @@ 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<number> }, | ||
| 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); | ||
|
jordan-simonovski marked this conversation as resolved.
Resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟠 major — Dashboard tile alerts never reach
|
||
| 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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — Disabling the flag makes an existing agent-channel alert impossible to edit or repair
|
||
| throw new Api400Error( | ||
| 'AI agent notification channels are not enabled on this deployment (set HDX_MANAGED_AGENTS_ENABLED)', | ||
| ); | ||
| } | ||
| await assertChannelRefsExist(agentIds, ManagedAgent, 'agent'); | ||
| } | ||
|
|
||
| return refs; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 ' + | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — MCP tool now asks for an The |
||
| 'enabled, an AI agent investigation ({type:"agent", agentId}). Pass ' + | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The MCP tool and the v2 OpenAPI now ask for an MCP exposes |
||
| '"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: { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The MCP narrowing that decides what gets persisted has no test
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — No test covers an agent channel through the MCP tool, including the new narrowing
|
||
| 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -106,11 +106,18 @@ const mcpAlertChartConfigSchema = z | |
| const mcpAlertChannelSchema = z | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — The MCP channel schema accepts webhookId and agentId together and the off-variant id is silently dropped
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — MCP's flat channel shape is hashed before narrowing, so the duplicate/mismatch check disagrees with what gets persisted
|
||
| .object({ | ||
| type: z | ||
| .literal('webhook') | ||
| .enum(['webhook', 'agent']) | ||
| .describe('Channel type for alert notifications.'), | ||
| webhookId: z | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — MCP channel schema admits both ids at once; the extra one is silently dropped and defeats duplicate detection With |
||
| .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 ?? [])]) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — MCP duplicate/mismatch channel rules run before per-type narrowing, so a stray foreign id defeats them
|
||
| if (!channel) continue; | ||
| if (channel.type === 'webhook' && !channel.webhookId) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔵 minor — MCP channel shape accepts an agentId on a webhook channel, which defeats duplicate detection
|
||
| 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) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<ReturnType<typeof getLoggedInAgent>>['agent']; | ||
| let team: Awaited<ReturnType<typeof getLoggedInAgent>>['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); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.