Repository navigation
feat(alerts): acknowledge alert incidents (core, API, MCP) - #2287
Confidence 10/10 · No issues found
🟢 Confidence 10/10 · safe to merge. The only diff since the previous review is an equivalent Arr.findFirst/Option lookup in logRow, with no behavior change.
no findings · tests not needed · risk low
Alert incidents can now be acknowledged and unacknowledged across core, the v2 API and MCP, with an append-only event history. The only change since the last review makes the delivery log's connector lookup use Arr.findFirst instead of an equivalent ?. chain; it is safe to merge. It does not address the open McpQueryError issues in apps/ai/src/mcp/tools/registry.ts: that diff only registers the new tool.
Before merge
- Migration · Check it is safe on production data and with the code still deployed ·
packages/db/drizzle/20261105234724_alert_incident_ack/migration.sql - Migration · Check it is safe on production data and with the code still deployed ·
packages/db/drizzle/20261106113821_merge_alert_ack_slos/migration.sql
Production impact
Open errors in the changed files (2)
| Issue | Service | Occurrences | File |
|---|---|---|---|
| @maple/mcp/errors/McpQueryError (4 issues) | maple-chat | 30 | apps/ai/src/mcp/tools/registry.ts |
| @maple/mcp/errors/McpQueryError (6 issues) | maple-ai | 18 | apps/ai/src/mcp/tools/registry.ts |
After this merges, Maple checks whether they stop.
Production traffic of the changed files (last 7 days)
| File | Calls/day | Busiest operations |
|---|---|---|
packages/domain/src/http/v2/alert-incidents.ts |
119 | GET /v2/alerts/incidents 119/day, 0.0% err |
packages/domain/src/http/alerts.ts |
26 | test 26/day, 18.9% err |
Telemetry this change adds and removes (4)
- ➕ span name
McpTool.acknowledgeAlertIncident·apps/ai/src/mcp/tools/acknowledge-alert-incident.ts:27 - ➕ span name
HttpV2AlertIncidents.acknowledge·apps/api/src/routes/v2/alert-incidents.http.ts:124 - ➕ span name
HttpV2AlertIncidents.unacknowledge·apps/api/src/routes/v2/alert-incidents.http.ts:131 - ➕ span name
AlertIncidentAckService.listEvents·packages/backend/src/services/alerts/AlertIncidentAckService.ts:183
What was checked
chatConnectorisundefined-typed (optionalKey,packages/domain/src/http/alerts.ts:675), soOption.flatMapNullishOrdrops a missing destination and an absent connector alike, as the old?.…ArrandOptionare already imported atapps/web/src/page/alerts/hub/view-routing.ts:41, and data-firstArr.findFirsttakes aReadonlyArray
3924399 · Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.