From 3170654f755b5a61ac7267d7b3bbb4b8fca68ee1 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 10:50:30 +0000 Subject: [PATCH 1/6] fix(alerts): read metric alert rules from org-scoped /detectors/ Migrate the metric-alert read path (list/view/get) off the legacy /organizations/{org}/alert-rules/ endpoint, which is being retired on 2026-08-17, to the new org-scoped /organizations/{org}/detectors/ endpoint filtered to type:metric_issue. Detectors return a nested shape, so mapDetectorToMetricAlertRule flattens each detector into the existing MetricAlertRule shape the list/view/resolve commands already consume (threshold fields live in dataSources[0], active/disabled state is the top-level enabled flag). Mirrors the proven issue-alert read migration (#1215/#1216). Metric alert mutations (create/edit/delete) still use the legacy endpoint pending the follow-up mutation migration. Refs #1274, #1182 --- src/lib/api/alerts.ts | 126 +++++++++++++++++-- test/commands/alert/metrics/list.test.ts | 152 ++++++++++------------- test/lib/api/alerts.test.ts | 94 ++++++++++++++ 3 files changed, 279 insertions(+), 93 deletions(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index f10ac199f2..be59c38ed0 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -55,6 +55,88 @@ export type MetricAlertRule = { dateCreated: string; }; +/** + * A metric-issue detector as returned by the org-scoped `/detectors/` endpoint. + * + * The new detectors API replaces the legacy `/alert-rules/` endpoint. The + * threshold query fields (aggregate, dataset, query, timeWindow) live inside + * the first entry of `dataSources`, and the active/disabled state is the + * top-level `enabled` boolean. Only the fields the CLI reads are typed; the + * nested `dataSources`/`config` objects are otherwise opaque. + */ +type MetricDetector = { + id: string | number; + name: string; + enabled?: boolean; + environment?: string | null; + projectSlug?: string | null; + projects?: string[] | null; + owner?: { type: string; name?: string; id?: string } | string | null; + dateCreated?: string; + dataSources?: Record[] | null; +}; + +/** + * Map a metric-issue detector onto the flat `MetricAlertRule` shape the CLI + * commands (list/view/resolve) already consume. + * + * The threshold query fields live in the first data source. Some deployments + * nest them under a `queryObj`/`snubaQuery` sub-object, so we look there as a + * fallback. `enabled === false` maps to the legacy disabled status (1); an + * enabled or absent flag maps to active (0), matching `metricAlertStatusLabel`. + */ +function pickDetectorString(value: unknown): string { + return typeof value === "string" ? value : ""; +} + +function pickDetectorNumber(value: unknown): number { + if (typeof value === "number") { + return value; + } + if (typeof value === "string" && value.trim() !== "") { + return Number(value); + } + return 0; +} + +/** Reduce a detector owner (object or bare string) to the flat owner value. */ +function pickDetectorOwner(owner: MetricDetector["owner"]): string | null { + if (typeof owner === "string") { + return owner; + } + if (owner && typeof owner === "object") { + return owner.name ?? owner.id ?? null; + } + return null; +} + +function mapDetectorToMetricAlertRule( + detector: MetricDetector +): MetricAlertRule { + const source = detector.dataSources?.[0] ?? {}; + const nested = + (source.queryObj as Record | undefined) ?? + (source.snubaQuery as Record | undefined) ?? + source; + + const environment = + pickDetectorString(nested.environment) || detector.environment || null; + + return { + id: String(detector.id), + name: detector.name, + status: detector.enabled === false ? 1 : 0, + query: pickDetectorString(nested.query), + aggregate: pickDetectorString(nested.aggregate), + dataset: pickDetectorString(nested.dataset), + timeWindow: pickDetectorNumber(nested.timeWindow), + environment, + owner: pickDetectorOwner(detector.owner), + projects: detector.projects ?? [], + dateCreated: detector.dateCreated ?? "", + }; +} + // Issue alerts // // Issue alert rules are read from the org-scoped `/organizations/{org}/workflows/` @@ -164,10 +246,26 @@ export async function getIssueAlertRule( } // Metric alerts +// +// Metric alert rules are read from the org-scoped +// `/organizations/{org}/detectors/` endpoint (filtered to `type:metric_issue`). +// The legacy `/organizations/{org}/alert-rules/` endpoint is being retired on +// 2026-08-17 (getsentry/cli#1274, #1182). Detectors return a nested shape, so +// `mapDetectorToMetricAlertRule` flattens each detector into the existing +// `MetricAlertRule` shape the commands already consume. Mutations +// (create/update/delete) still use the legacy endpoint pending a follow-up +// migration to the detectors/monitors + workflows write APIs. + +/** Search query that limits the detectors endpoint to metric alert rules. */ +const METRIC_DETECTOR_QUERY = "type:metric_issue"; /** * List metric alert rules for an organization with cursor-based pagination. * + * Reads from the org-scoped `/detectors/` endpoint filtered to + * `type:metric_issue` and maps each detector onto the flat `MetricAlertRule` + * shape. + * * @param orgSlug - Organization slug * @param options - Pagination parameters (perPage, cursor) * @returns Paginated response with metric alert rules and optional next cursor @@ -177,13 +275,20 @@ export async function listMetricAlertsPaginated( options: { perPage?: number; cursor?: string } = {} ): Promise> { const regionUrl = await resolveOrgRegion(orgSlug); - const { data, headers } = await apiRequestToRegion( + const { data, headers } = await apiRequestToRegion( regionUrl, - `/organizations/${orgSlug}/alert-rules/`, - { params: { per_page: options.perPage, cursor: options.cursor } } + `/organizations/${orgSlug}/detectors/`, + { + params: { + query: METRIC_DETECTOR_QUERY, + sortBy: "-id", + per_page: options.perPage, + cursor: options.cursor, + }, + } ); const { nextCursor } = parseLinkHeader(headers.get("link") ?? null); - return { data, nextCursor }; + return { data: data.map(mapDetectorToMetricAlertRule), nextCursor }; } /** Single GET for org metric alert rule (typed or full JSON for PUT). */ @@ -202,16 +307,23 @@ async function fetchMetricAlertRuleJson( /** * Get a single metric alert rule by ID. * + * Reads from the org-scoped `/detectors/{id}/` endpoint and maps the detector + * onto the flat `MetricAlertRule` shape. + * * @param orgSlug - Organization slug - * @param ruleId - Alert rule ID + * @param ruleId - Detector (alert rule) ID * @returns The metric alert rule */ export async function getMetricAlertRule( orgSlug: string, ruleId: string ): Promise { - const data = await fetchMetricAlertRuleJson(orgSlug, ruleId); - return data as MetricAlertRule; + const regionUrl = await resolveOrgRegion(orgSlug); + const { data } = await apiRequestToRegion( + regionUrl, + `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/` + ); + return mapDetectorToMetricAlertRule(data); } // Issue alert write operations diff --git a/test/commands/alert/metrics/list.test.ts b/test/commands/alert/metrics/list.test.ts index 7234fc7134..66b53203db 100644 --- a/test/commands/alert/metrics/list.test.ts +++ b/test/commands/alert/metrics/list.test.ts @@ -13,6 +13,46 @@ const getConfigDir = useTestConfigDir("test-alert-metrics-list-", { isolateProjectRoot: true, }); +/** + * Build a metric-issue detector payload (the shape returned by the org-scoped + * `/detectors/` endpoint) from the flat metric-alert fields the assertions use. + * `status` 1 maps to a disabled detector (`enabled: false`); anything else is + * enabled. + */ +function detector(fields: { + id: string; + name: string; + status?: number; + query?: string; + aggregate?: string; + dataset?: string; + timeWindow?: number; + environment?: string | null; + projects?: string[]; + dateCreated?: string; +}) { + return { + id: fields.id, + name: fields.name, + type: "metric_issue", + enabled: fields.status !== 1, + projects: fields.projects ?? [], + owner: null, + dateCreated: fields.dateCreated ?? "2026-01-01T00:00:00Z", + dataSources: [ + { + aggregate: fields.aggregate ?? "count()", + dataset: fields.dataset ?? "errors", + query: fields.query ?? "event.type:error", + timeWindow: fields.timeWindow ?? 5, + environment: fields.environment ?? null, + }, + ], + conditionGroup: null, + config: { detectionType: "static" }, + }; +} + type ListFlags = { readonly web: boolean; readonly fresh: boolean; @@ -81,23 +121,9 @@ describe("alert metrics list pagination", () => { test("hasMore is false when limit is reached without next cursor", async () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); - if (req.url.includes("/organizations/test-org/alert-rules/")) { + if (req.url.includes("/organizations/test-org/detectors/")) { return new Response( - JSON.stringify([ - { - id: "1", - name: "Metric Rule Alpha", - status: 0, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: null, - owner: null, - projects: [], - dateCreated: "2026-01-01T00:00:00Z", - }, - ]), + JSON.stringify([detector({ id: "1", name: "Metric Rule Alpha" })]), { status: 200, headers: { @@ -132,23 +158,9 @@ describe("alert metrics list pagination", () => { test("empty query page keeps hasMore when next cursor exists", async () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); - if (req.url.includes("/organizations/test-org/alert-rules/")) { + if (req.url.includes("/organizations/test-org/detectors/")) { return new Response( - JSON.stringify([ - { - id: "1", - name: "Metric Rule Alpha", - status: 0, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: null, - owner: null, - projects: [], - dateCreated: "2026-01-01T00:00:00Z", - }, - ]), + JSON.stringify([detector({ id: "1", name: "Metric Rule Alpha" })]), { status: 200, headers: { @@ -220,7 +232,7 @@ describe("alert metrics list pagination", () => { name: "My Project", }); } - if (url.pathname.includes("/alert-rules/")) { + if (url.pathname.includes("/detectors/")) { throw new Error("rule fetch should not be called for --web"); } return new Response(JSON.stringify({ detail: "Not found" }), { @@ -270,7 +282,7 @@ describe("alert metrics list pagination", () => { name: "My Project", }); } - if (url.pathname.includes("/alert-rules/")) { + if (url.pathname.includes("/detectors/")) { throw new Error("rule fetch should not be called for --web"); } return new Response(JSON.stringify({ detail: "Not found" }), { @@ -311,21 +323,13 @@ describe("alert metrics list pagination", () => { name: "Metrics Project", }); } - if (url.pathname === "/api/0/organizations/org-one/alert-rules/") { + if (url.pathname === "/api/0/organizations/org-one/detectors/") { return Response.json([ - { + detector({ id: "metrics-rule", name: "Metrics Rule", - status: 0, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: null, - owner: null, projects: ["metrics"], - dateCreated: "2026-01-01T00:00:00Z", - }, + }), ]); } return new Response(JSON.stringify({ detail: "Not found" }), { @@ -354,25 +358,20 @@ describe("alert metrics list pagination", () => { setOrgRegion("org-one", DEFAULT_SENTRY_URL); setOrgRegion("org-two", DEFAULT_SENTRY_URL); - const rule = (org: string) => ({ - id: `${org}-1`, - name: `${org} Metric Rule`, - status: org === "org-one" ? 0 : 1, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: "prod", - owner: null, - projects: ["myproj"], - dateCreated: "2026-01-01T00:00:00Z", - }); + const rule = (org: string) => + detector({ + id: `${org}-1`, + name: `${org} Metric Rule`, + status: org === "org-one" ? 0 : 1, + environment: "prod", + projects: ["myproj"], + }); globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); const url = new URL(req.url); const alertMatch = url.pathname.match( - /\/api\/0\/organizations\/([^/]+)\/alert-rules\// + /\/api\/0\/organizations\/([^/]+)\/detectors\// ); if (alertMatch) { return Response.json([rule(alertMatch[1] as string)]); @@ -429,7 +428,7 @@ describe("alert metrics list pagination", () => { const req = new Request(input, init); const url = new URL(req.url); const alertMatch = url.pathname.match( - /\/api\/0\/organizations\/([^/]+)\/alert-rules\// + /\/api\/0\/organizations\/([^/]+)\/detectors\// ); if (alertMatch) { const org = alertMatch[1] as string; @@ -439,19 +438,11 @@ describe("alert metrics list pagination", () => { }); } return Response.json([ - { + detector({ id: "ok-1", name: "Surviving Metric Rule", - status: 0, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: null, - owner: null, projects: ["myproj"], - dateCreated: "2026-01-01T00:00:00Z", - }, + }), ]); } if (url.pathname === "/api/0/organizations/") { @@ -507,7 +498,7 @@ describe("alert metrics list pagination", () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); const url = new URL(req.url); - if (url.pathname === "/api/0/organizations/test-org/alert-rules/") { + if (url.pathname === "/api/0/organizations/test-org/detectors/") { return new Response(JSON.stringify({ detail: "scope denied" }), { status: 403, }); @@ -532,7 +523,7 @@ describe("alert metrics list pagination", () => { ).rejects.toMatchObject({ name: "ApiError", status: 403, - endpoint: "/organizations/test-org/alert-rules/", + endpoint: "/organizations/test-org/detectors/", } satisfies Partial); }); @@ -542,25 +533,14 @@ describe("alert metrics list pagination", () => { setOrgRegion("org-three", DEFAULT_SENTRY_URL); const perPageByOrg = new Map(); - const rule = (org: string, index: number) => ({ - id: `${org}-${index}`, - name: `${org} Metric Rule ${index}`, - status: 0, - query: "event.type:error", - aggregate: "count()", - dataset: "errors", - timeWindow: 5, - environment: null, - owner: null, - projects: [], - dateCreated: "2026-01-01T00:00:00Z", - }); + const rule = (org: string, index: number) => + detector({ id: `${org}-${index}`, name: `${org} Metric Rule ${index}` }); globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); const url = new URL(req.url); const alertMatch = url.pathname.match( - /\/api\/0\/organizations\/([^/]+)\/alert-rules\// + /\/api\/0\/organizations\/([^/]+)\/detectors\// ); if (alertMatch) { const org = alertMatch[1] as string; diff --git a/test/lib/api/alerts.test.ts b/test/lib/api/alerts.test.ts index 617ec4ec17..00db79991e 100644 --- a/test/lib/api/alerts.test.ts +++ b/test/lib/api/alerts.test.ts @@ -3,7 +3,9 @@ import { deleteIssueAlertRule, deleteMetricAlertRule, getIssueAlertRule, + getMetricAlertRule, listIssueAlertsPaginated, + listMetricAlertsPaginated, } from "../../../src/lib/api/alerts.js"; import { DEFAULT_SENTRY_URL } from "../../../src/lib/constants.js"; import { setAuthToken } from "../../../src/lib/db/auth.js"; @@ -109,6 +111,98 @@ describe("getIssueAlertRule", () => { }); }); +/** Minimal metric-issue detector payload from the org-scoped `/detectors/` endpoint. */ +function metricDetector(overrides: Record) { + return { + id: "9", + name: "P95 latency", + type: "metric_issue", + enabled: true, + projects: ["backend"], + owner: null, + dateCreated: "2026-01-01T00:00:00Z", + dataSources: [ + { + aggregate: "p95(transaction.duration)", + dataset: "transactions", + query: "environment:prod", + timeWindow: 5, + environment: "prod", + }, + ], + conditionGroup: null, + config: { detectionType: "static" }, + ...overrides, + }; +} + +describe("listMetricAlertsPaginated", () => { + test("reads from org-scoped /detectors/ filtered to metric_issue and flattens the payload", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const url = new URL(new Request(input!, init).url); + expect(url.pathname).toBe("/api/0/organizations/test-org/detectors/"); + expect(url.searchParams.get("query")).toBe("type:metric_issue"); + return Response.json([metricDetector({ id: "9", name: "P95 latency" })]); + }); + + const { data } = await listMetricAlertsPaginated("test-org"); + expect(data).toHaveLength(1); + expect(data[0]).toMatchObject({ + id: "9", + name: "P95 latency", + status: 0, + aggregate: "p95(transaction.duration)", + dataset: "transactions", + query: "environment:prod", + timeWindow: 5, + environment: "prod", + projects: ["backend"], + }); + }); +}); + +describe("getMetricAlertRule", () => { + test("reads from /detectors/{id}/ and maps disabled detectors to status 1", async () => { + globalThis.fetch = mockFetch(async (input, init) => { + const url = new URL(new Request(input!, init).url); + expect(url.pathname).toBe("/api/0/organizations/test-org/detectors/9/"); + return Response.json( + metricDetector({ id: "9", name: "Disabled rule", enabled: false }) + ); + }); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.id).toBe("9"); + expect(rule.name).toBe("Disabled rule"); + expect(rule.status).toBe(1); + }); + + test("reads threshold fields nested under snubaQuery", async () => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ + dataSources: [ + { + snubaQuery: { + aggregate: "count()", + dataset: "errors", + query: "event.type:error", + timeWindow: 15, + }, + }, + ], + }) + ) + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.aggregate).toBe("count()"); + expect(rule.dataset).toBe("errors"); + expect(rule.query).toBe("event.type:error"); + expect(rule.timeWindow).toBe(15); + }); +}); + describe("deleteMetricAlertRule", () => { test("treats empty-body 202 as success", async () => { globalThis.fetch = mockFetch(async (input, init) => { From 227bf7538a2c9d398fb0a8c0761610a1f15bb3a7 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 11:07:34 +0000 Subject: [PATCH 2/6] fix(alerts): normalize detector timeWindow (seconds) and project scope Metric detectors expose the threshold window in seconds via dataSources/snubaQuery, but MetricAlertRule and every CLI formatter treat timeWindow as minutes (list/view append 'm'). Convert seconds->minutes so windows no longer render ~60x too large. Detectors are project-scoped and expose projectSlug rather than the legacy projects slug array, so metric alert view was showing '(all)'; map projects from projectSlug with a projects[] fallback. Also harden pickDetectorNumber to coerce non-numeric/NaN values to 0 so a malformed window renders as 0m instead of NaNm. Addresses Cursor Bugbot and Sentry review findings on #1275. --- src/lib/api/alerts.ts | 37 +++++++++++++++++++++--- test/commands/alert/metrics/list.test.ts | 7 +++-- test/lib/api/alerts.test.ts | 33 +++++++++++++++++++-- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index be59c38ed0..18e9efca18 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -91,14 +91,43 @@ function pickDetectorString(value: unknown): string { function pickDetectorNumber(value: unknown): number { if (typeof value === "number") { - return value; + return Number.isFinite(value) ? value : 0; } if (typeof value === "string" && value.trim() !== "") { - return Number(value); + const parsed = Number(value); + return Number.isFinite(parsed) ? parsed : 0; } return 0; } +/** + * Convert a detector `timeWindow` (seconds) to the minutes the CLI expects. + * + * The detectors/`snubaQuery` payload exposes `timeWindow` in seconds, while the + * legacy `/alert-rules/` serializer (and every CLI formatter — list/view append + * `m`) treats `MetricAlertRule.timeWindow` as minutes. Without this conversion + * windows render ~60× too large. Metric windows are always whole minutes, so we + * round to guard against float drift. + */ +function detectorTimeWindowToMinutes(value: unknown): number { + const seconds = pickDetectorNumber(value); + return seconds > 0 ? Math.round(seconds / 60) : 0; +} + +/** + * Resolve the project slug list for a detector. + * + * Detectors are project-scoped and expose a single `projectSlug`, not the + * legacy `projects` slug array; fall back to `projects` for deployments that + * still return it so metric alert view doesn't incorrectly show `(all)`. + */ +function pickDetectorProjects(detector: MetricDetector): string[] { + if (detector.projectSlug) { + return [detector.projectSlug]; + } + return detector.projects ?? []; +} + /** Reduce a detector owner (object or bare string) to the flat owner value. */ function pickDetectorOwner(owner: MetricDetector["owner"]): string | null { if (typeof owner === "string") { @@ -129,10 +158,10 @@ function mapDetectorToMetricAlertRule( query: pickDetectorString(nested.query), aggregate: pickDetectorString(nested.aggregate), dataset: pickDetectorString(nested.dataset), - timeWindow: pickDetectorNumber(nested.timeWindow), + timeWindow: detectorTimeWindowToMinutes(nested.timeWindow), environment, owner: pickDetectorOwner(detector.owner), - projects: detector.projects ?? [], + projects: pickDetectorProjects(detector), dateCreated: detector.dateCreated ?? "", }; } diff --git a/test/commands/alert/metrics/list.test.ts b/test/commands/alert/metrics/list.test.ts index 66b53203db..373dbf19b8 100644 --- a/test/commands/alert/metrics/list.test.ts +++ b/test/commands/alert/metrics/list.test.ts @@ -31,12 +31,15 @@ function detector(fields: { projects?: string[]; dateCreated?: string; }) { + // Detectors are project-scoped (`projectSlug`) and expose the window in + // seconds, so mirror that here; the builder accepts minutes for readability. + const [projectSlug] = fields.projects ?? []; return { id: fields.id, name: fields.name, type: "metric_issue", enabled: fields.status !== 1, - projects: fields.projects ?? [], + projectSlug: projectSlug ?? null, owner: null, dateCreated: fields.dateCreated ?? "2026-01-01T00:00:00Z", dataSources: [ @@ -44,7 +47,7 @@ function detector(fields: { aggregate: fields.aggregate ?? "count()", dataset: fields.dataset ?? "errors", query: fields.query ?? "event.type:error", - timeWindow: fields.timeWindow ?? 5, + timeWindow: (fields.timeWindow ?? 5) * 60, environment: fields.environment ?? null, }, ], diff --git a/test/lib/api/alerts.test.ts b/test/lib/api/alerts.test.ts index 00db79991e..e3e623d0b8 100644 --- a/test/lib/api/alerts.test.ts +++ b/test/lib/api/alerts.test.ts @@ -118,7 +118,7 @@ function metricDetector(overrides: Record) { name: "P95 latency", type: "metric_issue", enabled: true, - projects: ["backend"], + projectSlug: "backend", owner: null, dateCreated: "2026-01-01T00:00:00Z", dataSources: [ @@ -126,7 +126,8 @@ function metricDetector(overrides: Record) { aggregate: "p95(transaction.duration)", dataset: "transactions", query: "environment:prod", - timeWindow: 5, + // Detectors expose the window in seconds; 300s == 5m. + timeWindow: 300, environment: "prod", }, ], @@ -154,6 +155,7 @@ describe("listMetricAlertsPaginated", () => { aggregate: "p95(transaction.duration)", dataset: "transactions", query: "environment:prod", + // 300s from the detector payload is normalized to 5 minutes. timeWindow: 5, environment: "prod", projects: ["backend"], @@ -187,7 +189,8 @@ describe("getMetricAlertRule", () => { aggregate: "count()", dataset: "errors", query: "event.type:error", - timeWindow: 15, + // 900s == 15m after normalization. + timeWindow: 900, }, }, ], @@ -201,6 +204,30 @@ describe("getMetricAlertRule", () => { expect(rule.query).toBe("event.type:error"); expect(rule.timeWindow).toBe(15); }); + + test("prefers projectSlug and falls back to a projects array for projects", async () => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ projectSlug: undefined, projects: ["frontend"] }) + ) + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.projects).toEqual(["frontend"]); + }); + + test("coerces a non-numeric timeWindow to 0 instead of NaN", async () => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ + dataSources: [{ snubaQuery: { timeWindow: "not-a-number" } }], + }) + ) + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.timeWindow).toBe(0); + }); }); describe("deleteMetricAlertRule", () => { From f098c22a1969deacddebea6aeb63d853ab3065d7 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 11:10:56 +0000 Subject: [PATCH 3/6] fix(alerts): resolve nested queryObj.snubaQuery threshold fields The detectors API can nest the SnubaQuery threshold fields (aggregate/dataset/query/timeWindow) at dataSources[0].queryObj.snubaQuery, not just under a flat queryObj or snubaQuery. The old queryObj ?? snubaQuery ?? source chain picked the queryObj wrapper and read empty fields off it, flattening aggregate/query/dataset to empty and timeWindow to 0 on list/view. Add resolveThresholdSource to walk queryObj.snubaQuery -> queryObj -> snubaQuery -> dataSource and return the first container that actually exposes an aggregate/query. Adds a queryObj.snubaQuery mapping test. Addresses Cursor Bugbot finding on #1275. --- src/lib/api/alerts.ts | 49 +++++++++++++++++++++++++++++++------ test/lib/api/alerts.test.ts | 27 ++++++++++++++++++++ 2 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index 18e9efca18..844750b58c 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -80,10 +80,11 @@ type MetricDetector = { * Map a metric-issue detector onto the flat `MetricAlertRule` shape the CLI * commands (list/view/resolve) already consume. * - * The threshold query fields live in the first data source. Some deployments - * nest them under a `queryObj`/`snubaQuery` sub-object, so we look there as a - * fallback. `enabled === false` maps to the legacy disabled status (1); an - * enabled or absent flag maps to active (0), matching `metricAlertStatusLabel`. + * The threshold query fields live in the first data source, possibly nested + * under `queryObj`, `snubaQuery`, or `queryObj.snubaQuery` — `resolveThresholdSource` + * picks the right container. `enabled === false` maps to the legacy disabled + * status (1); an enabled or absent flag maps to active (0), matching + * `metricAlertStatusLabel`. */ function pickDetectorString(value: unknown): string { return typeof value === "string" ? value : ""; @@ -139,14 +140,46 @@ function pickDetectorOwner(owner: MetricDetector["owner"]): string | null { return null; } +/** + * Locate the object holding the threshold query fields for a detector. + * + * Depending on deployment the `SnubaQuery` fields (aggregate/dataset/query/ + * timeWindow) sit directly on `dataSources[0]`, under a `queryObj`, under a + * `snubaQuery`, or nested at `queryObj.snubaQuery`. Walk those candidates + * breadth-first and return the first that actually exposes an `aggregate` or + * `query`, so a wrapper object (e.g. a `queryObj` whose real fields live in a + * nested `snubaQuery`) doesn't flatten everything to empty. + */ +function resolveThresholdSource( + source: Record +): Record { + const asRecord = (value: unknown): Record | undefined => + value && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : undefined; + + const queryObj = asRecord(source.queryObj); + const snubaQuery = asRecord(source.snubaQuery); + const candidates = [ + queryObj && asRecord(queryObj.snubaQuery), + queryObj, + snubaQuery, + source, + ]; + + for (const candidate of candidates) { + if (candidate && ("aggregate" in candidate || "query" in candidate)) { + return candidate; + } + } + return source; +} + function mapDetectorToMetricAlertRule( detector: MetricDetector ): MetricAlertRule { const source = detector.dataSources?.[0] ?? {}; - const nested = - (source.queryObj as Record | undefined) ?? - (source.snubaQuery as Record | undefined) ?? - source; + const nested = resolveThresholdSource(source); const environment = pickDetectorString(nested.environment) || detector.environment || null; diff --git a/test/lib/api/alerts.test.ts b/test/lib/api/alerts.test.ts index e3e623d0b8..446e56b337 100644 --- a/test/lib/api/alerts.test.ts +++ b/test/lib/api/alerts.test.ts @@ -205,6 +205,33 @@ describe("getMetricAlertRule", () => { expect(rule.timeWindow).toBe(15); }); + test("reads threshold fields nested under queryObj.snubaQuery", async () => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ + dataSources: [ + { + queryObj: { + snubaQuery: { + aggregate: "p75(measurements.lcp)", + dataset: "transactions", + query: "transaction.op:pageload", + timeWindow: 600, + }, + }, + }, + ], + }) + ) + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.aggregate).toBe("p75(measurements.lcp)"); + expect(rule.dataset).toBe("transactions"); + expect(rule.query).toBe("transaction.op:pageload"); + expect(rule.timeWindow).toBe(10); + }); + test("prefers projectSlug and falls back to a projects array for projects", async () => { globalThis.fetch = mockFetch(async () => Response.json( From 472b6b8ca6e8246202de9452cbcac429836d52b6 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 11:14:22 +0000 Subject: [PATCH 4/6] fix(alerts): reject non-metric detectors in getMetricAlertRule The /detectors/{id}/ endpoint returns any detector kind (uptime, cron, error), unlike the type:metric_issue-filtered list. getMetricAlertRule mapped the payload blind, so a numeric id resolving to a non-metric detector rendered as a metric alert with empty threshold fields instead of 404ing like the old /alert-rules/{id}/ path. Type the detector 'type' field and throw a 404 ApiError when it is present and not metric_issue; an absent type still maps (defensive for deployments that omit it). Adds a rejection test. Addresses Cursor Bugbot finding on #1275. --- src/lib/api/alerts.ts | 18 +++++++++++++++++- test/lib/api/alerts.test.ts | 11 +++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index 844750b58c..2810a99eed 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -67,6 +67,8 @@ export type MetricAlertRule = { type MetricDetector = { id: string | number; name: string; + /** Detector kind, e.g. `metric_issue`. Other kinds (uptime, cron) are rejected on read. */ + type?: string; enabled?: boolean; environment?: string | null; projectSlug?: string | null; @@ -372,6 +374,11 @@ async function fetchMetricAlertRuleJson( * Reads from the org-scoped `/detectors/{id}/` endpoint and maps the detector * onto the flat `MetricAlertRule` shape. * + * The `/detectors/{id}/` endpoint returns any detector kind (uptime, cron, + * error, …), unlike the `type:metric_issue`-filtered list. A non-metric id is + * rejected with a 404 so view/edit/delete don't silently render an unrelated + * detector as a metric alert with empty threshold fields. + * * @param orgSlug - Organization slug * @param ruleId - Detector (alert rule) ID * @returns The metric alert rule @@ -381,10 +388,19 @@ export async function getMetricAlertRule( ruleId: string ): Promise { const regionUrl = await resolveOrgRegion(orgSlug); + const endpoint = `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/`; const { data } = await apiRequestToRegion( regionUrl, - `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/` + endpoint ); + if (data.type !== undefined && data.type !== "metric_issue") { + throw new ApiError( + `Metric alert rule '${ruleId}' not found`, + 404, + `Detector '${ruleId}' is of type '${data.type}', not a metric alert.`, + endpoint + ); + } return mapDetectorToMetricAlertRule(data); } diff --git a/test/lib/api/alerts.test.ts b/test/lib/api/alerts.test.ts index 446e56b337..0b845ecb03 100644 --- a/test/lib/api/alerts.test.ts +++ b/test/lib/api/alerts.test.ts @@ -255,6 +255,17 @@ describe("getMetricAlertRule", () => { const rule = await getMetricAlertRule("test-org", "9"); expect(rule.timeWindow).toBe(0); }); + + test("rejects a non-metric detector with a 404 instead of mapping it", async () => { + globalThis.fetch = mockFetch(async () => + Response.json(metricDetector({ type: "uptime_domain_failure" })) + ); + + await expect(getMetricAlertRule("test-org", "9")).rejects.toMatchObject({ + name: "ApiError", + status: 404, + }); + }); }); describe("deleteMetricAlertRule", () => { From 77894accf49a77928b30f3c1cb1629c5d1bc4836 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 11:17:58 +0000 Subject: [PATCH 5/6] fix(alerts): preserve actor type in detector owner mapping pickDetectorOwner flattened object owners to name/id, dropping the actor type. Legacy /alert-rules/ returned owner as an actor string (user:123 / team:456) and the CLI formatters/JSON still treat it that way, so list/ view emitted a different, less-usable owner value. Reconstruct type:id from the detector owner object when both are present; fall back to bare id/name otherwise. Adds a mapping test. Addresses Cursor Bugbot finding on #1275. --- src/lib/api/alerts.ts | 14 ++++++++++++-- test/lib/api/alerts.test.ts | 11 +++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index 2810a99eed..a86b7cdf28 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -131,13 +131,23 @@ function pickDetectorProjects(detector: MetricDetector): string[] { return detector.projects ?? []; } -/** Reduce a detector owner (object or bare string) to the flat owner value. */ +/** + * Reduce a detector owner (object or bare string) to the flat actor identifier. + * + * Legacy `/alert-rules/` returned `owner` as an actor string like `user:123` + * or `team:456`, and the CLI formatters/JSON still treat it that way. Detectors + * expose the owner as `{ type, id, name }`, so reconstruct `type:id` to preserve + * that shape; fall back to a bare `id`/`name` when `type` is absent. + */ function pickDetectorOwner(owner: MetricDetector["owner"]): string | null { if (typeof owner === "string") { return owner; } if (owner && typeof owner === "object") { - return owner.name ?? owner.id ?? null; + if (owner.type && owner.id) { + return `${owner.type}:${owner.id}`; + } + return owner.id ?? owner.name ?? null; } return null; } diff --git a/test/lib/api/alerts.test.ts b/test/lib/api/alerts.test.ts index 0b845ecb03..db37ea993a 100644 --- a/test/lib/api/alerts.test.ts +++ b/test/lib/api/alerts.test.ts @@ -266,6 +266,17 @@ describe("getMetricAlertRule", () => { status: 404, }); }); + + test("reconstructs the legacy actor identifier from an object owner", async () => { + globalThis.fetch = mockFetch(async () => + Response.json( + metricDetector({ owner: { type: "team", id: "42", name: "backend" } }) + ) + ); + + const rule = await getMetricAlertRule("test-org", "9"); + expect(rule.owner).toBe("team:42"); + }); }); describe("deleteMetricAlertRule", () => { From 26bacd5d5f982a473fe8fc2ccaae4622b5b1da28 Mon Sep 17 00:00:00 2001 From: "jared-outpost[bot]" Date: Wed, 22 Jul 2026 12:37:20 +0000 Subject: [PATCH 6/6] refactor(alerts): use @sentry/api detector SDK helpers for metric reads Address review feedback on #1275: - listMetricAlertsPaginated / getMetricAlertRule now call the generated @sentry/api listOrganizationDetectors / getOrganizationDetector helpers via getOrgSdkConfig, instead of hand-rolled /detectors/ URL strings + apiRequestToRegion. Matches the repositories.ts / monitors.ts pattern and the AGENTS.md guidance to prefer @sentry/api. Errors now flow through the shared unwrap* helpers (endpoint is no longer set, consistent with every other SDK-backed module); tests updated accordingly. - Inline the one-line seconds->minutes timeWindow conversion instead of a standalone helper. - Name the detector fixture dateCreated constant (DETECTOR_DATE_CREATED) rather than an inline magic timestamp. --- src/lib/api/alerts.ts | 82 ++++++++++++++---------- test/commands/alert/metrics/list.test.ts | 14 ++-- 2 files changed, 58 insertions(+), 38 deletions(-) diff --git a/src/lib/api/alerts.ts b/src/lib/api/alerts.ts index a86b7cdf28..1a70196c11 100644 --- a/src/lib/api/alerts.ts +++ b/src/lib/api/alerts.ts @@ -6,13 +6,20 @@ * - Metric alerts: threshold-based rules that trigger on metric queries (org-wide) */ +import { + getOrganizationDetector, + listOrganizationDetectors, +} from "@sentry/api"; import { ApiError } from "../errors.js"; import { resolveOrgRegion } from "../region.js"; import { apiRequestToRegion, apiRequestToRegionNoContent, + getOrgSdkConfig, type PaginatedResponse, parseLinkHeader, + unwrapPaginatedResult, + unwrapResult, } from "./infrastructure.js"; // Types @@ -103,20 +110,6 @@ function pickDetectorNumber(value: unknown): number { return 0; } -/** - * Convert a detector `timeWindow` (seconds) to the minutes the CLI expects. - * - * The detectors/`snubaQuery` payload exposes `timeWindow` in seconds, while the - * legacy `/alert-rules/` serializer (and every CLI formatter — list/view append - * `m`) treats `MetricAlertRule.timeWindow` as minutes. Without this conversion - * windows render ~60× too large. Metric windows are always whole minutes, so we - * round to guard against float drift. - */ -function detectorTimeWindowToMinutes(value: unknown): number { - const seconds = pickDetectorNumber(value); - return seconds > 0 ? Math.round(seconds / 60) : 0; -} - /** * Resolve the project slug list for a detector. * @@ -196,6 +189,10 @@ function mapDetectorToMetricAlertRule( const environment = pickDetectorString(nested.environment) || detector.environment || null; + // The detector payload exposes timeWindow in seconds, but MetricAlertRule + // (and every CLI formatter, which appends "m") treats it as whole minutes. + const timeWindowSeconds = pickDetectorNumber(nested.timeWindow); + return { id: String(detector.id), name: detector.name, @@ -203,7 +200,7 @@ function mapDetectorToMetricAlertRule( query: pickDetectorString(nested.query), aggregate: pickDetectorString(nested.aggregate), dataset: pickDetectorString(nested.dataset), - timeWindow: detectorTimeWindowToMinutes(nested.timeWindow), + timeWindow: timeWindowSeconds > 0 ? Math.round(timeWindowSeconds / 60) : 0, environment, owner: pickDetectorOwner(detector.owner), projects: pickDetectorProjects(detector), @@ -348,20 +345,26 @@ export async function listMetricAlertsPaginated( orgSlug: string, options: { perPage?: number; cursor?: string } = {} ): Promise> { - const regionUrl = await resolveOrgRegion(orgSlug); - const { data, headers } = await apiRequestToRegion( - regionUrl, - `/organizations/${orgSlug}/detectors/`, - { - params: { - query: METRIC_DETECTOR_QUERY, - sortBy: "-id", - per_page: options.perPage, - cursor: options.cursor, - }, - } + const config = await getOrgSdkConfig(orgSlug); + const result = await listOrganizationDetectors({ + ...config, + path: { organization_id_or_slug: orgSlug }, + query: { + query: METRIC_DETECTOR_QUERY, + sortBy: "-id", + cursor: options.cursor, + per_page: options.perPage, + } as { + query?: string; + sortBy?: string; + cursor?: string; + per_page?: number; + }, + }); + const { data, nextCursor } = unwrapPaginatedResult( + result, + "Failed to list metric alert rules" ); - const { nextCursor } = parseLinkHeader(headers.get("link") ?? null); return { data: data.map(mapDetectorToMetricAlertRule), nextCursor }; } @@ -397,18 +400,29 @@ export async function getMetricAlertRule( orgSlug: string, ruleId: string ): Promise { - const regionUrl = await resolveOrgRegion(orgSlug); - const endpoint = `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/`; - const { data } = await apiRequestToRegion( - regionUrl, - endpoint + const config = await getOrgSdkConfig(orgSlug); + const result = await getOrganizationDetector({ + ...config, + // The SDK types detector_id as number, but detector IDs are opaque strings + // the API accepts verbatim; pass the string through the string-keyed path. + path: { + organization_id_or_slug: orgSlug, + detector_id: ruleId, + } as unknown as { + organization_id_or_slug: string; + detector_id: number; + }, + }); + const data = unwrapResult( + result, + `Failed to get metric alert rule '${ruleId}'` ); if (data.type !== undefined && data.type !== "metric_issue") { throw new ApiError( `Metric alert rule '${ruleId}' not found`, 404, `Detector '${ruleId}' is of type '${data.type}', not a metric alert.`, - endpoint + `/organizations/${orgSlug}/detectors/${encodeURIComponent(ruleId)}/` ); } return mapDetectorToMetricAlertRule(data); diff --git a/test/commands/alert/metrics/list.test.ts b/test/commands/alert/metrics/list.test.ts index 373dbf19b8..2fed40cec0 100644 --- a/test/commands/alert/metrics/list.test.ts +++ b/test/commands/alert/metrics/list.test.ts @@ -13,6 +13,13 @@ const getConfigDir = useTestConfigDir("test-alert-metrics-list-", { isolateProjectRoot: true, }); +/** + * Fixed `dateCreated` for detector fixtures. Detectors always carry a creation + * timestamp, so the builder stamps this deterministic value; the exact instant + * is arbitrary and not asserted on — only its presence matters. + */ +const DETECTOR_DATE_CREATED = "2026-01-01T00:00:00Z"; + /** * Build a metric-issue detector payload (the shape returned by the org-scoped * `/detectors/` endpoint) from the flat metric-alert fields the assertions use. @@ -41,7 +48,7 @@ function detector(fields: { enabled: fields.status !== 1, projectSlug: projectSlug ?? null, owner: null, - dateCreated: fields.dateCreated ?? "2026-01-01T00:00:00Z", + dateCreated: fields.dateCreated ?? DETECTOR_DATE_CREATED, dataSources: [ { aggregate: fields.aggregate ?? "count()", @@ -489,7 +496,7 @@ describe("alert metrics list pagination", () => { expect.objectContaining({ org: "org-two", status: 500, - message: expect.stringContaining("API request failed"), + message: expect.stringContaining("Failed to list metric alert rules"), }), ]); expect(warnSpy).toHaveBeenCalledWith( @@ -497,7 +504,7 @@ describe("alert metrics list pagination", () => { ); }); - test("all org failures preserve ApiError status and endpoint", async () => { + test("all org failures preserve ApiError status", async () => { globalThis.fetch = mockFetch(async (input, init) => { const req = new Request(input, init); const url = new URL(req.url); @@ -526,7 +533,6 @@ describe("alert metrics list pagination", () => { ).rejects.toMatchObject({ name: "ApiError", status: 403, - endpoint: "/organizations/test-org/detectors/", } satisfies Partial); });