From 0dc1062af985584fe7e2d59e4b4d184a9ad0ea90 Mon Sep 17 00:00:00 2001 From: Drew Davis Date: Tue, 15 Sep 2026 13:43:50 -0400 Subject: [PATCH] feat: Add pagination to the list alerts API --- .changeset/paginate-alerts-list.md | 6 + packages/api/scripts/seed-alerts.ts | 58 ++- .../controllers/__tests__/alertsPage.test.ts | 226 ++++++++++ packages/api/src/controllers/alerts.ts | 47 +- packages/api/src/controllers/alertsPage.ts | 170 +++++++ .../mcp/__tests__/listMetricsCursor.test.ts | 2 +- .../api/src/mcp/tools/sources/listMetrics.ts | 36 +- packages/api/src/models/alert.ts | 4 + .../routers/api/__tests__/alerts.int.test.ts | 10 +- .../__tests__/alertsPagination.int.test.ts | 414 ++++++++++++++++++ packages/api/src/routers/api/alerts.ts | 86 ++-- packages/api/src/routers/api/prometheus.ts | 24 +- .../src/utils/__tests__/pagination.test.ts | 51 +++ packages/api/src/utils/pagination.ts | 28 ++ packages/api/src/utils/zod.ts | 14 + .../__tests__/AlertDetailProperties.test.tsx | 25 +- packages/common-utils/src/types.ts | 19 +- 17 files changed, 1074 insertions(+), 146 deletions(-) create mode 100644 .changeset/paginate-alerts-list.md create mode 100644 packages/api/src/controllers/__tests__/alertsPage.test.ts create mode 100644 packages/api/src/controllers/alertsPage.ts create mode 100644 packages/api/src/routers/api/__tests__/alertsPagination.int.test.ts create mode 100644 packages/api/src/utils/__tests__/pagination.test.ts diff --git a/.changeset/paginate-alerts-list.md b/.changeset/paginate-alerts-list.md new file mode 100644 index 0000000000..b837f08194 --- /dev/null +++ b/.changeset/paginate-alerts-list.md @@ -0,0 +1,6 @@ +--- +'@hyperdx/api': patch +'@hyperdx/common-utils': patch +--- + +feat: page and filter GET /alerts server-side \ No newline at end of file diff --git a/packages/api/scripts/seed-alerts.ts b/packages/api/scripts/seed-alerts.ts index 8e30b3eefc..2f257f061b 100644 --- a/packages/api/scripts/seed-alerts.ts +++ b/packages/api/scripts/seed-alerts.ts @@ -15,6 +15,7 @@ * MONGO_URI. The dev stack must be up (`yarn dev`) and registered once, since * the script attaches everything to the existing team and log source. */ +import { formatTileAlertDisplayName } from '@hyperdx/common-utils/dist/alerts'; import { DisplayType } from '@hyperdx/common-utils/dist/types'; import { execFileSync } from 'child_process'; import mongoose from 'mongoose'; @@ -41,10 +42,9 @@ const INSERT_CHUNK_SIZE = 500; /** Roughly what a real team looks like: mostly quiet, a few firing. */ const STATE_WEIGHTS: [AlertState, number][] = [ - [AlertState.OK, 74], + [AlertState.OK, 79], [AlertState.ALERT, 12], [AlertState.PENDING, 9], - [AlertState.DISABLED, 5], ]; const INTERVALS = ['1m', '5m', '15m', '30m', '1h', '6h', '12h', '1d'] as const; @@ -183,6 +183,8 @@ function makeAlertDoc({ teamId, channel, index, + displayName, + tags, savedSearchId, dashboardId, tileId, @@ -190,6 +192,8 @@ function makeAlertDoc({ teamId: unknown; channel: { type: 'webhook'; webhookId: string }; index: number; + displayName: string; + tags: string[]; savedSearchId?: string; dashboardId?: string; tileId?: string; @@ -197,6 +201,10 @@ function makeAlertDoc({ return { team: teamId, source: savedSearchId ? AlertSource.SAVED_SEARCH : AlertSource.TILE, + // Stored on the alert rather than left to derive from the referenced + // saved search or dashboard tile, matching what writers persist now. + displayName, + tags, savedSearch: savedSearchId ?? null, groupBy: null, dashboard: dashboardId ?? null, @@ -320,24 +328,32 @@ async function seed(count: number, tag: string) { ]; // --- Saved searches: up to MAX_ALERTS_PER_SAVED_SEARCH alerts each -------- - const savedSearchPlans: { doc: Record; alerts: number }[] = - []; + const savedSearchPlans: { + doc: Record; + alerts: number; + name: string; + tags: string[]; + }[] = []; for (let remaining = savedSearchAlertTotal; remaining > 0; ) { const alerts = Math.min( remaining, randomInt(1, MAX_ALERTS_PER_SAVED_SEARCH), ); const index = savedSearchPlans.length; + const name = `Seeded search ${stamp}-${index}`; + const tags = tagsFor(index); savedSearchPlans.push({ alerts, + name, + tags, doc: { team: teamId, - name: `Seeded search ${stamp}-${index}`, + name, select: '', where: '', whereLanguage: 'lucene', source: sourceId, - tags: tagsFor(index), + tags, }, }); remaining -= alerts; @@ -355,6 +371,8 @@ async function seed(count: number, tag: string) { teamId, channel, index: alertDocs.length, + displayName: plan.name, + tags: plan.tags, savedSearchId, }), ); @@ -364,7 +382,8 @@ async function seed(count: number, tag: string) { // --- Dashboards: up to MAX_TILES_PER_DASHBOARD tiles, <=1 alert per tile -- const dashboardPlans: { doc: Record; - alertedTileIds: string[]; + alertedTiles: { id: string; displayName: string }[]; + tags: string[]; }[] = []; for (let remaining = tileAlertTotal; remaining > 0; ) { const index = dashboardPlans.length; @@ -374,13 +393,19 @@ async function seed(count: number, tag: string) { ); // Some tiles are left un-alerted, which is the normal case on a dashboard. const alertedCount = Math.min(remaining, randomInt(1, tileCount)); + const name = `Seeded dashboard ${stamp}-${index}`; + const tags = tagsFor(index); dashboardPlans.push({ - alertedTileIds: tiles.slice(0, alertedCount).map(tile => tile.id), + alertedTiles: tiles.slice(0, alertedCount).map(tile => ({ + id: tile.id, + displayName: formatTileAlertDisplayName(name, tile.config.name), + })), + tags, doc: { team: teamId, - name: `Seeded dashboard ${stamp}-${index}`, + name, tiles, - tags: tagsFor(index), + tags, filters: [], }, }); @@ -392,14 +417,16 @@ async function seed(count: number, tag: string) { ); for (const [plan, dashboardId] of zip(dashboardPlans, dashboardIds)) { - for (const tileId of plan.alertedTileIds) { + for (const tile of plan.alertedTiles) { alertDocs.push( makeAlertDoc({ teamId, channel, index: alertDocs.length, + displayName: tile.displayName, + tags: plan.tags, dashboardId, - tileId, + tileId: tile.id, }), ); } @@ -412,7 +439,6 @@ async function seed(count: number, tag: string) { const state = String(alert.state); byState.set(state, (byState.get(state) ?? 0) + 1); } - const disabledCount = byState.get(AlertState.DISABLED) ?? 0; console.log( [ @@ -427,12 +453,6 @@ async function seed(count: number, tag: string) { `Tagged "${tag}" — remove with: yarn seed:alerts --purge --tag ${tag}`, ].join('\n'), ); - if (disabledCount > 0) { - console.log( - `Note: the ${disabledCount} disabled alerts are not listed on the alerts ` + - 'page, which only renders the triggered, pending and OK sections.', - ); - } } async function main() { diff --git a/packages/api/src/controllers/__tests__/alertsPage.test.ts b/packages/api/src/controllers/__tests__/alertsPage.test.ts new file mode 100644 index 0000000000..c3362e3099 --- /dev/null +++ b/packages/api/src/controllers/__tests__/alertsPage.test.ts @@ -0,0 +1,226 @@ +import { MAX_TAGS } from '@hyperdx/common-utils/dist/types'; + +import { + alertsPageQuerySchema, + buildAlertsPageFilter, +} from '@/controllers/alertsPage'; +import { AlertSource, AlertState } from '@/models/alert'; +import { encodeCursor } from '@/utils/pagination'; + +const TEAM_ID = '6500000000000000000000aa'; +const ALERT_ID = '6500000000000000000000bb'; + +const parse = (query: Record) => + alertsPageQuerySchema.parse(query); + +const buildFilter = (query: Record = {}) => + buildAlertsPageFilter(TEAM_ID, parse(query)); + +describe('alertsPageQuerySchema', () => { + it('accepts an empty query', () => { + expect(parse({})).toEqual({}); + }); + + it('normalizes a repeatable param whether it arrives once or many times', () => { + expect(parse({ tag: 'prod' }).tag).toEqual(['prod']); + expect(parse({ tag: ['prod', 'web'] }).tag).toEqual(['prod', 'web']); + expect(parse({ tag: '' }).tag).toBeUndefined(); + }); + + it('rejects more tags than an alert can carry', () => { + expect(() => parse({ tag: Array(MAX_TAGS + 1).fill('t') })).toThrow(); + }); + + it('coerces limit and rejects out-of-range values', () => { + expect(parse({ limit: '25' }).limit).toBe(25); + expect(() => parse({ limit: '0' })).toThrow(); + expect(() => parse({ limit: '501' })).toThrow(); + expect(() => parse({ limit: '1.5' })).toThrow(); + }); + + it('accepts limit and tag count at their inclusive bounds', () => { + expect(parse({ limit: '1' }).limit).toBe(1); + expect(parse({ limit: '500' }).limit).toBe(500); + expect(parse({ tag: Array(MAX_TAGS).fill('t') }).tag).toHaveLength( + MAX_TAGS, + ); + }); + + it('trims search and rejects an overlong one', () => { + expect(parse({ search: ' errors ' }).search).toBe('errors'); + expect(() => parse({ search: 'x'.repeat(513) })).toThrow(); + }); + + it('rejects values outside the source and state enums', () => { + expect(parse({ source: 'tile' }).source).toEqual([AlertSource.TILE]); + expect(() => parse({ source: 'nope' })).toThrow(); + expect(() => parse({ state: 'nope' })).toThrow(); + }); + + it('rejects a non-ObjectId createdBy', () => { + expect(() => parse({ createdBy: 'not-an-id' })).toThrow(); + }); + + it('survives re-parsing its own output', () => { + // processRequest writes the parsed query back onto req.query, and the + // handler parses that again to recover the coerced types. + const once = parse({ + limit: '2', + tag: ['prod', 'web'], + source: 'tile', + state: 'OK', + search: ' errors ', + createdBy: ALERT_ID, + }); + expect(alertsPageQuerySchema.parse(once)).toEqual(once); + }); + + it('rejects a cursor with no limit to continue', () => { + const cursor = encodeCursor({ n: 'a', id: ALERT_ID }); + expect(() => parse({ cursor })).toThrow('cursor requires limit'); + expect(parse({ cursor, limit: '2' }).cursor).toBe(cursor); + }); + + it.each([ + ['garbage', 'not a cursor!'], + ['a non-ObjectId id', encodeCursor({ n: 'a', id: 'nope' })], + // JSON.stringify drops an undefined value, so a writer that doesn't + // normalize a missing displayName to null issues a cursor like this. + ['a missing name', encodeCursor({ id: ALERT_ID })], + ])('rejects a cursor with %s', (_label, cursor) => { + expect(() => parse({ cursor, limit: '2' })).toThrow('invalid cursor'); + }); +}); + +describe('buildAlertsPageFilter', () => { + it('scopes to the team and nothing else when unfiltered', () => { + expect(buildFilter()).toEqual({ team: TEAM_ID }); + }); + + it('matches any requested tag', () => { + expect(buildFilter({ tag: ['prod', 'web'] }).tags).toEqual({ + $in: ['prod', 'web'], + }); + }); + + it('matches documents with no source field when saved_search is requested', () => { + // `source` postdates the first alerts; those documents are saved searches. + expect( + buildFilter({ source: [AlertSource.SAVED_SEARCH, AlertSource.TILE] }) + .source, + ).toEqual({ $in: [AlertSource.SAVED_SEARCH, AlertSource.TILE, null] }); + + expect(buildFilter({ source: AlertSource.TILE }).source).toEqual({ + $in: [AlertSource.TILE], + }); + }); + + it('matches documents with no state field when ok is requested', () => { + // `state` postdates the first alerts; those documents hydrate as OK. + expect( + buildFilter({ state: [AlertState.OK, AlertState.ALERT] }).state, + ).toEqual({ $in: [AlertState.OK, AlertState.ALERT, null] }); + + expect(buildFilter({ state: AlertState.ALERT }).state).toEqual({ + $in: [AlertState.ALERT], + }); + }); + + it('filters on state and createdBy by equality', () => { + const filter = buildFilter({ + state: [AlertState.ALERT], + createdBy: ALERT_ID, + }); + expect(filter.state).toEqual({ $in: [AlertState.ALERT] }); + expect(filter.createdBy).toBe(ALERT_ID); + }); + + it('treats regex metacharacters in search as literals', () => { + expect(buildFilter({ search: 'a.b(c' }).displayName).toEqual({ + $regex: 'a\\.b\\(c', + $options: 'i', + }); + }); + + it('adds no displayName predicate for a blank search', () => { + // A cleared search box sends `search=`, and trimming a whitespace-only one + // lands in the same place. Neither may become a match-everything regex. + expect(buildFilter({ search: '' }).displayName).toBeUndefined(); + expect(buildFilter({ search: ' ' }).displayName).toBeUndefined(); + }); + + it('pages past a named alert with a scoped tie-break', () => { + const filter = buildFilter({ + limit: '2', + cursor: encodeCursor({ n: 'Checkout', id: ALERT_ID }), + }); + + // The _id tie-break is scoped to the cursor's own name so neither branch + // rescans the run of rows sharing it. + expect(filter.displayName).toBeUndefined(); + expect(filter.$or).toEqual([ + { displayName: { $gt: 'Checkout' } }, + { displayName: 'Checkout', _id: { $gt: ALERT_ID } }, + ]); + }); + + it('pages out of the unnamed block into the named alerts', () => { + const filter = buildFilter({ + limit: '2', + cursor: encodeCursor({ n: null, id: ALERT_ID }), + }); + + expect(filter.displayName).toBeUndefined(); + expect(filter.$or).toEqual([ + { displayName: null, _id: { $gt: ALERT_ID } }, + { displayName: { $gte: '' } }, + ]); + }); + + it('keeps the search predicate alongside a named cursor', () => { + const filter = buildFilter({ + limit: '2', + search: 'err', + cursor: encodeCursor({ n: 'Checkout', id: ALERT_ID }), + }); + + expect(filter.displayName).toEqual({ $regex: 'err', $options: 'i' }); + expect(filter.$or).toEqual([ + { displayName: { $gt: 'Checkout' } }, + { displayName: 'Checkout', _id: { $gt: ALERT_ID } }, + ]); + }); + + it('keeps the search predicate alongside a null cursor', () => { + const filter = buildFilter({ + limit: '2', + search: 'err', + cursor: encodeCursor({ n: null, id: ALERT_ID }), + }); + + expect(filter.displayName).toEqual({ $regex: 'err', $options: 'i' }); + expect(filter.$or).toEqual([ + { displayName: null, _id: { $gt: ALERT_ID } }, + { displayName: { $gte: '' } }, + ]); + }); + + it('leaves $or to the cursor when the widened filters are applied', () => { + // The missing-field branches spell "documents with no source/state field" + // as `$in: [..., null]` rather than a second `$or`, which would overwrite + // the cursor's and silently restart the walk. + const filter = buildFilter({ + limit: '2', + source: AlertSource.SAVED_SEARCH, + state: AlertState.OK, + cursor: encodeCursor({ n: null, id: ALERT_ID }), + }); + + expect(filter.source).toEqual({ $in: [AlertSource.SAVED_SEARCH, null] }); + expect(filter.state).toEqual({ $in: [AlertState.OK, null] }); + expect(filter.$or).toEqual([ + { displayName: null, _id: { $gt: ALERT_ID } }, + { displayName: { $gte: '' } }, + ]); + }); +}); diff --git a/packages/api/src/controllers/alerts.ts b/packages/api/src/controllers/alerts.ts index 4cf27c8916..0e8762e48c 100644 --- a/packages/api/src/controllers/alerts.ts +++ b/packages/api/src/controllers/alerts.ts @@ -514,29 +514,42 @@ export const getAlertWithDisplayRefs = async ( }).populate(DISPLAY_REF_POPULATE); }; -export const getAlertsEnhanced = async (teamId: ObjectId) => { - return Alert.find({ team: teamId }).populate<{ - savedSearch: ISavedSearch; - dashboard: IDashboard; - createdBy?: IUser; - silenced?: IAlert['silenced'] & { - by: IUser; - }; - }>(['savedSearch', 'dashboard', 'createdBy', 'silenced.by']); +/** Represents the documents populated and projected by ALERT_PAGE_POPULATE */ +export type AlertPageRefs = { + // `_id` is not in the select below but Mongo returns it anyway + savedSearch: Pick | null; + dashboard: + | (Pick & { + tiles: { id: string; config?: { name?: string } }[]; + }) + | null; + createdBy?: Pick; + silenced?: IAlert['silenced'] & { + by: Pick; + }; }; +/** + * Projections for the internal alerts surfaces (the alerts page and the alert + * detail endpoint), kept to what the response actually renders. + */ +export const ALERT_PAGE_POPULATE = [ + { path: 'savedSearch', select: 'name tags' }, + { + path: 'dashboard', + select: 'name provisioned tags tiles.id tiles.config.name', + }, + { path: 'createdBy', select: 'email name' }, + { path: 'silenced.by', select: 'email' }, +]; + export const getAlertEnhanced = async ( alertId: ObjectId | string, teamId: ObjectId, ) => { - return Alert.findOne({ _id: alertId, team: teamId }).populate<{ - savedSearch: ISavedSearch; - dashboard: IDashboard; - createdBy?: IUser; - silenced?: IAlert['silenced'] & { - by: IUser; - }; - }>(['savedSearch', 'dashboard', 'createdBy', 'silenced.by']); + return Alert.findOne({ _id: alertId, team: teamId }).populate( + ALERT_PAGE_POPULATE, + ); }; export const deleteAlert = async (id: string, teamId: ObjectId) => { diff --git a/packages/api/src/controllers/alertsPage.ts b/packages/api/src/controllers/alertsPage.ts new file mode 100644 index 0000000000..87410fb536 --- /dev/null +++ b/packages/api/src/controllers/alertsPage.ts @@ -0,0 +1,170 @@ +import { MAX_ALERT_DISPLAY_NAME_LENGTH } from '@hyperdx/common-utils/dist/types'; +import { escapeRegExp } from 'lodash'; +import type { FilterQuery } from 'mongoose'; +import { z } from 'zod'; + +import { ALERT_PAGE_POPULATE, type AlertPageRefs } from '@/controllers/alerts'; +import type { ObjectId } from '@/models'; +import Alert, { AlertSource, AlertState, IAlert } from '@/models/alert'; +import { decodeCursor, encodeCursor } from '@/utils/pagination'; +import { objectIdSchema, stringListQueryParam, tagsSchema } from '@/utils/zod'; + +const MAX_PAGE_LIMIT = 500; + +/** The page cursor's payload: the sort key of the last item on the page. */ +const alertsPageCursorSchema = z.object({ + n: z.string().nullable(), + id: objectIdSchema, +}); + +type AlertsPageCursor = z.infer; + +/** Query parameters schema for the list alerts endpoint. */ +export const alertsPageQuerySchema = z + .object({ + /** Number of alerts to return in one page. Omitted means no paging is applied */ + limit: z.coerce.number().int().min(1).max(MAX_PAGE_LIMIT).optional(), + /** The cursor pointing to the last item of the previous page, if there was a previous page */ + cursor: z.string().optional(), + /** A user ID, to filter for alerts created by a specific user */ + createdBy: objectIdSchema.optional(), + /** A list of tags to filter alerts by. Returns alerts that have any of the specified tags */ + tag: stringListQueryParam.pipe(tagsSchema).optional(), + /** A list of alert sources to filter by. Returns alerts that match any of the specified sources */ + source: stringListQueryParam + .pipe(z.array(z.nativeEnum(AlertSource)).optional()) + .optional(), + /** A list of alert states to filter by. Returns alerts that match any of the specified states */ + state: stringListQueryParam + .pipe(z.array(z.nativeEnum(AlertState)).optional()) + .optional(), + /** A search string to filter alerts by their display name. Substring, case-insensitive match */ + search: z.string().trim().max(MAX_ALERT_DISPLAY_NAME_LENGTH).optional(), + }) + .superRefine((query, ctx) => { + if (query.cursor == null) { + return; + } + // A cursor implies pagination, which requires a limit + if (query.limit == null) { + ctx.addIssue({ + code: 'custom', + path: ['cursor'], + message: 'cursor requires limit', + }); + } + if (decodeCursor(query.cursor, alertsPageCursorSchema) == null) { + ctx.addIssue({ + code: 'custom', + path: ['cursor'], + message: 'invalid cursor', + }); + } + }); + +export type AlertsPageParams = z.infer; + +/** + * Mongo filters for a single page. + */ +export function buildAlertsPageFilter( + teamId: ObjectId | string, + params: AlertsPageParams, +): FilterQuery { + const filter: FilterQuery = { team: teamId }; + + if (params.createdBy != null) { + filter.createdBy = params.createdBy; + } + + if (params.tag != null) { + filter.tags = { $in: params.tag }; + } + + if (params.state != null) { + // OK is the default value when state is missing from the document, so we include + // documents without the field as well when filtering for OK state alerts. + filter.state = inOrMissing(params.state, AlertState.OK); + } + + if (params.source != null) { + // SAVED_SEARCH is the default value when source is missing from the document, so we include + // documents without the field as well when filtering for saved-search alerts. + filter.source = inOrMissing(params.source, AlertSource.SAVED_SEARCH); + } + + if (params.search) { + // Case-insensitive substring. + filter.displayName = { + $regex: escapeRegExp(params.search), + $options: 'i', + }; + } + + const cursor = params.cursor + ? decodeCursor(params.cursor, alertsPageCursorSchema) + : null; + if (cursor != null) { + applyCursor(filter, cursor); + } + + return filter; +} + +function inOrMissing(values: T[], schemaDefault: T) { + return { + $in: values.includes(schemaDefault) ? [...values, null] : values, + }; +} + +/** + * Filters for documents strictly after `cursor` in `{displayName: 1, _id: 1}` order. + * + * Null and missing displayNames sort first as one block, so which predicate applies + * depends on whether the page ended inside that block. + */ +function applyCursor( + filter: FilterQuery, + cursor: AlertsPageCursor, +): void { + // When the previous page ended inside the null block of displayNames, + // the next row may be another null displayName or a named one. + if (cursor.n === null) { + filter.$or = [ + { displayName: null, _id: { $gt: cursor.id } }, + { displayName: { $gte: '' } }, + ]; + return; + } + + // Otherwise all remaining displayNames are non-null strings, so we find documents + // with displayNames after the cursor, or displayName equal to the cursor but with + // a higher _id. + filter.$or = [ + { displayName: { $gt: cursor.n } }, + { displayName: cursor.n, _id: { $gt: cursor.id } }, + ]; +} + +export async function getAlertsPage( + teamId: ObjectId, + params: AlertsPageParams, +) { + const { limit } = params; + const query = Alert.find(buildAlertsPageFilter(teamId, params)) + .sort({ displayName: 1, _id: 1 }) + .populate(ALERT_PAGE_POPULATE); + + // One row past the page answers "is there more?" without a second count + // query. Omitting limit keeps the legacy unpaginated response. + const docs = limit == null ? await query : await query.limit(limit + 1); + const hasMore = limit != null && docs.length > limit; + const data = hasMore ? docs.slice(0, limit) : docs; + const last = data.at(-1); + const nextCursor = + hasMore && last != null + ? encodeCursor({ n: last.displayName ?? null, id: last._id.toString() }) + : undefined; + + return { data, hasMore, nextCursor }; +} diff --git a/packages/api/src/mcp/__tests__/listMetricsCursor.test.ts b/packages/api/src/mcp/__tests__/listMetricsCursor.test.ts index 150f677e28..6cb4fbe105 100644 --- a/packages/api/src/mcp/__tests__/listMetricsCursor.test.ts +++ b/packages/api/src/mcp/__tests__/listMetricsCursor.test.ts @@ -13,7 +13,7 @@ describe('listMetrics cursor', () => { it('round-trips a gauge cursor', () => { const payload = { kind: 'gauge' as const, lastName: 'system.cpu.idle' }; const encoded = encodeCursor(payload); - expect(encoded).toMatch(/^[A-Za-z0-9+/]+=*$/); // base64 + expect(encoded).toMatch(/^[A-Za-z0-9_-]+$/); // base64url, unpadded expect(decodeCursor(encoded)).toEqual(payload); }); diff --git a/packages/api/src/mcp/tools/sources/listMetrics.ts b/packages/api/src/mcp/tools/sources/listMetrics.ts index 63714fa32c..5ad4f60dea 100644 --- a/packages/api/src/mcp/tools/sources/listMetrics.ts +++ b/packages/api/src/mcp/tools/sources/listMetrics.ts @@ -13,6 +13,10 @@ import { getSource } from '@/controllers/sources'; import type { ToolRegistrar } from '@/mcp/tools/types'; import { mcpServerError, mcpUserError } from '@/mcp/utils/errors'; import logger from '@/utils/logger'; +import { + decodeCursor as decodeCursorPayload, + encodeCursor as encodeCursorPayload, +} from '@/utils/pagination'; import { DISCOVERABLE_METRIC_KINDS, @@ -32,37 +36,21 @@ const MAX_EXEC_SECONDS = 8; // ─── Cursor ────────────────────────────────────────────────────────────────── -export type ListMetricsCursorPayload = { - kind: DiscoverableMetricKind; - lastName: string; -}; +const cursorPayloadSchema = z.object({ + kind: z.enum(DISCOVERABLE_METRIC_KINDS), + lastName: z.string(), +}); + +export type ListMetricsCursorPayload = z.infer; /** @internal Exported for testing. */ export function encodeCursor(payload: ListMetricsCursorPayload): string { - return Buffer.from(JSON.stringify(payload), 'utf8').toString('base64'); + return encodeCursorPayload(payload); } /** @internal Exported for testing. */ export function decodeCursor(raw: string): ListMetricsCursorPayload | null { - try { - const decoded = Buffer.from(raw, 'base64').toString('utf8'); - const parsed = JSON.parse(decoded); - if ( - typeof parsed === 'object' && - parsed !== null && - typeof parsed.kind === 'string' && - typeof parsed.lastName === 'string' && - (DISCOVERABLE_METRIC_KINDS as readonly string[]).includes(parsed.kind) - ) { - return { - kind: parsed.kind as DiscoverableMetricKind, - lastName: parsed.lastName, - }; - } - } catch { - // fall through - } - return null; + return decodeCursorPayload(raw, cursorPayloadSchema); } // ─── Schema ────────────────────────────────────────────────────────────────── diff --git a/packages/api/src/models/alert.ts b/packages/api/src/models/alert.ts index 62c52eb3df..3e62d19eab 100644 --- a/packages/api/src/models/alert.ts +++ b/packages/api/src/models/alert.ts @@ -302,4 +302,8 @@ const AlertSchema = new Schema( // and sort by _id. Compound so the sort is index-covered (no in-memory sort). AlertSchema.index({ team: 1, _id: 1 }); +// The alerts page: filter on team, sort by displayName with _id as the +// tie-break, and walk that order with a keyset cursor. +AlertSchema.index({ team: 1, displayName: 1, _id: 1 }); + export default mongoose.model('Alert', AlertSchema); 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 2faa49c296..8d81390593 100644 --- a/packages/api/src/routers/api/__tests__/alerts.int.test.ts +++ b/packages/api/src/routers/api/__tests__/alerts.int.test.ts @@ -518,9 +518,7 @@ describe('alerts router', () => { }); }); - // Documents written before the fields existed have neither, and the startup - // backfill has not necessarily run yet. - it('resolves displayName and tags for a document stored without them', async () => { + it('Derives name and tags from the saved search for a document stored without them', async () => { const savedSearch = await SavedSearch.create({ name: 'Legacy search', source: new mongoose.Types.ObjectId(), @@ -546,6 +544,12 @@ describe('alerts router', () => { displayName: 'Legacy search', tags: ['legacy'], }); + + const list = await agent.get('/alerts').expect(200); + expect(list.body.data[0]).toMatchObject({ + displayName: 'Legacy search', + tags: ['legacy'], + }); }); it('preserves scheduleStartAt when omitted in updates and clears when null', async () => { diff --git a/packages/api/src/routers/api/__tests__/alertsPagination.int.test.ts b/packages/api/src/routers/api/__tests__/alertsPagination.int.test.ts new file mode 100644 index 0000000000..b725eeee1b --- /dev/null +++ b/packages/api/src/routers/api/__tests__/alertsPagination.int.test.ts @@ -0,0 +1,414 @@ +import { AlertThresholdType } from '@hyperdx/common-utils/dist/types'; +import mongoose from 'mongoose'; + +import { + getLoggedInAgent, + getServer, + makeTile, + randomMongoId, +} from '@/fixtures'; +import Alert, { AlertSource, AlertState } from '@/models/alert'; +import { SavedSearch } from '@/models/savedSearch'; +import User from '@/models/user'; +import Webhook, { WebhookDocument, WebhookService } from '@/models/webhook'; + +const MOCK_TILES = [makeTile()]; + +describe('alerts list paging and filtering', () => { + const server = getServer(); + let agent: Awaited>['agent']; + let team: Awaited>['team']; + let user: Awaited>['user']; + let webhook: WebhookDocument; + let dashboardId: string; + + beforeAll(async () => { + await server.start(); + }); + + beforeEach(async () => { + const result = await getLoggedInAgent(server); + agent = result.agent; + team = result.team; + user = result.user; + webhook = await Webhook.create({ + name: 'Test Webhook', + service: WebhookService.Slack, + url: 'https://hooks.slack.com/test', + team: team._id, + }); + const dashboard = await agent + .post('/dashboards') + .send({ name: 'Test Dashboard', tiles: MOCK_TILES, tags: ['test'] }) + .expect(200); + dashboardId = dashboard.body.id; + }); + + afterEach(async () => { + await server.clearDBs(); + }); + + afterAll(async () => { + await server.stop(); + }); + + // Written straight to Mongo rather than through POST /alerts: the create + // endpoint derives displayName/tags when they're absent, and these tests need + // documents that genuinely have none. + const createAlert = (overrides: Record = {}) => + Alert.create({ + team: team._id, + channel: { type: 'webhook', webhookId: webhook._id.toString() }, + interval: '15m', + threshold: 8, + thresholdType: AlertThresholdType.ABOVE, + source: AlertSource.TILE, + dashboard: dashboardId, + tileId: MOCK_TILES[0].id, + createdBy: user._id, + ...overrides, + }); + + const listIds = async (query = '') => { + const res = await agent.get(`/alerts${query}`).expect(200); + return res.body.data.map((alert: { _id: string }) => alert._id); + }; + + describe('unpaginated (no params)', () => { + it('returns every alert in name order, with no page metadata', async () => { + await createAlert({ displayName: 'Gamma' }); + await createAlert({ displayName: 'Alpha' }); + await createAlert({ displayName: 'Beta' }); + + const res = await agent.get('/alerts').expect(200); + + expect( + res.body.data.map((a: { displayName: string }) => a.displayName), + ).toEqual(['Alpha', 'Beta', 'Gamma']); + expect(res.body.hasMore).toBe(false); + expect(res.body.nextCursor).toBeUndefined(); + }); + + // Sorting runs on the stored displayName, so alerts without one lead the + // page even though they render under a name derived from the dashboard. + it('sorts alerts with no stored name first, rendering the derived name', async () => { + await createAlert({ displayName: 'Alpha' }); + await createAlert(); + await createAlert({ displayName: null }); + + const res = await agent.get('/alerts').expect(200); + + expect( + res.body.data.map((a: { displayName: string }) => a.displayName), + ).toEqual([ + 'Test Dashboard - Test Chart', + 'Test Dashboard - Test Chart', + 'Alpha', + ]); + expect(res.body.data[0].tags).toEqual(['test']); + }); + + it('still populates the referenced dashboard, saved search and creator', async () => { + const savedSearch = await SavedSearch.create({ + name: 'Checkout errors', + source: new mongoose.Types.ObjectId(), + team: team._id, + tags: ['checkout'], + }); + await createAlert({ displayName: 'A tile alert' }); + await createAlert({ + displayName: 'B search alert', + source: AlertSource.SAVED_SEARCH, + dashboard: undefined, + tileId: undefined, + savedSearch: savedSearch._id, + }); + + const res = await agent.get('/alerts').expect(200); + const [tileAlert, searchAlert] = res.body.data; + + expect(tileAlert.dashboardId).toBe(dashboardId); + expect(tileAlert.dashboard).toMatchObject({ + name: 'Test Dashboard', + tiles: [{ id: MOCK_TILES[0].id }], + }); + // Each row carries only what the client derives a display name from. + // The ids moved to the flat dashboardId / savedSearchId above; the + // parent's own tags and timestamps are not echoed per row. + expect(tileAlert.dashboard._id).toBeUndefined(); + expect(tileAlert.dashboard.tags).toBeUndefined(); + expect(tileAlert.dashboard.updatedAt).toBeUndefined(); + expect(tileAlert.createdBy).toEqual({ + email: user.email, + name: user.name, + }); + expect(searchAlert.savedSearchId).toBe(savedSearch._id.toString()); + expect(searchAlert.savedSearch).toEqual({ name: 'Checkout errors' }); + }); + }); + + describe('paging', () => { + // Unnamed documents sort before every named one, so a walk over this set + // has to cross the null -> string boundary; the duplicate 'Beta' has to + // tie-break on _id. Both spellings of "unnamed" are here: the field + // missing entirely, and an explicit null (which is what the create path + // persists when there's nothing to derive a name from). + const seedMixedNames = async () => { + await createAlert({ displayName: 'Beta' }); + await createAlert({ displayName: 'Alpha' }); + await createAlert(); + await createAlert({ displayName: 'Beta' }); + await createAlert({ displayName: null }); + }; + + const walk = async (limit: number) => { + const ids: string[] = []; + let cursor: string | undefined; + let pages = 0; + + for (;;) { + const query = cursor + ? `?limit=${limit}&cursor=${encodeURIComponent(cursor)}` + : `?limit=${limit}`; + const res = await agent.get(`/alerts${query}`).expect(200); + pages += 1; + + expect(res.body.data.length).toBeLessThanOrEqual(limit); + ids.push(...res.body.data.map((a: { _id: string }) => a._id)); + + if (!res.body.hasMore) { + expect(res.body.nextCursor).toBeUndefined(); + return { ids, pages }; + } + expect(res.body.nextCursor).toEqual(expect.any(String)); + cursor = res.body.nextCursor; + expect(pages).toBeLessThan(20); // guard against a non-advancing cursor + } + }; + + it('walks the full list without gaps or duplicates', async () => { + await seedMixedNames(); + const expected = await listIds(); + + const { ids, pages } = await walk(2); + + expect(ids).toEqual(expected); + expect(new Set(ids).size).toBe(5); + expect(pages).toBe(3); + }); + + it('advances one row at a time', async () => { + await seedMixedNames(); + const expected = await listIds(); + + const { ids } = await walk(1); + + expect(ids).toEqual(expected); + }); + + it('breaks ties between identical names on _id', async () => { + await createAlert({ displayName: 'Same' }); + await createAlert({ displayName: 'Same' }); + await createAlert({ displayName: 'Same' }); + const expected = await listIds(); + + const { ids } = await walk(1); + + expect(ids).toEqual(expected); + expect(ids).toEqual([...ids].sort()); + }); + + it('reports hasMore false on a page that exactly empties the list', async () => { + await createAlert({ displayName: 'Alpha' }); + await createAlert({ displayName: 'Beta' }); + + const res = await agent.get('/alerts?limit=2').expect(200); + + expect(res.body.data).toHaveLength(2); + expect(res.body.hasMore).toBe(false); + expect(res.body.nextCursor).toBeUndefined(); + }); + }); + + describe('filters', () => { + it('filters by createdBy', async () => { + const other = await User.create({ + email: 'other@example.test', + name: 'Other', + team: team._id, + }); + await createAlert({ displayName: 'Mine' }); + const theirs = await createAlert({ + displayName: 'Theirs', + createdBy: other._id, + }); + + expect(await listIds(`?createdBy=${other._id.toString()}`)).toEqual([ + theirs._id.toString(), + ]); + }); + + it('matches any repeated tag', async () => { + const both = await createAlert({ + displayName: 'Both', + tags: ['prod', 'web'], + }); + const one = await createAlert({ displayName: 'One', tags: ['prod'] }); + const other = await createAlert({ displayName: 'Other', tags: ['web'] }); + await createAlert({ displayName: 'Untouched', tags: ['staging'] }); + + expect(await listIds('?tag=prod')).toHaveLength(2); + // Union, not intersection: 'One' and 'Other' each carry one of the two. + expect(await listIds('?tag=prod&tag=web')).toEqual([ + both._id.toString(), + one._id.toString(), + other._id.toString(), + ]); + }); + + it('filters by state', async () => { + const firing = await createAlert({ + displayName: 'Firing', + state: AlertState.ALERT, + }); + await createAlert({ displayName: 'Fine' }); + + expect(await listIds('?state=ALERT')).toEqual([firing._id.toString()]); + expect(await listIds('?state=ALERT&state=OK')).toHaveLength(2); + }); + + it('matches documents with no state field when filtering by OK', async () => { + // The model defaults `state` to OK, so unset it to reproduce a document + // written before the field existed. It renders as OK, so the OK filter + // has to return it. + const legacy = await createAlert({ displayName: 'Legacy' }); + await Alert.updateOne({ _id: legacy._id }, { $unset: { state: 1 } }); + const firing = await createAlert({ + displayName: 'Firing', + state: AlertState.ALERT, + }); + + expect(await listIds('?state=OK')).toEqual([legacy._id.toString()]); + expect(await listIds('?state=ALERT')).toEqual([firing._id.toString()]); + expect(await listIds('?state=OK&state=ALERT')).toHaveLength(2); + }); + + it('matches documents with no source field when filtering by saved_search', async () => { + const savedSearch = await SavedSearch.create({ + name: 'Legacy search', + source: new mongoose.Types.ObjectId(), + team: team._id, + }); + // The model defaults `source` to saved_search, so unset it to reproduce a + // document written before the field existed. + const legacy = await createAlert({ + displayName: 'Legacy', + dashboard: undefined, + tileId: undefined, + savedSearch: savedSearch._id, + }); + await Alert.updateOne({ _id: legacy._id }, { $unset: { source: 1 } }); + const tile = await createAlert({ displayName: 'Tile' }); + + expect(await listIds('?source=saved_search')).toEqual([ + legacy._id.toString(), + ]); + expect(await listIds('?source=tile')).toEqual([tile._id.toString()]); + expect(await listIds('?source=tile&source=saved_search')).toHaveLength(2); + }); + + it('searches display names case-insensitively, treating metacharacters literally', async () => { + const errors = await createAlert({ displayName: 'Checkout ERRORS' }); + await createAlert({ displayName: 'Latency' }); + const literal = await createAlert({ displayName: 'a.b(c' }); + + expect(await listIds('?search=errors')).toEqual([errors._id.toString()]); + expect(await listIds('?search=out%20ERR')).toEqual([ + errors._id.toString(), + ]); + expect(await listIds('?search=a.b(')).toEqual([literal._id.toString()]); + expect(await listIds('?search=a%3Fb(')).toEqual([]); + }); + + // The next two cases pin the residual gap: the row renders the value + // derived from the referenced dashboard, but filters run against what Mongo + // holds, so an alert the startup backfill missed can be read yet not found. + it('cannot match an alert whose tags were never stored', async () => { + const untagged = await createAlert({ displayName: 'Untagged' }); + + const res = await agent.get('/alerts').expect(200); + expect(res.body.data[0]).toMatchObject({ + _id: untagged._id.toString(), + tags: ['test'], + }); + expect(await listIds('?tag=test')).toEqual([]); + }); + + it('cannot match an alert whose display name was never stored', async () => { + const unnamed = await createAlert(); + + const res = await agent.get('/alerts').expect(200); + expect(res.body.data[0]).toMatchObject({ + _id: unnamed._id.toString(), + displayName: 'Test Dashboard - Test Chart', + }); + expect(await listIds('?search=Test%20Dashboard')).toEqual([]); + }); + + it('combines a filter with paging', async () => { + await createAlert({ displayName: 'Alpha', tags: ['prod'] }); + await createAlert({ displayName: 'Beta', tags: ['prod'] }); + await createAlert({ displayName: 'Gamma' }); + + const first = await agent.get('/alerts?limit=1&tag=prod').expect(200); + expect(first.body.data[0].displayName).toBe('Alpha'); + expect(first.body.hasMore).toBe(true); + + const second = await agent + .get( + `/alerts?limit=1&tag=prod&cursor=${encodeURIComponent(first.body.nextCursor)}`, + ) + .expect(200); + expect(second.body.data[0].displayName).toBe('Beta'); + expect(second.body.hasMore).toBe(false); + }); + }); + + describe('rejected requests', () => { + it('rejects an otherwise valid cursor with no limit to continue', async () => { + await createAlert({ displayName: 'Alpha' }); + await createAlert({ displayName: 'Beta' }); + const first = await agent.get('/alerts?limit=1').expect(200); + + await agent + .get(`/alerts?cursor=${encodeURIComponent(first.body.nextCursor)}`) + .expect(400); + }); + + it.each([ + ['a malformed cursor', '/alerts?limit=2&cursor=not-a-cursor'], + [ + 'a cursor whose id is not an ObjectId', + `/alerts?limit=2&cursor=${Buffer.from( + JSON.stringify({ n: 'a', id: 'nope' }), + ).toString('base64url')}`, + ], + ['a non-ObjectId createdBy', '/alerts?createdBy=nope'], + ['limit=0', '/alerts?limit=0'], + ['limit above the cap', '/alerts?limit=501'], + ['an unknown source', '/alerts?source=nope'], + ['an unknown state', '/alerts?state=nope'], + ])('rejects %s', async (_label, path) => { + await createAlert({ displayName: 'Alpha' }); + await agent.get(path).expect(400); + }); + }); + + it('does not leak another team’s alerts', async () => { + await createAlert({ displayName: 'Mine' }); + await createAlert({ displayName: 'Theirs', team: randomMongoId() }); + + expect(await listIds()).toHaveLength(1); + expect(await listIds('?limit=10')).toHaveLength(1); + }); +}); diff --git a/packages/api/src/routers/api/alerts.ts b/packages/api/src/routers/api/alerts.ts index fa8a5174f4..3085e3d764 100644 --- a/packages/api/src/routers/api/alerts.ts +++ b/packages/api/src/routers/api/alerts.ts @@ -23,13 +23,14 @@ import { deleteAlert, getAlertById, getAlertEnhanced, - getAlertsEnhanced, updateAlert, validateAlertInput, } from '@/controllers/alerts'; +import { alertsPageQuerySchema, getAlertsPage } from '@/controllers/alertsPage'; import { AlertSource, getAlertChannels } from '@/models/alert'; import { IAlertHistory } from '@/models/alertHistory'; import { resolveAlertDisplayFields } from '@/utils/alerts'; +import { setBusinessContext } from '@/utils/instrumentation'; import { PreSerialized, sendJson } from '@/utils/serialization'; import { internalAlertSchema, objectIdSchema } from '@/utils/zod'; @@ -79,31 +80,24 @@ const formatAlertResponse = ( ...(alert.dashboard && { dashboardId: alert.dashboard._id, dashboard: { + name: alert.dashboard.name, tiles: alert.dashboard.tiles .filter(tile => tile.id === alert.tileId) .map(tile => ({ id: tile.id, - config: { name: tile.config.name }, + config: { name: tile.config?.name }, })), - ...pick(alert.dashboard, ['_id', 'updatedAt', 'name', 'tags']), }, }), ...(alert.savedSearch && { savedSearchId: alert.savedSearch._id, - savedSearch: pick(alert.savedSearch, [ - '_id', - 'createdAt', - 'name', - 'updatedAt', - 'tags', - ]), + savedSearch: { name: alert.savedSearch.name }, }), // Inline alerts carry their persisted config so edit surfaces can seed // the chart editor and the detail page can render the query — but only on - // the single-alert response. The list endpoint is unpaginated, so - // attaching every alert's full config (raw SQL templates included) would - // bloat every alerts-page load and create a contract that couldn't be - // paginated away later. + // the single-alert response. Attaching every alert's full config (raw SQL + // templates included) would bloat the list; surfaces that need it fetch + // the one alert. ...(includeChartConfig && alert.chartConfig && { chartConfig: alert.chartConfig }), ...pick(alert, [ @@ -130,33 +124,47 @@ const formatAlertResponse = ( }; type AlertsExpRes = express.Response; -router.get('/', async (req, res: AlertsExpRes, next) => { - try { - const teamId = req.user?.team; - if (teamId == null) { - return res.sendStatus(403); - } +router.get( + '/', + processRequest({ query: alertsPageQuerySchema }), + async (req, res: AlertsExpRes, next) => { + try { + const teamId = req.user?.team; + if (teamId == null) { + return res.sendStatus(403); + } - const alerts = await getAlertsEnhanced(teamId); + const params = alertsPageQuerySchema.parse(req.query ?? {}); + const { + data: alerts, + hasMore, + nextCursor, + } = await getAlertsPage(teamId, params); - const historyMap = await getRecentAlertHistoriesBatch( - alerts.map(alert => ({ - alertId: new ObjectId(alert._id), - interval: alert.interval, - })), - 20, - ); - - const data = alerts.map(alert => { - const history = historyMap.get(alert._id.toString()) ?? []; - return formatAlertResponse(alert, history); - }); - - sendJson(res, { data }); - } catch (e) { - next(e); - } -}); + setBusinessContext({ + 'hyperdx.alerts.list.page_size': alerts.length, + 'hyperdx.alerts.list.limit': params.limit, + }); + + const historyMap = await getRecentAlertHistoriesBatch( + alerts.map(alert => ({ + alertId: new ObjectId(alert._id), + interval: alert.interval, + })), + 20, + ); + + const data = alerts.map(alert => { + const history = historyMap.get(alert._id.toString()) ?? []; + return formatAlertResponse(alert, history); + }); + + sendJson(res, { data, hasMore, ...(nextCursor && { nextCursor }) }); + } catch (e) { + next(e); + } + }, +); type AlertExpRes = express.Response; router.get( diff --git a/packages/api/src/routers/api/prometheus.ts b/packages/api/src/routers/api/prometheus.ts index 01a716d329..c0f018a0ec 100644 --- a/packages/api/src/routers/api/prometheus.ts +++ b/packages/api/src/routers/api/prometheus.ts @@ -16,7 +16,7 @@ import { import { getNonNullUserWithTeam } from '@/middleware/auth'; import { getCounter, getHistogram } from '@/utils/instrumentation'; import logger from '@/utils/logger'; -import { objectIdSchema } from '@/utils/zod'; +import { objectIdSchema, stringListQueryParam } from '@/utils/zod'; const router = express.Router(); @@ -832,26 +832,18 @@ const prometheusTimestampSchema = z } }); -/** - * Prometheus's series selector, spelled `match[]` and repeatable. Express's query - * parser drops the brackets, so both `match[]=up` and `match=up` land on a - * `match` key and a repeated one arrives as an array. The selectors themselves - * are never interpreted here — only Prometheus can — so an empty one is treated - * as absent rather than forwarded for upstream to reject. - */ -const prometheusMatchSchema = z - .union([z.string(), z.array(z.string())]) - .transform(v => { - const selectors = (Array.isArray(v) ? v : [v]).filter(m => m !== ''); - return selectors.length ? selectors : undefined; - }); - const labelLookupRequestQuerySchema = z .object({ connectionId: objectIdSchema, start: prometheusTimestampSchema.optional(), end: prometheusTimestampSchema.optional(), - match: prometheusMatchSchema.optional(), + /** + * Prometheus's series selector, spelled `match[]` and repeatable. The + * selectors themselves are never interpreted here — only Prometheus can — + * so an empty one is treated as absent (see `stringListQueryParam`) rather + * than forwarded for upstream to reject. + */ + match: stringListQueryParam.optional(), database: z.string().optional(), table: z.string().optional(), limit: z.preprocess( diff --git a/packages/api/src/utils/__tests__/pagination.test.ts b/packages/api/src/utils/__tests__/pagination.test.ts new file mode 100644 index 0000000000..b95cdb7a31 --- /dev/null +++ b/packages/api/src/utils/__tests__/pagination.test.ts @@ -0,0 +1,51 @@ +import { z } from 'zod'; + +import { decodeCursor, encodeCursor } from '@/utils/pagination'; + +const payloadSchema = z.object({ + n: z.string().nullable(), + id: z.string(), +}); + +describe('cursor encoding', () => { + it('round-trips a payload', () => { + const payload = { n: 'Checkout errors', id: 'abc123' }; + expect(decodeCursor(encodeCursor(payload), payloadSchema)).toEqual(payload); + }); + + it('round-trips null and unicode values', () => { + const payload = { n: null, id: 'µ-naïve/id+1' }; + expect(decodeCursor(encodeCursor(payload), payloadSchema)).toEqual(payload); + }); + + it('emits a URL-safe cursor', () => { + // A cursor travels in a query string, so it must survive unescaped. + const encoded = encodeCursor({ n: 'a+b/c?d&e=f', id: 'x' }); + expect(encoded).toMatch(/^[A-Za-z0-9_-]+$/); + expect(encodeURIComponent(encoded)).toBe(encoded); + }); + + it('decodes standard base64 as well as base64url', () => { + // Cursors issued before the encoding switched are still in flight. + const payload = { n: 'a+b/c', id: 'x' }; + const standard = Buffer.from(JSON.stringify(payload), 'utf8').toString( + 'base64', + ); + expect(decodeCursor(standard, payloadSchema)).toEqual(payload); + }); + + it.each([ + ['non-base64 input', 'not a cursor!'], + ['an empty string', ''], + ['base64 of invalid JSON', Buffer.from('not json').toString('base64url')], + ])('returns null for %s', (_label, raw) => { + expect(decodeCursor(raw, payloadSchema)).toBeNull(); + }); + + it('returns null for a payload the schema rejects', () => { + expect(decodeCursor(encodeCursor({ id: 'x' }), payloadSchema)).toBeNull(); + expect( + decodeCursor(encodeCursor({ n: 1, id: 'x' }), payloadSchema), + ).toBeNull(); + }); +}); diff --git a/packages/api/src/utils/pagination.ts b/packages/api/src/utils/pagination.ts index b8f55bf66b..b5aeb0dc94 100644 --- a/packages/api/src/utils/pagination.ts +++ b/packages/api/src/utils/pagination.ts @@ -62,3 +62,31 @@ export function paginationMeta( } return { total, limit, offset }; } + +/** + * Opaque cursor for keyset pagination: base64url JSON, so a cursor survives a + * query string unescaped. Opaque by convention only — it is neither signed nor + * encrypted, so keep client-visible data in the payload and always validate the + * decoded shape with `decodeCursor`. + */ +export function encodeCursor(payload: unknown): string { + return Buffer.from(JSON.stringify(payload), 'utf8').toString('base64url'); +} + +/** + * `null` for anything that isn't a cursor the caller's schema recognizes — + * truncated, tampered with, or left over from an earlier payload shape. Callers + * turn that into a 400 (or start the walk over) rather than leaking a decode + * error. Decoding accepts standard base64 as well as base64url, so cursors + * issued before this encoding switched remain readable. + */ +export function decodeCursor(raw: string, schema: z.ZodType): T | null { + let parsed: unknown; + try { + parsed = JSON.parse(Buffer.from(raw, 'base64url').toString('utf8')); + } catch { + return null; + } + const result = schema.safeParse(parsed); + return result.success ? result.data : null; +} diff --git a/packages/api/src/utils/zod.ts b/packages/api/src/utils/zod.ts index 583f2d1489..78622d9c1a 100644 --- a/packages/api/src/utils/zod.ts +++ b/packages/api/src/utils/zod.ts @@ -43,6 +43,20 @@ export const objectIdSchema = z.string().refine(val => { return Types.ObjectId.isValid(val); }, 'Invalid ObjectId'); +/** + * A repeatable query param. Express's parser yields a bare string for a single + * occurrence and an array for several, and `?p[]=x` lands on the same key as + * `?p=x`. Normalizes both to an array, dropping empty values — `?p=` means + * "absent", not "match the empty string" — and collapsing to `undefined` when + * nothing is left, so `.optional()` covers both spellings of "not supplied". + */ +export const stringListQueryParam = z + .union([z.string(), z.array(z.string())]) + .transform(v => { + const values = (Array.isArray(v) ? v : [v]).filter(s => s !== ''); + return values.length ? values : undefined; + }); + // ================================ // Charts & Dashboards (old format) // ================================ diff --git a/packages/app/src/components/alerts/__tests__/AlertDetailProperties.test.tsx b/packages/app/src/components/alerts/__tests__/AlertDetailProperties.test.tsx index 803bc90c77..b449524368 100644 --- a/packages/app/src/components/alerts/__tests__/AlertDetailProperties.test.tsx +++ b/packages/app/src/components/alerts/__tests__/AlertDetailProperties.test.tsx @@ -41,7 +41,8 @@ const baseAlert = { createdAt: '2026-01-01T00:00:00.000Z', updatedAt: '2026-02-01T00:00:00.000Z', history: [], -} as unknown as AlertsPageItem; + displayName: 'Alert', +} satisfies AlertsPageItem; describe('AlertDetailProperties', () => { it('renders all persisted metadata fields when set', () => { @@ -61,14 +62,8 @@ describe('AlertDetailProperties', () => { until: '2099-01-01T00:00:00.000Z', }, tags: ['prod', 'payments'], - savedSearch: { - _id: 'saved-search-id', - name: 'My Search', - createdAt: '2026-01-01T00:00:00.000Z', - updatedAt: '2026-01-01T00:00:00.000Z', - tags: ['prod', 'payments'], - }, - } as AlertsPageItem + savedSearch: { name: 'My Search' }, + } satisfies AlertsPageItem } />, ); @@ -98,21 +93,15 @@ describe('AlertDetailProperties', () => { expect(screen.getByText(/Team Slack/)).toBeInTheDocument(); }); - it("renders the alert's own tags, not the saved search's", () => { + it("renders the alert's own tags", () => { renderWithMantine( , ); diff --git a/packages/common-utils/src/types.ts b/packages/common-utils/src/types.ts index 6200ea1851..f8bc6b2316 100644 --- a/packages/common-utils/src/types.ts +++ b/packages/common-utils/src/types.ts @@ -2697,8 +2697,8 @@ export const AlertsPageItemSchema = z.object({ // tile. See isTileAlertUnaddressable. unaddressableTile: z.boolean().optional(), // Inline alerts: the persisted chart config. Only present on the - // single-alert (detail) response — the unpaginated list omits it so every - // alerts-page load doesn't carry every alert's full query definition. + // single-alert (detail) response — the list omits it so a page doesn't carry + // every alert's full query definition. chartConfig: AlertChartConfigSchema.optional(), groupBy: z.string().optional(), name: z.string().nullish(), @@ -2711,10 +2711,7 @@ export const AlertsPageItemSchema = z.object({ history: z.array(AlertHistorySchema), dashboard: z .object({ - _id: z.string(), name: z.string(), - updatedAt: z.string(), - tags: z.array(z.string()), tiles: z.array( z.object({ id: z.string(), @@ -2725,11 +2722,7 @@ export const AlertsPageItemSchema = z.object({ .optional(), savedSearch: z .object({ - _id: z.string(), - createdAt: z.string(), name: z.string(), - updatedAt: z.string(), - tags: z.array(z.string()), }) .optional(), createdBy: z @@ -2753,6 +2746,14 @@ export type AlertsPageItem = z.infer; export const AlertsApiResponseSchema = z.object({ data: z.array(AlertsPageItemSchema), + /** True when more alerts match the request beyond the returned page. */ + hasMore: z.boolean(), + /** + * Keyset cursor for the next page: pass it back unchanged as `cursor` + * (alongside `limit`) to continue get the next page. Absent on the last + * page and on unpaginated responses. + */ + nextCursor: z.string().optional(), }); export type AlertsApiResponse = z.infer;