diff --git a/docs/openapi/monitoring-api.json b/docs/openapi/monitoring-api.json index 41903c1..02f8524 100644 --- a/docs/openapi/monitoring-api.json +++ b/docs/openapi/monitoring-api.json @@ -6412,7 +6412,9 @@ "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] } }, @@ -23307,11 +23309,13 @@ }, "managedBy": { "type": "string", - "description": "Who manages this monitor: DASHBOARD or CLI", + "description": "Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API. Use the value matching your surface so audit logs, drift detection, and analytics attribute correctly.", "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "environmentId": { @@ -27194,11 +27198,13 @@ }, "managedBy": { "type": "string", - "description": "Management source: DASHBOARD or CLI", + "description": "Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API", "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "createdAt": { @@ -33345,12 +33351,14 @@ }, "managedBy": { "type": "string", - "description": "New management source; null preserves current", + "description": "New ownership source: DASHBOARD, CLI, TERRAFORM, MCP, or API; null preserves current value", "nullable": true, "enum": [ "DASHBOARD", "CLI", - "TERRAFORM" + "TERRAFORM", + "MCP", + "API" ] }, "environmentId": { diff --git a/package.json b/package.json index 854bae6..294fbac 100644 --- a/package.json +++ b/package.json @@ -47,7 +47,84 @@ "@oclif/plugin-help", "@oclif/plugin-not-found" ], - "topicSeparator": " " + "topicSeparator": " ", + "topics": { + "alert-channels": { + "description": "Manage alert channels (slack, webhook, email, pagerduty, opsgenie, teams, discord)" + }, + "api-keys": { + "description": "Manage API keys for the DevHelm public API" + }, + "auth": { + "description": "Manage CLI authentication, contexts, and current identity" + }, + "auth:context": { + "description": "Switch between saved CLI contexts (workspace + token pairs)" + }, + "data": { + "description": "Inspect runtime data — service catalog status, monitor results, and uptime windows" + }, + "data:services": { + "description": "Look up the current status and metadata of upstream services" + }, + "dependencies": { + "description": "Track upstream service dependencies (Stripe, GitHub, etc.) for incident correlation" + }, + "environments": { + "description": "Manage logical environments (production, staging, …) used to scope monitors and policies" + }, + "forensics": { + "description": "Inspect detection forensics: rule evaluations, state transitions, policy snapshots, check traces" + }, + "incidents": { + "description": "Create, inspect, and resolve incidents" + }, + "monitors": { + "description": "Manage HTTP, TCP, DNS, ICMP, MCP, and heartbeat monitors" + }, + "monitors:versions": { + "description": "Inspect historical monitor configuration versions" + }, + "notification-policies": { + "description": "Manage escalation policies that route incidents to alert channels" + }, + "resource-groups": { + "description": "Manage resource groups bundling monitors and services for shared health rollups" + }, + "secrets": { + "description": "Manage workspace secrets used in monitor configurations and headers" + }, + "skills": { + "description": "Generate and install Cursor / Claude agent skill packages for DevHelm" + }, + "state": { + "description": "Manage the local deploy state file (.devhelm/state.json)" + }, + "status-pages": { + "description": "Manage public status pages with components, branding, custom domains, and incidents" + }, + "status-pages:components": { + "description": "Manage components on a status page (monitor, group, or static rows)" + }, + "status-pages:domains": { + "description": "Manage custom domains attached to a status page" + }, + "status-pages:groups": { + "description": "Manage component groups that organise rows on a status page" + }, + "status-pages:incidents": { + "description": "Manage status-page-only incidents (separate from monitor-driven incidents)" + }, + "status-pages:subscribers": { + "description": "Manage email and webhook subscribers for a status page" + }, + "tags": { + "description": "Manage tags applied to monitors for filtering, grouping, and routing" + }, + "webhooks": { + "description": "Manage outbound webhook subscriptions for monitor and incident lifecycle events" + } + } }, "engines": { "node": ">=18.0.0" diff --git a/skills/devhelm-configure/references/_generated/monitors.fields.md b/skills/devhelm-configure/references/_generated/monitors.fields.md index 41daeb5..926e687 100644 --- a/skills/devhelm-configure/references/_generated/monitors.fields.md +++ b/skills/devhelm-configure/references/_generated/monitors.fields.md @@ -13,7 +13,7 @@ | `frequencySeconds` | integer (int32) | | ✓ | Check frequency in seconds (30–86400); null defaults to plan minimum (60s on most paid plans) | | `enabled` | boolean | | ✓ | Whether the monitor is active (default: true) | | `regions` | string[] | | ✓ | Probe regions to run checks from, e.g. us-east, eu-west | -| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" | ✓ | | Who manages this monitor: DASHBOARD or CLI | +| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" \| "MCP" \| "API" | ✓ | | Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API. Use the value matching your surface so audit logs, drift detection, and analytics attribute correctly. | | `environmentId` | string (uuid) | | ✓ | Environment to associate with this monitor | | `assertions` | CreateAssertionRequest[] | | ✓ | Assertions to evaluate against each check result | | `auth` | any | | ✓ | | @@ -30,7 +30,7 @@ | `frequencySeconds` | integer (int32) | | ✓ | New check frequency in seconds (30–86400); null preserves current | | `enabled` | boolean | | ✓ | Enable or disable the monitor; null preserves current | | `regions` | string[] | | ✓ | New probe regions; null preserves current | -| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" | | ✓ | New management source; null preserves current | +| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" \| "MCP" \| "API" | | ✓ | New ownership source: DASHBOARD, CLI, TERRAFORM, MCP, or API; null preserves current value | | `environmentId` | string (uuid) | | ✓ | New environment ID; null preserves current (use clearEnvironmentId to unset) | | `clearEnvironmentId` | boolean | | ✓ | Set to true to remove the environment association | | `assertions` | CreateAssertionRequest[] | | ✓ | Replace all assertions; null preserves current | @@ -52,7 +52,7 @@ | `frequencySeconds` | integer (int32) | ✓ | | Check frequency in seconds (30–86400) | | `enabled` | boolean | ✓ | | Whether the monitor is active | | `regions` | string[] | ✓ | | Probe regions where checks are executed | -| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" | ✓ | | Management source: DASHBOARD or CLI | +| `managedBy` | "DASHBOARD" \| "CLI" \| "TERRAFORM" \| "MCP" \| "API" | ✓ | | Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API | | `createdAt` | string (date-time) | ✓ | | Timestamp when the monitor was created | | `updatedAt` | string (date-time) | ✓ | | Timestamp when the monitor was last updated | | `assertions` | MonitorAssertionDto[] | | ✓ | Assertions evaluated against each check result; null on list responses | diff --git a/src/commands/deploy/index.ts b/src/commands/deploy/index.ts index a04ce2b..8f88e74 100644 --- a/src/commands/deploy/index.ts +++ b/src/commands/deploy/index.ts @@ -17,6 +17,7 @@ export default class Deploy extends Command { '<%= config.bin %> deploy --yes', '<%= config.bin %> deploy -f monitors.yml', '<%= config.bin %> deploy --prune --yes', + '<%= config.bin %> deploy --prune-org-cli --yes', '<%= config.bin %> deploy --prune-all --yes', '<%= config.bin %> deploy --dry-run', '<%= config.bin %> deploy --dry-run --detailed-exitcode', @@ -36,11 +37,21 @@ export default class Deploy extends Command { default: false, }), prune: Flags.boolean({ - description: 'Delete CLI-managed resources not present in config', + description: + 'Delete resources tracked in this config\'s .devhelm/state.json that are absent from YAML. ' + + 'Safe in multi-config orgs — does NOT touch CLI-managed resources from other configs.', + default: false, + }), + 'prune-org-cli': Flags.boolean({ + description: + 'Also delete CLI-managed resources elsewhere in the org that are absent from YAML. ' + + 'Use with caution in shared workspaces; widens --prune to the legacy org-wide CLI scope.', default: false, }), 'prune-all': Flags.boolean({ - description: 'Delete ALL resources not in config, including those not managed by the CLI (use with caution)', + description: + 'Delete ALL resources not in YAML, including dashboard- and Terraform-managed ones. ' + + 'The most destructive option; intended for single-tenant workspaces or scripted teardowns.', default: false, }), 'dry-run': Flags.boolean({ @@ -155,7 +166,16 @@ export default class Deploy extends Command { const currentChildren = await prefetchChildSnapshots(config, refs, client) const changeset = await diff( config, refs, - {prune: flags.prune || flags['prune-all'], pruneAll: flags['prune-all']}, + { + // `--prune` is the safe default (state-scoped). The wider scopes + // imply `--prune` so users don't have to combine flags to enable + // the broader behaviour — `--prune-org-cli` deletes state-tracked + // resources AND legacy CLI-managed orphans, and `--prune-all` + // also picks up dashboard- and Terraform-managed resources. + prune: flags.prune || flags['prune-org-cli'] || flags['prune-all'], + pruneOrgCli: flags['prune-org-cli'] || flags['prune-all'], + pruneAll: flags['prune-all'], + }, currentState, currentChildren, ) diff --git a/src/commands/monitors/set-channels.ts b/src/commands/monitors/set-channels.ts new file mode 100644 index 0000000..6ee5c80 --- /dev/null +++ b/src/commands/monitors/set-channels.ts @@ -0,0 +1,49 @@ +import {Command, Flags} from '@oclif/core' +import {globalFlags, buildClient} from '../../lib/base-command.js' +import {parseAlertChannelsFlag, setAlertChannels} from '../../lib/monitor-alert-channels.js' +import {uuidArg} from '../../lib/validators.js' + +/** + * Replace the alert channel set linked to a monitor. + * + * Standalone subcommand for the common workflow of attaching channels + * to an existing monitor without re-running `monitors update`. Pass + * `--channel-ids ""` (empty) to clear all channels — matching the + * semantics of the underlying `PUT /api/v1/monitors/{id}/alert-channels` + * which always replaces the full list. + */ +export default class MonitorsSetChannels extends Command { + static description = 'Replace the alert channel set linked to a monitor' + + static examples = [ + '<%= config.bin %> monitors set-channels --channel-ids ch-1,ch-2', + '<%= config.bin %> monitors set-channels --channel-ids ""', + ] + + static args = { + id: uuidArg({description: 'monitor id', required: true}), + } + + static flags = { + ...globalFlags, + 'channel-ids': Flags.string({ + description: + 'Comma-separated alert channel IDs (replaces current list; pass empty string to clear all channels)', + required: true, + }), + } + + async run() { + const {args, flags} = await this.parse(MonitorsSetChannels) + const client = buildClient(flags) + const ids = parseAlertChannelsFlag(flags['channel-ids']) ?? [] + await setAlertChannels(args.id as string, ids, client) + if (ids.length === 0) { + this.log(`Cleared all alert channels from monitor '${args.id}'.`) + } else { + this.log( + `Linked ${ids.length} alert channel(s) to monitor '${args.id}': ${ids.join(', ')}`, + ) + } + } +} diff --git a/src/lib/api-zod.generated.ts b/src/lib/api-zod.generated.ts index c26bc1f..f427af5 100644 --- a/src/lib/api-zod.generated.ts +++ b/src/lib/api-zod.generated.ts @@ -672,7 +672,7 @@ const CreateMonitorRequest = z frequencySeconds: z.number().int().nullish(), enabled: z.boolean().nullish(), regions: z.array(z.string()).nullish(), - managedBy: z.enum(["DASHBOARD", "CLI", "TERRAFORM"]), + managedBy: z.enum(["DASHBOARD", "CLI", "TERRAFORM", "MCP", "API"]), environmentId: z.string().uuid().nullish(), assertions: z.array(CreateAssertionRequest).nullish(), auth: MonitorAuthConfig.nullish(), @@ -697,7 +697,9 @@ const UpdateMonitorRequest = z frequencySeconds: z.number().int().nullable(), enabled: z.boolean().nullable(), regions: z.array(z.string()).nullable(), - managedBy: z.enum(["DASHBOARD", "CLI", "TERRAFORM"]).nullable(), + managedBy: z + .enum(["DASHBOARD", "CLI", "TERRAFORM", "MCP", "API"]) + .nullable(), environmentId: z.string().uuid().nullable(), clearEnvironmentId: z.boolean().nullable(), assertions: z.array(CreateAssertionRequest).nullable(), @@ -2219,7 +2221,7 @@ const MonitorDto = z frequencySeconds: z.number().int(), enabled: z.boolean(), regions: z.array(z.string()), - managedBy: z.enum(["DASHBOARD", "CLI", "TERRAFORM"]), + managedBy: z.enum(["DASHBOARD", "CLI", "TERRAFORM", "MCP", "API"]), createdAt: z.string().datetime({ offset: true }), updatedAt: z.string().datetime({ offset: true }), assertions: z.array(MonitorAssertionDto).nullish(), diff --git a/src/lib/api.generated.ts b/src/lib/api.generated.ts index e3b9275..2f2f685 100644 --- a/src/lib/api.generated.ts +++ b/src/lib/api.generated.ts @@ -3188,10 +3188,10 @@ export interface components { /** @description Probe regions to run checks from, e.g. us-east, eu-west */ regions?: string[] | null; /** - * @description Who manages this monitor: DASHBOARD or CLI + * @description Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API. Use the value matching your surface so audit logs, drift detection, and analytics attribute correctly. * @enum {string} */ - managedBy: "DASHBOARD" | "CLI" | "TERRAFORM"; + managedBy: "DASHBOARD" | "CLI" | "TERRAFORM" | "MCP" | "API"; /** * Format: uuid * @description Environment to associate with this monitor @@ -4834,10 +4834,10 @@ export interface components { /** @description Probe regions where checks are executed */ regions: string[]; /** - * @description Management source: DASHBOARD or CLI + * @description Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API * @enum {string} */ - managedBy: "DASHBOARD" | "CLI" | "TERRAFORM"; + managedBy: "DASHBOARD" | "CLI" | "TERRAFORM" | "MCP" | "API"; /** * Format: date-time * @description Timestamp when the monitor was created @@ -7181,10 +7181,10 @@ export interface components { /** @description New probe regions; null preserves current */ regions?: string[] | null; /** - * @description New management source; null preserves current + * @description New ownership source: DASHBOARD, CLI, TERRAFORM, MCP, or API; null preserves current value * @enum {string|null} */ - managedBy?: "DASHBOARD" | "CLI" | "TERRAFORM" | null; + managedBy?: "DASHBOARD" | "CLI" | "TERRAFORM" | "MCP" | "API" | null; /** * Format: uuid * @description New environment ID; null preserves current (use clearEnvironmentId to unset) @@ -12790,7 +12790,7 @@ export interface operations { /** @description Filter by monitor type */ type?: "HTTP" | "DNS" | "MCP_SERVER" | "TCP" | "ICMP" | "HEARTBEAT"; /** @description Filter by managed-by source */ - managedBy?: "DASHBOARD" | "CLI" | "TERRAFORM"; + managedBy?: "DASHBOARD" | "CLI" | "TERRAFORM" | "MCP" | "API"; /** @description Filter by tag names, comma-separated (e.g. prod,critical) */ tags?: string; /** @description Case-insensitive name search */ diff --git a/src/lib/crud-commands.ts b/src/lib/crud-commands.ts index 2f5cb27..00e2b07 100644 --- a/src/lib/crud-commands.ts +++ b/src/lib/crud-commands.ts @@ -80,6 +80,18 @@ export interface CreatableResource extends ResourceConfig { * `responseSchema` for the common case where create echoes the entity. */ createResponseSchema?: ZodType + /** + * Optional post-create hook for side-effecting follow-ups that need + * the freshly-created entity's id (alert-channel attachments, + * assertions, etc.). Throw to abort — the hook is responsible for + * any rollback (e.g. DELETE'ing the half-created entity) since the + * factory has no idea which calls are safe to undo. + */ + afterCreate?: ( + created: C, + rawFlags: Record, + ctx: AfterHookContext, + ) => Promise } /** @@ -94,6 +106,35 @@ export interface UpdatableResource extends ResourceConfig { bodyBuilder: (flags: Record) => object updateBodyBuilder?: (flags: Record) => object updateRequestSchema: ZodType + /** Optional post-update hook — see {@link CreatableResource.afterCreate}. */ + afterUpdate?: ( + updated: T, + rawFlags: Record, + ctx: AfterHookContext & {id: string}, + ) => Promise +} + +/** + * Context passed to `afterCreate` / `afterUpdate` hooks. The `command` + * is the live oclif command instance, so hooks can use `command.log` / + * `command.warn` for user-visible output without re-importing oclif. + */ +export interface AfterHookContext { + client: ApiClient + command: Command + apiPath: string +} + +/** + * Pick "a" or "an" based on the next word's first sound. Used for the + * autogenerated CRUD command descriptions (`Get a monitor by id`, + * `Delete an alert channel`) so resource names that start with a vowel + * read naturally. Heuristic on the first letter only — covers every + * resource name we ship today without a full pronunciation table. + */ +export function aOrAn(word: string): 'a' | 'an' { + const first = word.trim().charAt(0).toLowerCase() + return 'aeiou'.includes(first) ? 'an' : 'a' } export function createListCommand(config: ResourceConfig) { @@ -132,7 +173,7 @@ export function createGetCommand(config: ResourceConfig) { const idLabel = config.idField ?? 'id' const getResponseSchema = (config.getResponseSchema ?? config.responseSchema) as ZodType class GetCmd extends Command { - static description = `Get a ${config.name} by ${idLabel}` + static description = `Get ${aOrAn(config.name)} ${config.name} by ${idLabel}` static examples = [`<%= config.bin %> ${config.plural} get <${idLabel}>`] static args = {[idLabel]: idArg(config)} static flags = {...globalFlags} @@ -165,6 +206,9 @@ export function createCreateCommand(config: CreatableResource) { const built = config.bodyBuilder(raw) const body = parseSchema(config.createRequestSchema, built, `${config.name}.create body invalid`) const value = await apiPostSingle(client, config.apiPath, createResponseSchema, body) + if (config.afterCreate) { + await config.afterCreate(value, raw, {client, command: this, apiPath: config.apiPath}) + } display(this, value, flags.output) } } @@ -177,7 +221,7 @@ export function createUpdateCommand(config: UpdatableResource) { const resourceFlags = config.updateFlags ?? config.createFlags ?? {} const builder = config.updateBodyBuilder ?? config.bodyBuilder class UpdateCmd extends Command { - static description = `Update a ${config.name}` + static description = `Update ${aOrAn(config.name)} ${config.name}` static examples = [`<%= config.bin %> ${config.plural} update <${idLabel}>`] static args = {[idLabel]: idArg(config)} static flags = {...globalFlags, ...resourceFlags} @@ -185,12 +229,15 @@ export function createUpdateCommand(config: UpdatableResource) { async run() { const {args, flags} = await this.parse(UpdateCmd) const client = buildClient(flags) - const id = args[idLabel] + const id = args[idLabel] as string const raw = extractResourceFlags(flags, Object.keys(resourceFlags)) const built = builder(raw) const body = parseSchema(config.updateRequestSchema, built, `${config.name}.update body invalid`) const path = `${config.apiPath}/${id}` const value = await apiPutSingle(client, path, config.responseSchema, body) + if (config.afterUpdate) { + await config.afterUpdate(value, raw, {client, command: this, apiPath: config.apiPath, id}) + } display(this, value, flags.output) } } @@ -201,7 +248,7 @@ export function createUpdateCommand(config: UpdatableResource) { export function createDeleteCommand(config: ResourceConfig) { const idLabel = config.idField ?? 'id' class DeleteCmd extends Command { - static description = `Delete a ${config.name}` + static description = `Delete ${aOrAn(config.name)} ${config.name}` static examples = [ `<%= config.bin %> ${config.plural} delete <${idLabel}>`, `<%= config.bin %> ${config.plural} delete <${idLabel}> --yes`, diff --git a/src/lib/descriptions.generated.ts b/src/lib/descriptions.generated.ts index 890378a..bee76e0 100644 --- a/src/lib/descriptions.generated.ts +++ b/src/lib/descriptions.generated.ts @@ -9,7 +9,7 @@ export const fieldDescriptions: Record> = "frequencySeconds": "Check frequency in seconds (30–86400); null defaults to plan minimum (60s on most paid plans)", "enabled": "Whether the monitor is active (default: true)", "regions": "Probe regions to run checks from, e.g. us-east, eu-west", - "managedBy": "Who manages this monitor: DASHBOARD or CLI", + "managedBy": "Source that created/owns this monitor: DASHBOARD, CLI, TERRAFORM, MCP, or API. Use the value matching your surface so audit logs, drift detection, and analytics attribute correctly.", "environmentId": "Environment to associate with this monitor", "assertions": "Assertions to evaluate against each check result", "alertChannelIds": "Alert channels to notify when this monitor triggers" @@ -19,7 +19,7 @@ export const fieldDescriptions: Record> = "frequencySeconds": "New check frequency in seconds (30–86400); null preserves current", "enabled": "Enable or disable the monitor; null preserves current", "regions": "New probe regions; null preserves current", - "managedBy": "New management source; null preserves current", + "managedBy": "New ownership source: DASHBOARD, CLI, TERRAFORM, MCP, or API; null preserves current value", "environmentId": "New environment ID; null preserves current (use clearEnvironmentId to unset)", "clearEnvironmentId": "Set to true to remove the environment association", "assertions": "Replace all assertions; null preserves current", diff --git a/src/lib/monitor-alert-channels.ts b/src/lib/monitor-alert-channels.ts new file mode 100644 index 0000000..d410b5b --- /dev/null +++ b/src/lib/monitor-alert-channels.ts @@ -0,0 +1,58 @@ +/** + * Helpers for the `--alert-channels` flag on `devhelm monitors + * create / update` and the `monitors set-channels` subcommand + * (DevEx P1.Bug3). Wraps `PUT /api/v1/monitors/{id}/alert-channels`, + * which replaces the linked channel set. + * + * Parsing is intentionally lenient: leading/trailing whitespace and + * commas are stripped, and the empty string clears all channels (for + * `update` and `set-channels`). On `create`, an empty list is treated + * as "no channels requested" and the call is skipped. + */ +import {apiPut, apiDelete, type ApiClient} from './api-client.js' + +export function parseAlertChannelsFlag(raw: string | undefined): string[] | undefined { + if (raw === undefined) return undefined + return raw.split(',').map((s) => s.trim()).filter(Boolean) +} + +export async function setAlertChannels( + monitorId: string, + channelIds: readonly string[], + client: ApiClient, + monitorPath: string = '/api/v1/monitors', +): Promise { + await apiPut(client, `${monitorPath}/${monitorId}/alert-channels`, { + channelIds: [...channelIds], + }) +} + +/** + * Attach channels right after `create` succeeded. On failure, delete the + * monitor so the operator isn't left with a half-configured resource. + * Mirrors the rollback story in `applyAssertions` so the two + * post-create hooks behave the same way under failure. + */ +export async function attachAlertChannelsOrRollback( + monitorId: string, + channelIds: readonly string[], + client: ApiClient, + monitorPath: string = '/api/v1/monitors', +): Promise { + if (channelIds.length === 0) return + try { + await setAlertChannels(monitorId, channelIds, client, monitorPath) + } catch (err) { + let cleanup = true + try { + await apiDelete(client, `${monitorPath}/${monitorId}`) + } catch { + cleanup = false + } + const original = err instanceof Error ? err.message : String(err) + const tail = cleanup + ? 'Monitor was rolled back.' + : `Monitor ${monitorId} could NOT be rolled back automatically — delete it manually if it still exists.` + throw new Error(`Failed to attach --alert-channels: ${original}. ${tail}`) + } +} diff --git a/src/lib/monitor-assertions.ts b/src/lib/monitor-assertions.ts new file mode 100644 index 0000000..d8ff948 --- /dev/null +++ b/src/lib/monitor-assertions.ts @@ -0,0 +1,173 @@ +/** + * Helpers for the `--assertion` flag on `devhelm monitors create` + * (DevEx P1.Bug2). Two input forms are accepted: + * + * 1. JSON form — full passthrough so any of the ~40 assertion types in + * the spec can be expressed: + * --assertion '{"severity":"fail","config":{"type":"status_code", + * "expected":"200","operator":"equals"}}' + * + * 2. Shorthand DSL — the three most common assertions, written + * operator-style. Implemented as a tiny ad-hoc parser (no + * tokenizer, no schema) because the surface is intentionally + * narrow. Anything more elaborate should use the JSON form. + * status_code=200 → fail / status_code equals 200 + * response_time<5000 → warn / response_time thresholdMs 5000 + * ssl_expiry>=14 → warn / ssl_expiry minDaysRemaining 14 + * + * After the monitor is created the CLI POSTs each parsed assertion to + * `/api/v1/monitors/{id}/assertions`. If any POST fails the monitor is + * deleted to avoid a half-configured state — see `applyAssertions`. + */ +import type {ApiClient} from './api-client.js' +import {apiPost, apiDelete} from './api-client.js' + +export interface ParsedAssertion { + severity: 'fail' | 'warn' + config: Record +} + +const DSL_RE = /^([a-z_]+)\s*(>=|<=|<|>|=)\s*(.+?)\s*$/i + +export function parseAssertionFlag(raw: string): ParsedAssertion { + const trimmed = raw.trim() + if (trimmed.length === 0) { + throw new Error('--assertion: empty value') + } + // `{` and `[` both look like JSON to the user — route them through the + // JSON parser so the error message is structural ("must be an object") + // instead of the generic "cannot parse" from the DSL fallback. + if (trimmed.startsWith('{') || trimmed.startsWith('[')) { + return parseJsonForm(trimmed) + } + return parseDslForm(trimmed) +} + +function parseJsonForm(raw: string): ParsedAssertion { + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch (err) { + const msg = err instanceof Error ? err.message : String(err) + throw new Error(`--assertion: invalid JSON — ${msg}`) + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new Error('--assertion JSON must be an object with {severity, config}') + } + const obj = parsed as Record + const severity = obj.severity ?? 'fail' + if (severity !== 'fail' && severity !== 'warn') { + throw new Error(`--assertion JSON: severity must be "fail" or "warn", got ${JSON.stringify(severity)}`) + } + if (!obj.config || typeof obj.config !== 'object' || Array.isArray(obj.config)) { + throw new Error('--assertion JSON: missing or invalid `config` object') + } + const config = obj.config as Record + if (typeof config.type !== 'string' || config.type.length === 0) { + throw new Error('--assertion JSON: config.type is required (e.g. "status_code", "response_time")') + } + return {severity, config} +} + +function parseDslForm(raw: string): ParsedAssertion { + const match = raw.match(DSL_RE) + if (!match) { + throw new Error( + `--assertion: cannot parse "${raw}" — expected JSON object or DSL like ` + + `"status_code=200", "response_time<5000", "ssl_expiry>=14"`, + ) + } + const [, type, op, value] = match + switch (type) { + case 'status_code': + if (op !== '=') { + throw new Error(`--assertion status_code only supports "=" (got "${op}")`) + } + return { + severity: 'fail', + config: {type: 'status_code', expected: value, operator: 'equals'}, + } + case 'response_time': { + if (op !== '<') { + throw new Error(`--assertion response_time only supports "<" (got "${op}")`) + } + const ms = Number(value) + if (!Number.isFinite(ms) || ms <= 0 || !Number.isInteger(ms)) { + throw new Error( + `--assertion response_time threshold must be a positive integer (got "${value}")`, + ) + } + return {severity: 'warn', config: {type: 'response_time', thresholdMs: ms}} + } + case 'ssl_expiry': { + if (op !== '>=') { + throw new Error(`--assertion ssl_expiry only supports ">=" (got "${op}")`) + } + const days = Number(value) + if (!Number.isFinite(days) || days <= 0 || !Number.isInteger(days)) { + throw new Error( + `--assertion ssl_expiry days must be a positive integer (got "${value}")`, + ) + } + return { + severity: 'warn', + config: {type: 'ssl_expiry', minDaysRemaining: days}, + } + } + default: + throw new Error( + `--assertion DSL only supports status_code, response_time, ssl_expiry. ` + + `Use the JSON form for "${type}", e.g. ` + + `--assertion '{"severity":"fail","config":{"type":"${type}",...}}'`, + ) + } +} + +/** + * POST each assertion to `/api/v1/monitors/{id}/assertions`. If any + * call fails, delete the monitor so the user isn't left with a partial + * setup — better to fail loudly and re-run than to ship a monitor with + * a different alert profile than what the YAML/CLI invocation requested. + * + * The cleanup is best-effort: if the rollback DELETE itself fails (rare, + * usually a transient API hiccup) we surface the original assertion + * error and append a hint about the orphaned monitor so the operator can + * clean it up by hand. + */ +export async function applyAssertions( + monitorId: string, + assertions: readonly ParsedAssertion[], + client: ApiClient, + monitorPath: string = '/api/v1/monitors', +): Promise { + const path = `${monitorPath}/${monitorId}/assertions` + for (const [idx, a] of assertions.entries()) { + try { + await apiPost(client, path, a) + } catch (err) { + const cleanup = await rollbackMonitor(monitorId, monitorPath, client) + const original = err instanceof Error ? err.message : String(err) + const tail = cleanup + ? 'Monitor was rolled back.' + : `Monitor ${monitorId} could NOT be rolled back automatically — delete it manually if it still exists.` + const assertionType = + typeof a.config.type === 'string' ? a.config.type : 'unknown' + throw new Error( + `Failed to apply --assertion #${idx + 1} (${assertionType}): ${original}. ${tail}`, + ) + } + } +} + +async function rollbackMonitor( + monitorId: string, + monitorPath: string, + client: ApiClient, +): Promise { + try { + await apiDelete(client, `${monitorPath}/${monitorId}`) + return true + } catch { + return false + } +} diff --git a/src/lib/resources.ts b/src/lib/resources.ts index 2bd848d..786e1df 100644 --- a/src/lib/resources.ts +++ b/src/lib/resources.ts @@ -6,6 +6,8 @@ import type {components} from './api.generated.js' import {schemas as apiSchemas} from './api-zod.generated.js' import {fieldDescriptions} from './descriptions.generated.js' import {urlFlag} from './validators.js' +import {applyAssertions, parseAssertionFlag, type ParsedAssertion} from './monitor-assertions.js' +import {attachAlertChannelsOrRollback, parseAlertChannelsFlag, setAlertChannels} from './monitor-alert-channels.js' import { MONITOR_TYPES, HTTP_METHODS, @@ -87,6 +89,18 @@ export const MONITORS: FullResource = { }), port: Flags.string({description: desc('TcpMonitorConfig', 'port', 'TCP port to connect to')}), regions: Flags.string({description: desc('CreateMonitorRequest', 'regions')}), + assertion: Flags.string({ + description: + 'Assertion to attach (repeatable). DSL: status_code=200, response_time<5000, ' + + 'ssl_expiry>=14. JSON: \'{"severity":"fail","config":{"type":"...","..."}}\'. ' + + 'Failures roll back the monitor.', + multiple: true, + }), + 'alert-channels': Flags.string({ + description: + 'Comma-separated alert channel IDs to attach after create. ' + + 'Failures roll back the monitor.', + }), }, updateFlags: { name: Flags.string({description: desc('UpdateMonitorRequest', 'name')}), @@ -94,6 +108,10 @@ export const MONITORS: FullResource = { frequency: Flags.string({description: desc('UpdateMonitorRequest', 'frequencySeconds')}), method: Flags.string({description: desc('HttpMonitorConfig', 'method'), options: [...HTTP_METHODS]}), port: Flags.string({description: desc('TcpMonitorConfig', 'port', 'TCP port to connect to')}), + 'alert-channels': Flags.string({ + description: + 'Comma-separated alert channel IDs (replaces current list; pass empty string to clear)', + }), }, // bodyBuilder returns a plain `Record`; the outer // `parseSchema(MONITORS.createRequestSchema, ...)` / `updateRequestSchema` @@ -113,6 +131,17 @@ export const MONITORS: FullResource = { body.config = buildMonitorConfig(monitorType, raw) if (raw.regions) { body.regions = String(raw.regions).split(',').map((s) => s.trim()).filter(Boolean) + } else if (REGIONS_REQUIRED_TYPES.has(monitorType)) { + // The API rejects probe-driven monitors without at least one + // region (P1.Bug9 — the unhelpful `400: At least one region is + // required for HTTP monitors`). Default to a single region and + // let users know via stderr so the create still succeeds while + // it's obvious where the choice came from. Heartbeat / push + // monitors are excluded because they don't run from probes. + body.regions = [...DEFAULT_MONITOR_REGIONS] + process.stderr.write( + 'note: defaulting --regions us-east; use --regions to override\n', + ) } } else { if (raw.frequency) body.frequencySeconds = Number(raw.frequency) @@ -122,8 +151,72 @@ export const MONITORS: FullResource = { } return body }, + // Post-create: attach assertions and alert channels. Both APIs are + // separate calls — the create endpoint only accepts the core monitor + // shape. If either step fails, roll back the monitor so the operator + // isn't left with a partial setup. Assertions run first because + // they're typically the reason someone is creating the monitor; a + // successful assertion attach + failed channel attach still rolls + // back so behaviour is uniform. + afterCreate: async (created, raw, ctx) => { + const monitorId = extractMonitorId(created) + if (!monitorId) return + const assertions = collectAssertions(raw.assertion) + if (assertions.length > 0) { + await applyAssertions(monitorId, assertions, ctx.client, ctx.apiPath) + } + const channelIds = parseAlertChannelsFlag(rawString(raw['alert-channels'])) + if (channelIds && channelIds.length > 0) { + await attachAlertChannelsOrRollback(monitorId, channelIds, ctx.client, ctx.apiPath) + } + }, + // Post-update: only handles --alert-channels (assertions on update + // would race with the API's own assertion list, so we keep that to + // the dedicated `monitors assertions` topic). An empty string + // explicitly clears all channels — undefined leaves them alone. + afterUpdate: async (_updated, raw, ctx) => { + const channelIds = parseAlertChannelsFlag(rawString(raw['alert-channels'])) + if (channelIds === undefined) return + await setAlertChannels(ctx.id, channelIds, ctx.client, ctx.apiPath) + }, +} + +function extractMonitorId(value: unknown): string | undefined { + if (!value || typeof value !== 'object') return undefined + const id = (value as {id?: unknown}).id + return typeof id === 'string' && id.length > 0 ? id : undefined } +function rawString(v: unknown): string | undefined { + return typeof v === 'string' ? v : undefined +} + +function collectAssertions(raw: unknown): ParsedAssertion[] { + if (raw === undefined) return [] + // oclif passes `multiple: true` flags as a string array, but be + // defensive against single-value invocations and unknown shapes — + // the body builder + hook are reused across tests and YAML paths. + const items = Array.isArray(raw) ? raw : [raw] + const out: ParsedAssertion[] = [] + for (const item of items) { + if (typeof item !== 'string') continue + out.push(parseAssertionFlag(item)) + } + return out +} + +// Monitor types that the API requires to declare at least one probe +// region. Includes the four shipping types plus the experimental +// browser variants (HTTP_HEADLESS, HTTP_BROWSER) that are wired in the +// CLI ahead of the spec — once they land in MONITOR_TYPES this set +// stays accurate without code changes. MCP_SERVER and HEARTBEAT are +// intentionally excluded: HEARTBEAT is push-driven, and MCP_SERVER's +// region semantics are still in flux. +const REGIONS_REQUIRED_TYPES: ReadonlySet = new Set([ + 'HTTP', 'TCP', 'DNS', 'ICMP', 'HTTP_HEADLESS', 'HTTP_BROWSER', +]) +const DEFAULT_MONITOR_REGIONS: readonly string[] = ['us-east'] + // Returns the monitor's `config` payload as a plain object — the discriminated // union (HttpMonitorConfig | TcpMonitorConfig | …) is enforced by the outer // `apiSchemas.Create/UpdateMonitorRequest` Zod parse, so an unknown `type` @@ -218,13 +311,17 @@ export const ALERT_CHANNELS: FullResource = { options: [...CHANNEL_TYPES], }), config: Flags.string({description: 'Channel-specific configuration as JSON'}), - 'webhook-url': urlFlag({description: desc('SlackChannelConfig', 'webhookUrl', 'Slack/Discord/Teams webhook URL')}), + 'webhook-url': urlFlag({ + description: 'Webhook URL (used by webhook, slack, discord, teams channel types)', + }), }, updateFlags: { name: Flags.string({description: desc('UpdateAlertChannelRequest', 'name')}), type: Flags.string({description: 'Alert channel type', options: [...CHANNEL_TYPES]}), config: Flags.string({description: 'Channel-specific configuration as JSON'}), - 'webhook-url': urlFlag({description: desc('SlackChannelConfig', 'webhookUrl', 'Slack/Discord/Teams webhook URL')}), + 'webhook-url': urlFlag({ + description: 'Webhook URL (used by webhook, slack, discord, teams channel types)', + }), }, // bodyBuilder produces a plain object; the outer // `apiSchemas.Create/UpdateAlertChannelRequest` Zod parse validates the diff --git a/src/lib/spec-facts.generated.ts b/src/lib/spec-facts.generated.ts index 9dd7cd3..33740cc 100644 --- a/src/lib/spec-facts.generated.ts +++ b/src/lib/spec-facts.generated.ts @@ -55,7 +55,7 @@ export type SpIncidentStatuses = (typeof SP_INCIDENT_STATUSES)[number] export const AUTH_TYPES = ['bearer', 'basic', 'header', 'api_key'] as const export type AuthTypes = (typeof AUTH_TYPES)[number] -export const MANAGED_BY = ['DASHBOARD', 'CLI', 'TERRAFORM'] as const +export const MANAGED_BY = ['DASHBOARD', 'CLI', 'TERRAFORM', 'MCP', 'API'] as const export type ManagedBy = (typeof MANAGED_BY)[number] export const MATCH_RULE_TYPES = ['severity_gte', 'monitor_id_in', 'region_in', 'incident_status', 'monitor_type_in', 'service_id_in', 'resource_group_id_in', 'component_name_in', 'monitor_tag_in'] as const diff --git a/src/lib/yaml/differ.ts b/src/lib/yaml/differ.ts index 6af0ebd..1554448 100644 --- a/src/lib/yaml/differ.ts +++ b/src/lib/yaml/differ.ts @@ -15,7 +15,7 @@ import type {components} from '../api.generated.js' import type {DevhelmConfig} from './schema.js' import type {ResolvedRefs} from './resolver.js' import {allHandlers, type ResourceHandler, type CurrentChildEntry} from './handlers.js' -import type {Change, Changeset, DiffOptions} from './types.js' +import type {Change, Changeset, DiffOptions, PruneScope} from './types.js' import {RESOURCE_ORDER} from './types.js' import type {DeployState} from './state.js' import {resourceAddress} from './state.js' @@ -68,8 +68,18 @@ export async function diff( return currentChildren.get(addr) ?? {} } + // Set of `
.` addresses present in this config's + // `.devhelm/state.json`. Drives the `--prune` (state-scoped) branch + // in `diffSection` — non-state addresses are only candidates under + // `--prune-org-cli` or `--prune-all`. Lazily an empty set when no + // state was passed so callers / tests can omit `priorState` without + // accidentally widening the prune scope. + const stateAddresses = new Set( + priorState ? Object.keys(priorState.resources) : [], + ) + for (const handler of allHandlers()) { - diffSection(handler, config[handler.configKey], refs, creates, updates, deletes, options, lookupCurrentChildren) + diffSection(handler, config[handler.configKey], refs, creates, updates, deletes, options, lookupCurrentChildren, stateAddresses) } diffMemberships(config, refs, memberships, options) @@ -78,12 +88,6 @@ export async function diff( updates.sort(byResourceOrder) deletes.sort((a, b) => byResourceOrderIndex(b.resourceType) - byResourceOrderIndex(a.resourceType)) - // Acknowledge unused param: kept in the signature for future drift hooks - // (e.g. detecting parents present in state but absent from both YAML and - // the live API) without forcing every caller to refactor. The differ - // itself reads parent attrs only from `refs` (live API) per RFC 0001. - void priorState - return {creates, updates, deletes, memberships} } @@ -160,6 +164,7 @@ function diffSection( deletes: Change[], options: DiffOptions, lookupCurrentChildren: (resourceType: ResourceType, refKey: string) => Record, + stateAddresses: ReadonlySet, ): void { const desired = new Set() @@ -204,23 +209,57 @@ function diffSection( } } - if ((options.prune || options.pruneAll) && items !== undefined) { + const anyPrune = options.prune || options.pruneOrgCli || options.pruneAll + if (anyPrune && items !== undefined) { for (const entry of refs.allEntries(handler.refType)) { if (entry.isPending) continue - if (!desired.has(entry.refKey)) { - if (handler.resourceType === 'monitor' && !options.pruneAll && entry.managedBy !== 'CLI') continue - deletes.push({ - action: 'delete', - resourceType: handler.resourceType, - refKey: entry.refKey, - existingId: entry.id, - current: entry.raw, - }) - } + if (desired.has(entry.refKey)) continue + + const address = resourceAddress(handler.resourceType as ResourceType, entry.refKey) + const scope = classifyDeleteScope(handler, entry, stateAddresses, address) + + // Decide whether to include this delete based on the requested + // prune flag(s). State-scoped deletes are the safe default and + // are always included when *any* prune flag is on; the wider + // scopes require their explicit opt-in. + const include = + (scope === 'state') || + (scope === 'org-cli' && (options.pruneOrgCli || options.pruneAll)) || + (scope === 'org-all' && options.pruneAll) + if (!include) continue + + deletes.push({ + action: 'delete', + resourceType: handler.resourceType, + refKey: entry.refKey, + existingId: entry.id, + current: entry.raw, + pruneScope: scope, + }) } } } +function classifyDeleteScope( + handler: ResourceHandler, + entry: {refKey: string; managedBy?: string | null}, + stateAddresses: ReadonlySet, + address: string, +): PruneScope { + if (stateAddresses.has(address)) return 'state' + // Monitors carry an explicit `managedBy`; non-CLI monitors are + // foreign (dashboard / Terraform / MCP / API) and only `--prune-all` + // is allowed to touch them. + if (handler.resourceType === 'monitor') { + return entry.managedBy === 'CLI' ? 'org-cli' : 'org-all' + } + // Other resource types lack a managedBy concept on the API, so the + // legacy `--prune` semantic was "delete anything not in YAML" — keep + // that under `--prune-org-cli` for parity, and reserve `--prune-all` + // for an unambiguous "everything". + return 'org-cli' +} + // ── Membership diff ──────────────────────────────────────────────────── // // Why memberships do **not** use the generic `ChildCollectionDef` machinery @@ -251,12 +290,13 @@ function memberKey(memberType: string, nameOrSlug: string): string { * Diff resource group memberships. * * Prune semantics (documented for M8): - * - `options.prune` or `options.pruneAll`: + * - Any prune flag (`--prune` / `--prune-org-cli` / `--prune-all`): * - For groups **present in YAML**: remove any current members not listed. * - For groups **absent from YAML** (orphan groups): *only* under - * `pruneAll` — because `prune` is scoped to CLI-managed resources and - * we don't know membership provenance. `pruneAll` takes the Terraform - * stance: anything not in config is fair game. + * `pruneAll` — membership provenance isn't tracked anywhere outside + * the YAML, so the safe default is to leave dashboard-curated + * memberships alone. `pruneAll` takes the Terraform stance: + * anything not in config is fair game. * - Without prune flags, memberships are additive: we only create missing * ones, never remove. */ @@ -311,7 +351,8 @@ function diffMemberships( } } - if ((options.prune || options.pruneAll) && groupEntry) { + const anyPrune = options.prune || options.pruneOrgCli || options.pruneAll + if (anyPrune && groupEntry) { for (const [key, member] of currentMembers) { if (!desired.has(key)) { const label = member.name ?? member.slug ?? member.id ?? 'unknown' @@ -415,8 +456,28 @@ export function formatPlan(changeset: Changeset): string { } } } - for (const c of changeset.deletes) { - lines.push(` - ${c.resourceType} "${c.refKey}"`) + // Group destroys by prune scope so multi-team users can tell at a + // glance which ones came from this config's state and which came from + // the wider `--prune-org-cli` / `--prune-all` widening. When every + // delete shares a single scope (the common case), only that group's + // header is printed. + if (changeset.deletes.length > 0) { + const groups: Array<{label: string; scope: PruneScope | undefined}> = [ + {label: 'Tracked by this config:', scope: 'state'}, + {label: 'Other CLI-managed resources:', scope: 'org-cli'}, + {label: 'Foreign resources (--prune-all):', scope: 'org-all'}, + ] + const present = new Set(changeset.deletes.map((d) => d.pruneScope ?? 'state')) + const showHeaders = present.size > 1 + for (const {label, scope} of groups) { + const items = changeset.deletes.filter((d) => (d.pruneScope ?? 'state') === scope) + if (items.length === 0) continue + if (showHeaders) lines.push(` ${label}`) + for (const c of items) { + const indent = showHeaders ? ' ' : ' ' + lines.push(`${indent}- ${c.resourceType} "${c.refKey}"`) + } + } } for (const c of changeset.memberships) { const icon = c.action === 'delete' ? '- membership' : '→' diff --git a/src/lib/yaml/types.ts b/src/lib/yaml/types.ts index 3091183..747f293 100644 --- a/src/lib/yaml/types.ts +++ b/src/lib/yaml/types.ts @@ -53,10 +53,38 @@ export interface Change { current?: unknown /** Attribute-level diffs for update changes */ attributeDiffs?: AttributeDiff[] + /** + * For `delete` actions only — which prune scope produced the change. + * `state` means the resource was tracked in this config's + * `.devhelm/state.json`; `org-cli` means it's CLI-managed elsewhere + * in the org; `org-all` is anything else surfaced under `--prune-all`. + * Drives the grouped output in `formatPlan` so multi-team users can + * tell at a glance which destroys are theirs. + */ + pruneScope?: PruneScope } +export type PruneScope = 'state' | 'org-cli' | 'org-all' + export interface DiffOptions { + /** + * Delete resources tracked in `.devhelm/state.json` that are absent + * from YAML. Safe in multi-config orgs because it only touches + * resources THIS config previously created (DevEx P0.Bug1). + */ prune?: boolean + /** + * Delete CLI-managed resources org-wide that are absent from YAML — + * the legacy `--prune` behaviour. Required to clean up resources + * created from a different YAML file or by an earlier reset of the + * local state. Use with caution in shared workspaces. + */ + pruneOrgCli?: boolean + /** + * Delete ALL resources absent from YAML, including dashboard- and + * Terraform-managed ones. The most destructive option; reserve for + * single-tenant workspaces or scripted teardowns. + */ pruneAll?: boolean } diff --git a/test/lib/crud-commands.test.ts b/test/lib/crud-commands.test.ts new file mode 100644 index 0000000..a04e952 --- /dev/null +++ b/test/lib/crud-commands.test.ts @@ -0,0 +1,31 @@ +import {describe, expect, it} from 'vitest' +import {aOrAn} from '../../src/lib/crud-commands.js' + +describe('aOrAn (P1.Bug8)', () => { + it.each([ + ['monitor', 'a'], + ['secret', 'a'], + ['tag', 'a'], + ['notification policy', 'a'], + ['resource group', 'a'], + ['webhook', 'a'], + ['dependency', 'a'], + ['status page', 'a'], + ])('"%s" → "%s"', (word, expected) => { + expect(aOrAn(word)).toBe(expected) + }) + + it.each([ + ['alert channel', 'an'], + ['API key', 'an'], + ['environment', 'an'], + ['incident', 'an'], + ['organization', 'an'], + ])('"%s" → "%s"', (word, expected) => { + expect(aOrAn(word)).toBe(expected) + }) + + it('strips leading whitespace before checking', () => { + expect(aOrAn(' alert channel')).toBe('an') + }) +}) diff --git a/test/lib/monitor-alert-channels.test.ts b/test/lib/monitor-alert-channels.test.ts new file mode 100644 index 0000000..27330f1 --- /dev/null +++ b/test/lib/monitor-alert-channels.test.ts @@ -0,0 +1,87 @@ +import {describe, expect, it, vi, beforeEach} from 'vitest' +import { + attachAlertChannelsOrRollback, + parseAlertChannelsFlag, + setAlertChannels, +} from '../../src/lib/monitor-alert-channels.js' + +describe('parseAlertChannelsFlag (P1.Bug3)', () => { + it('returns undefined when the flag was not provided', () => { + expect(parseAlertChannelsFlag(undefined)).toBeUndefined() + }) + + it('returns an empty array for the empty string (clears all channels)', () => { + expect(parseAlertChannelsFlag('')).toEqual([]) + }) + + it('splits a comma-separated list and trims each entry', () => { + expect(parseAlertChannelsFlag('a, b , c')).toEqual(['a', 'b', 'c']) + }) + + it('drops empty entries from trailing commas', () => { + expect(parseAlertChannelsFlag('a,,b,')).toEqual(['a', 'b']) + }) +}) + +describe('setAlertChannels / attachAlertChannelsOrRollback', () => { + type Outcome = {ok: true; data?: unknown} | {ok: false; status: number; message: string} + + function fakeClient(outcomes: Record) { + const calls: Array<{method: string; path: string; body?: unknown}> = [] + function makeResponse(method: string, path: string, body?: unknown) { + calls.push({method, path, body}) + const o = outcomes[`${method} ${path}`] ?? outcomes[method] ?? {ok: true} + const response = { + ok: o.ok, status: o.ok ? 200 : o.status, + statusText: o.ok ? 'OK' : 'ERR', + headers: new Headers(), + } as Response + return Promise.resolve(o.ok ? {data: o.data ?? {}, response} : {error: {message: o.message}, response}) + } + return { + client: { + PUT: vi.fn((path: string, opts?: {body?: unknown}) => makeResponse('PUT', path, opts?.body)), + DELETE: vi.fn((path: string) => makeResponse('DELETE', path)), + }, + calls, + } + } + + let stderr: ReturnType + beforeEach(() => { + stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + it('PUTs the channel list to the alert-channels endpoint', async () => { + const {client, calls} = fakeClient({}) + await setAlertChannels('mon-1', ['ch-1', 'ch-2'], client as never) + expect(calls).toHaveLength(1) + expect(calls[0].path).toBe('/api/v1/monitors/mon-1/alert-channels') + expect(calls[0].body).toEqual({channelIds: ['ch-1', 'ch-2']}) + }) + + it('attachAlertChannelsOrRollback skips the PUT when the list is empty', async () => { + const {client, calls} = fakeClient({}) + await attachAlertChannelsOrRollback('mon-1', [], client as never) + expect(calls).toHaveLength(0) + }) + + it('rolls back the monitor on a failed PUT', async () => { + const {client, calls} = fakeClient({PUT: {ok: false, status: 400, message: 'bad'}}) + await expect( + attachAlertChannelsOrRollback('mon-1', ['ch-1'], client as never), + ).rejects.toThrow(/Failed to attach --alert-channels.*Monitor was rolled back/s) + expect(calls.find((c) => c.method === 'DELETE')?.path).toBe('/api/v1/monitors/mon-1') + expect(stderr).not.toHaveBeenCalled() + }) + + it('reports both errors when the rollback DELETE itself fails', async () => { + const {client} = fakeClient({ + PUT: {ok: false, status: 500, message: 'boom'}, + DELETE: {ok: false, status: 500, message: 'oh no'}, + }) + await expect( + attachAlertChannelsOrRollback('mon-1', ['ch-1'], client as never), + ).rejects.toThrow(/could NOT be rolled back automatically/) + }) +}) diff --git a/test/lib/monitor-assertions.test.ts b/test/lib/monitor-assertions.test.ts new file mode 100644 index 0000000..c0ff3b1 --- /dev/null +++ b/test/lib/monitor-assertions.test.ts @@ -0,0 +1,200 @@ +import {describe, expect, it, vi, beforeEach} from 'vitest' +import {applyAssertions, parseAssertionFlag} from '../../src/lib/monitor-assertions.js' + +describe('parseAssertionFlag — DSL form (P1.Bug2)', () => { + it('parses status_code=200 → fail / equals', () => { + expect(parseAssertionFlag('status_code=200')).toEqual({ + severity: 'fail', + config: {type: 'status_code', expected: '200', operator: 'equals'}, + }) + }) + + it('parses status_code=2xx (range pattern)', () => { + const result = parseAssertionFlag('status_code=2xx') + expect(result.config).toMatchObject({type: 'status_code', expected: '2xx', operator: 'equals'}) + }) + + it('parses response_time<5000 → warn / thresholdMs', () => { + expect(parseAssertionFlag('response_time<5000')).toEqual({ + severity: 'warn', + config: {type: 'response_time', thresholdMs: 5000}, + }) + }) + + it('parses ssl_expiry>=14 → warn / minDaysRemaining', () => { + expect(parseAssertionFlag('ssl_expiry>=14')).toEqual({ + severity: 'warn', + config: {type: 'ssl_expiry', minDaysRemaining: 14}, + }) + }) + + it('tolerates surrounding whitespace and inline padding', () => { + expect(parseAssertionFlag(' status_code = 200 ')).toEqual({ + severity: 'fail', + config: {type: 'status_code', expected: '200', operator: 'equals'}, + }) + }) + + it('rejects empty input', () => { + expect(() => parseAssertionFlag(' ')).toThrow(/empty value/) + }) + + it('rejects unknown DSL types with a JSON-form hint', () => { + expect(() => parseAssertionFlag('redirect_count=2')).toThrow(/JSON form for "redirect_count"/) + }) + + it('rejects status_code with non-equals operator', () => { + expect(() => parseAssertionFlag('status_code<200')).toThrow(/status_code only supports "="/) + }) + + it('rejects response_time with non-< operator', () => { + expect(() => parseAssertionFlag('response_time>=5000')).toThrow(/response_time only supports "<"/) + }) + + it('rejects ssl_expiry with non->= operator', () => { + expect(() => parseAssertionFlag('ssl_expiry<14')).toThrow(/ssl_expiry only supports ">="/) + }) + + it('rejects non-integer response_time threshold', () => { + expect(() => parseAssertionFlag('response_time<5.5')).toThrow(/positive integer/) + expect(() => parseAssertionFlag('response_time<-1')).toThrow(/positive integer/) + expect(() => parseAssertionFlag('response_time { + expect(() => parseAssertionFlag('status_code 200')).toThrow(/cannot parse/) + }) +}) + +describe('parseAssertionFlag — JSON form', () => { + it('parses a full {severity, config} object', () => { + const json = JSON.stringify({ + severity: 'fail', + config: {type: 'status_code', expected: '200', operator: 'equals'}, + }) + expect(parseAssertionFlag(json)).toEqual({ + severity: 'fail', + config: {type: 'status_code', expected: '200', operator: 'equals'}, + }) + }) + + it('defaults severity to "fail" when omitted', () => { + const json = JSON.stringify({config: {type: 'response_size', maxBytes: 100}}) + expect(parseAssertionFlag(json)).toMatchObject({severity: 'fail'}) + }) + + it('passes through any assertion type (no DSL restriction)', () => { + const json = JSON.stringify({ + severity: 'warn', + config: {type: 'json_path', path: '$.status', expected: 'ok', operator: 'equals'}, + }) + expect(parseAssertionFlag(json).config).toMatchObject({type: 'json_path', path: '$.status'}) + }) + + it('rejects malformed JSON with a parse error', () => { + expect(() => parseAssertionFlag('{not-json')).toThrow(/invalid JSON/) + }) + + it('rejects JSON arrays', () => { + expect(() => parseAssertionFlag('[]')).toThrow(/object with \{severity, config\}/) + }) + + it('rejects unknown severity', () => { + const json = JSON.stringify({severity: 'critical', config: {type: 'status_code'}}) + expect(() => parseAssertionFlag(json)).toThrow(/severity must be "fail" or "warn"/) + }) + + it('rejects missing config', () => { + expect(() => parseAssertionFlag('{"severity":"fail"}')).toThrow(/missing or invalid `config`/) + }) + + it('rejects config without type', () => { + expect(() => parseAssertionFlag('{"severity":"fail","config":{}}')).toThrow(/config\.type is required/) + }) +}) + +describe('applyAssertions', () => { + type FakeRequest = {method: string; path: string; body?: unknown} + type Outcome = {ok: true; data?: unknown} | {ok: false; status: number; message: string} + + /** + * Minimal fake of the openapi-fetch ApiClient surface that matches + * what `apiPost` / `apiDelete` actually call. Each handler returns an + * Outcome that the fake materialises into the `{data, error, response}` + * shape `checkedFetch` expects. + */ + function fakeClient(outcomes: Record Outcome)>) { + const calls: FakeRequest[] = [] + function resolve(key: string): Outcome { + const v = outcomes[key] ?? outcomes[key.split(' ')[0]] ?? {ok: true} + return typeof v === 'function' ? v() : v + } + function makeResponse(method: string, path: string, body?: unknown): Promise<{data?: unknown; error?: unknown; response: Response}> { + calls.push({method, path, body}) + const o = resolve(`${method} ${path}`) + const response = { + ok: o.ok, status: o.ok ? 200 : o.status, + statusText: o.ok ? 'OK' : 'ERR', + headers: new Headers(), + } as Response + if (o.ok) return Promise.resolve({data: o.data ?? {}, response}) + return Promise.resolve({error: {message: o.message}, response}) + } + const client = { + POST: vi.fn((path: string, opts?: {body?: unknown}) => makeResponse('POST', path, opts?.body)), + DELETE: vi.fn((path: string) => makeResponse('DELETE', path)), + } + return {client, calls} + } + + let stderr: ReturnType + + beforeEach(() => { + stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + it('POSTs each assertion to the monitor assertions endpoint', async () => { + const {client, calls} = fakeClient({}) + const assertions = [ + {severity: 'fail' as const, config: {type: 'status_code', expected: '200', operator: 'equals'}}, + {severity: 'warn' as const, config: {type: 'response_time', thresholdMs: 5000}}, + ] + await applyAssertions('mon-1', assertions, client as never) + const posts = calls.filter((c) => c.method === 'POST') + expect(posts).toHaveLength(2) + expect(posts[0].path).toBe('/api/v1/monitors/mon-1/assertions') + expect(posts[0].body).toMatchObject({severity: 'fail', config: {type: 'status_code'}}) + }) + + it('rolls back the monitor when an assertion POST fails', async () => { + const {client, calls} = fakeClient({ + POST: {ok: false, status: 400, message: 'bad request'}, + }) + const assertions = [{severity: 'fail' as const, config: {type: 'status_code', expected: '200', operator: 'equals'}}] + await expect(applyAssertions('mon-1', assertions, client as never)).rejects.toThrow( + /Failed to apply --assertion #1 \(status_code\).*Monitor was rolled back/s, + ) + expect(calls.find((c) => c.method === 'DELETE')?.path).toBe('/api/v1/monitors/mon-1') + expect(stderr).not.toHaveBeenCalled() + }) + + it('reports both the original failure and a rollback failure', async () => { + const {client} = fakeClient({ + POST: {ok: false, status: 400, message: '400'}, + DELETE: {ok: false, status: 500, message: 'boom'}, + }) + await expect( + applyAssertions( + 'mon-1', + [{severity: 'fail' as const, config: {type: 'status_code'}}], + client as never, + ), + ).rejects.toThrow(/could NOT be rolled back automatically/) + }) + + it('is a no-op for an empty assertion list', async () => { + const {client, calls} = fakeClient({}) + await applyAssertions('mon-1', [], client as never) + expect(calls).toHaveLength(0) + }) +}) diff --git a/test/lib/resources-validation.test.ts b/test/lib/resources-validation.test.ts index c20aa83..15bac4b 100644 --- a/test/lib/resources-validation.test.ts +++ b/test/lib/resources-validation.test.ts @@ -1,8 +1,8 @@ -import {describe, it, expect, beforeEach, afterEach} from 'vitest' +import {describe, it, expect, beforeEach, afterEach, vi} from 'vitest' import {mkdtempSync, writeFileSync, rmSync} from 'node:fs' import {join} from 'node:path' import {tmpdir} from 'node:os' -import {ALERT_CHANNELS, STATUS_PAGES} from '../../src/lib/resources.js' +import {ALERT_CHANNELS, MONITORS, STATUS_PAGES} from '../../src/lib/resources.js' describe('ALERT_CHANNELS bodyBuilder --config validation', () => { it('accepts valid slack config JSON', () => { @@ -98,3 +98,51 @@ describe('STATUS_PAGES bodyBuilder --branding-file validation', () => { expect(() => STATUS_PAGES.bodyBuilder!({'branding-file': path})).toThrow(/Invalid branding file/) }) }) + +describe('MONITORS bodyBuilder --regions defaulting (P1.Bug9)', () => { + let stderrSpy: ReturnType + + beforeEach(() => { + stderrSpy = vi.spyOn(process.stderr, 'write').mockImplementation(() => true) + }) + + afterEach(() => { + stderrSpy.mockRestore() + }) + + it('defaults regions to [us-east] for HTTP without --regions', () => { + const body = MONITORS.bodyBuilder({name: 'X', type: 'HTTP', url: 'https://x.com'}) as Record + expect(body.regions).toEqual(['us-east']) + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining('defaulting --regions us-east'), + ) + }) + + it.each(['TCP', 'DNS', 'ICMP', 'HTTP_HEADLESS', 'HTTP_BROWSER'])( + 'defaults regions for %s without --regions', + (type) => { + const body = MONITORS.bodyBuilder({name: 'X', type, url: 'x.com'}) as Record + expect(body.regions).toEqual(['us-east']) + }, + ) + + it('respects an explicit --regions value (no default applied)', () => { + const body = MONITORS.bodyBuilder({ + name: 'X', type: 'HTTP', url: 'https://x.com', regions: 'eu-west,ap-south', + }) as Record + expect(body.regions).toEqual(['eu-west', 'ap-south']) + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('does NOT default regions for HEARTBEAT (push monitor)', () => { + const body = MONITORS.bodyBuilder({name: 'X', type: 'HEARTBEAT'}) as Record + expect(body.regions).toBeUndefined() + expect(stderrSpy).not.toHaveBeenCalled() + }) + + it('does NOT default regions for MCP_SERVER (region semantics in flux)', () => { + const body = MONITORS.bodyBuilder({name: 'X', type: 'MCP_SERVER', url: 'mcp-server'}) as Record + expect(body.regions).toBeUndefined() + expect(stderrSpy).not.toHaveBeenCalled() + }) +}) diff --git a/test/yaml/differ.test.ts b/test/yaml/differ.test.ts index 35397d4..0eac952 100644 --- a/test/yaml/differ.test.ts +++ b/test/yaml/differ.test.ts @@ -201,18 +201,29 @@ describe('differ', () => { expect(changeset.deletes).toHaveLength(0) }) - it('detects deletes with prune=true', async () => { + it('--prune-org-cli detects deletes for CLI-managed monitors org-wide', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'old-monitor', { id: 'mon-1', refKey: 'old-monitor', managedBy: 'CLI', raw: {managedBy: 'CLI'}, }) const config: DevhelmConfig = {monitors: []} - const changeset = await diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {pruneOrgCli: true}) expect(changeset.deletes).toHaveLength(1) expect(changeset.deletes[0].refKey).toBe('old-monitor') + expect(changeset.deletes[0].pruneScope).toBe('org-cli') + }) + + it('skips non-CLI monitors during --prune-org-cli', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'dashboard-monitor', { + id: 'mon-1', refKey: 'dashboard-monitor', managedBy: 'DASHBOARD', raw: {managedBy: 'DASHBOARD'}, + }) + const config: DevhelmConfig = {monitors: []} + const changeset = await diff(config, refs, {pruneOrgCli: true}) + expect(changeset.deletes).toHaveLength(0) }) - it('skips non-CLI monitors during prune', async () => { + it('skips non-CLI monitors during plain --prune even when state is empty', async () => { const refs = new ResolvedRefs() refs.set('monitors', 'dashboard-monitor', { id: 'mon-1', refKey: 'dashboard-monitor', managedBy: 'DASHBOARD', raw: {managedBy: 'DASHBOARD'}, @@ -251,12 +262,13 @@ describe('differ', () => { expect(changeset.deletes).toHaveLength(0) }) - it('prune: empty array deletes all existing', async () => { + it('--prune-org-cli: empty array deletes all existing tags', async () => { const refs = new ResolvedRefs() refs.set('tags', 'T', {id: '1', refKey: 'T', raw: {name: 'T'}}) const config: DevhelmConfig = {tags: []} - const changeset = await diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {pruneOrgCli: true}) expect(changeset.deletes).toHaveLength(1) + expect(changeset.deletes[0].pruneScope).toBe('org-cli') }) it('does not delete without prune flag', async () => { @@ -266,6 +278,92 @@ describe('differ', () => { const changeset = await diff(config, refs, {prune: false}) expect(changeset.deletes).toHaveLength(0) }) + }) + + // ── --prune scope to state file (P0.Bug1, multi-team safety) ────────── + describe('diff: --prune state-scoped (P0.Bug1)', () => { + function stateWith(addresses: string[]): import('../../src/lib/yaml/state.js').DeployState { + return { + version: '3' as const, serial: 1, lastDeployedAt: '2026-05-05T00:00:00Z', + resources: Object.fromEntries( + addresses.map((addr) => [addr, { + apiId: addr.split('.').slice(1).join('.') + '-id', + resourceType: addr.split('.')[0] as never, + children: {}, + }]), + ), + } + } + + it('deletes a monitor that IS in this config\'s state file', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'mine', { + id: 'mon-1', refKey: 'mine', managedBy: 'CLI', raw: {managedBy: 'CLI'}, + }) + const state = stateWith(['monitors.mine']) + const cs = await diff({monitors: []}, refs, {prune: true}, state) + expect(cs.deletes).toHaveLength(1) + expect(cs.deletes[0].refKey).toBe('mine') + expect(cs.deletes[0].pruneScope).toBe('state') + }) + + it('does NOT delete a CLI monitor that is NOT in this state file (multi-team safety)', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'theirs', { + id: 'mon-2', refKey: 'theirs', managedBy: 'CLI', raw: {managedBy: 'CLI'}, + }) + const state = stateWith([]) // empty state — this config never created `theirs` + const cs = await diff({monitors: []}, refs, {prune: true}, state) + expect(cs.deletes).toHaveLength(0) + }) + + it('--prune-org-cli widens scope and deletes the other team\'s CLI monitor', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'theirs', { + id: 'mon-2', refKey: 'theirs', managedBy: 'CLI', raw: {managedBy: 'CLI'}, + }) + const cs = await diff({monitors: []}, refs, {pruneOrgCli: true}, stateWith([])) + expect(cs.deletes).toHaveLength(1) + expect(cs.deletes[0].pruneScope).toBe('org-cli') + }) + + it('reports state-tracked and org-cli deletes with their scope', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'mine', { + id: 'mon-1', refKey: 'mine', managedBy: 'CLI', raw: {managedBy: 'CLI'}, + }) + refs.set('monitors', 'theirs', { + id: 'mon-2', refKey: 'theirs', managedBy: 'CLI', raw: {managedBy: 'CLI'}, + }) + const cs = await diff({monitors: []}, refs, {pruneOrgCli: true}, stateWith(['monitors.mine'])) + const byKey = Object.fromEntries(cs.deletes.map((d) => [d.refKey, d.pruneScope])) + expect(byKey).toEqual({mine: 'state', theirs: 'org-cli'}) + }) + + it('--prune-all picks up dashboard-managed monitors as scope=org-all', async () => { + const refs = new ResolvedRefs() + refs.set('monitors', 'dash', { + id: 'mon-3', refKey: 'dash', managedBy: 'DASHBOARD', raw: {managedBy: 'DASHBOARD'}, + }) + const cs = await diff({monitors: []}, refs, {pruneAll: true}, stateWith([])) + expect(cs.deletes).toHaveLength(1) + expect(cs.deletes[0].pruneScope).toBe('org-all') + }) + + it('non-monitor resources outside state are NOT touched under plain --prune', async () => { + const refs = new ResolvedRefs() + refs.set('tags', 'shared', {id: 't-1', refKey: 'shared', raw: {name: 'shared'}}) + const cs = await diff({tags: []}, refs, {prune: true}, stateWith([])) + expect(cs.deletes).toHaveLength(0) + }) + + it('non-monitor resources outside state ARE touched under --prune-org-cli', async () => { + const refs = new ResolvedRefs() + refs.set('tags', 'shared', {id: 't-1', refKey: 'shared', raw: {name: 'shared'}}) + const cs = await diff({tags: []}, refs, {pruneOrgCli: true}, stateWith([])) + expect(cs.deletes).toHaveLength(1) + expect(cs.deletes[0].pruneScope).toBe('org-cli') + }) it('creates in dependency order', async () => { const config: DevhelmConfig = { @@ -284,7 +382,7 @@ describe('differ', () => { refs.set('tags', 'T', {id: '1', refKey: 'T', raw: {}}) refs.set('monitors', 'M', {id: '2', refKey: 'M', managedBy: 'CLI', raw: {}}) const config: DevhelmConfig = {tags: [], monitors: []} - const changeset = await diff(config, refs, {prune: true}) + const changeset = await diff(config, refs, {pruneOrgCli: true}) const types = changeset.deletes.map((c) => c.resourceType) expect(types.indexOf('monitor')).toBeLessThan(types.indexOf('tag')) }) @@ -1244,5 +1342,32 @@ describe('differ', () => { expect(result).toContain('name: "API" → "Core API"') expect(result).toContain('frequencySeconds: 60 → 30') }) + + it('groups destroys by prune scope when scopes differ (P0.Bug1)', async () => { + const changeset = { + creates: [], updates: [], memberships: [], + deletes: [ + {action: 'delete' as const, resourceType: 'monitor' as const, refKey: 'mine', existingId: 'm-1', pruneScope: 'state' as const}, + {action: 'delete' as const, resourceType: 'monitor' as const, refKey: 'theirs', existingId: 'm-2', pruneScope: 'org-cli' as const}, + ], + } + const result = formatPlan(changeset) + expect(result).toContain('Tracked by this config:') + expect(result).toContain('Other CLI-managed resources:') + expect(result).toContain('- monitor "mine"') + expect(result).toContain('- monitor "theirs"') + }) + + it('omits destroy headers when every delete shares one scope', async () => { + const changeset = { + creates: [], updates: [], memberships: [], + deletes: [ + {action: 'delete' as const, resourceType: 'monitor' as const, refKey: 'mine', existingId: 'm-1', pruneScope: 'state' as const}, + ], + } + const result = formatPlan(changeset) + expect(result).not.toContain('Tracked by this config:') + expect(result).toContain('- monitor "mine"') + }) }) }) diff --git a/test/yaml/idempotency.test.ts b/test/yaml/idempotency.test.ts index 891f792..4c4bbf4 100644 --- a/test/yaml/idempotency.test.ts +++ b/test/yaml/idempotency.test.ts @@ -151,7 +151,7 @@ describe('idempotency', () => { expect(cs.updates).toHaveLength(0) }) - it('removing one monitor → that monitor in deletes (with prune)', async () => { + it('removing one monitor → that monitor in deletes (with prune-org-cli)', async () => { const config: DevhelmConfig = { monitors: [ {name: 'API', type: 'HTTP', config: {url: 'https://api.example.com', method: 'GET'}}, @@ -169,7 +169,7 @@ describe('idempotency', () => { managedBy: 'CLI', }, managedBy: 'CLI'}, ]) - const cs = await diff(config, refs, {prune: true}) + const cs = await diff(config, refs, {pruneOrgCli: true}) expect(cs.deletes).toHaveLength(1) expect(cs.deletes[0].refKey).toBe('Web') expect(cs.creates).toHaveLength(0)