Repository navigation
fix(alerts): migrate metric alert mutations to detectors API - #1338
Conversation
Repoint create/edit/delete of metric alert rules off the legacy org-scoped /alert-rules/ endpoint (retiring 2026-08-17) onto the detectors write API, following the read-path migration in #1275. The CLI commands still build the flat legacy body; the new buildDetectorBody translator in alerts.ts converts it into the nested detector document (dataSources / conditionGroup / config) so the command layer is unchanged: - createMetricAlertRule -> createOrganizationProjectDetector (project-scoped POST; project taken from the flat body's projects, now required for create) - putMetricAlertRule -> updateOrganizationDetector (PUT /detectors/{id}/) - getMetricAlertRuleDocument edit baseline -> detector GET, returned in the flat MetricAlertRule shape so the command-side merge stays coherent - deleteMetricAlertRule -> deleteOrganizationDetector Fixes #1277
|
|
fix-ci: attempt 1 — the E2E failure is |
|
CI is green after the re-run — the bundle build succeeded this time, confirming the earlier E2E failure was a transient bundle-build issue and not related to the diff. |
|
Marked ready for review. Full CI is green (Lint & Typecheck, Unit Tests, E2E, Warden, Secret Scan, CodeQL, semgrep, dependency-review) and self-review turned up nothing to fix. The metric-alert create/edit/delete now go through the detectors write API; see the description for the two known limitations (edit |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| const triggers = Array.isArray(body.triggers) ? body.triggers : undefined; | ||
| if (triggers) { | ||
| detectorBody.conditionGroup = { | ||
| logicType: "any", | ||
| conditions: triggers, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Bug: Editing a metric alert without the --trigger flag deletes all existing triggers because the CLI fails to fetch and preserve them during the update.
Severity: HIGH
Suggested Fix
Update the MetricAlertRule type and the mapDetectorToMetricAlertRule function to include the triggers field. This will ensure that when an alert is fetched for an edit, its existing triggers are included in the baseline data and preserved if not explicitly overridden.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/cli/src/lib/api/alerts.ts#L619-L625
Potential issue: When editing a metric alert via the CLI without explicitly
re-specifying the `--trigger` flag, all existing trigger conditions are deleted. This
occurs because the function `getMetricAlertRuleDocument`, which fetches the alert's
current state, fails to include the `triggers` field in the object it returns. The
`buildDetectorBody` function then constructs an update payload without a
`conditionGroup`, as `body.triggers` is `undefined`. The backend API interprets this as
a full replacement and removes the alert's triggers.
Did we get this right? 👍 / 👎 to inform future reviews.
| } | ||
|
|
||
| const orgSlug = await resolveMetricCreateOrg(arg, cwd); | ||
|
|
There was a problem hiding this comment.
Bug: The create and edit metric alert commands always show the status as "active" in their output, regardless of the rule's actual state.
Severity: LOW
Suggested Fix
Modify the create and edit commands to correctly handle the API response. Instead of directly using the raw response, map it to a MetricAlertRule object (which has the status field) before passing it to the output rendering logic, or adjust the rendering logic to read the enabled field.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/cli/src/commands/alert/metrics/create.ts#L195
Potential issue: The `create` and `edit` commands for metric alerts display an incorrect
status in their output. These commands receive a raw API response containing an
`enabled: boolean` field. However, the output rendering logic attempts to read a
`status` property from this response object. Because `created.status` (or
`updated.status`) is `undefined`, the `metricAlertStatusLabel` function incorrectly
defaults to returning "active", even if the rule was created or updated as disabled.
Also affects:
packages/cli/src/commands/alert/metrics/edit.ts:281
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b244375. Configure here.
| }); | ||
| return unwrapResult<Record<string, unknown>>( | ||
| result, | ||
| `Failed to update metric alert rule '${ruleId}'` |
There was a problem hiding this comment.
Mutation response status mishandled
Medium Severity
putMetricAlertRule and createMetricAlertRule return the raw detector payload (enabled) instead of the flat rule shape with status. Edit then runs metricAlertStatusLabel on updated.status, so a successful disable still reports active and JSON output exposes the nested detector document.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit b244375. Configure here.
There was a problem hiding this comment.
Confirmed — this is a real bug: createMetricAlertRule/putMetricAlertRule return the raw detector payload (enabled), but the create/edit commands render metricAlertStatusLabel(created.status), so a disabled rule prints as active and JSON exposes the nested detector doc. The PR merged before this could be addressed on-branch, so it needs a follow-up on main (map the mutation response through mapDetectorToMetricAlertRule before rendering). The paired sentry[bot] review also flagged the related edit-without---trigger trigger-loss issue — both should go in the same follow-up.
1 participant
Migrates metric-alert create/edit/delete off the legacy org-scoped
/organizations/{org}/alert-rules/endpoint (retiring 2026-08-17) onto the detectors write API, completing the read-path migration from #1275. The CLI commands keep building the flat legacy body; a newbuildDetectorBodytranslator inalerts.tsconverts it into the nested detector document (dataSources/conditionGroup/config), so the command layer is largely unchanged.createMetricAlertRule→createOrganizationProjectDetector(project-scopedPOST /projects/{project}/detectors/). Detector create is project-scoped, so--projectis now required forcreate; the target is taken from the flat body'sprojects.putMetricAlertRule→updateOrganizationDetector(PUT /detectors/{id}/).getMetricAlertRuleDocument(edit baseline) → detectorGET, returned in the flatMetricAlertRuleshape so the command-side merge stays coherent.deleteMetricAlertRule→deleteOrganizationDetector(DELETE /detectors/{id}/).Dataset →
queryType/eventTypesmapping: error-like datasets (errors/events) →queryType: 0+["error","default"]; everything else →queryType: 1+["trace_item_span"].timeWindowis converted minutes→seconds (inverse of the read mapping), andstatus(0/1) maps to the detectorenabledflag.Known limitations (best-effort draft)
--projecton edit is now a no-op: the detector update endpoint is not project-scoped and has noprojectsfield.config.detectionType: "static"); change/dynamic detection types aren't surfaced by the flat CLI input. Per Migrate alert commands off legacy APIs by Aug 17 #1274/alert metrics CLI doesn't support detector/workflow log alerts and list/view are inconsistent #1142 full write parity is still gated on backend feature gaps.Testing
pnpm exec vitest run test/lib/api/alerts.test.ts test/commands/alert— 170 passed. Added detector-endpoint assertions for create (nested body, project-scoped path, error-like dataset mapping, missing-project validation), put, delete, and the flat edit baseline.pnpm exec tsc --noEmit— clean.biome check— clean.Closes #1277