Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions apps/cli-docs/src/fragments/commands/alert.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
# Create an issue alert rule with inline JSON condition/action
sentry alert issues create my-org/my-project \
--name "Error Spike" \
--condition '{"id":"sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"}' \
--action '{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}' \
--action-match any
--condition '{"type":"first_seen_event","comparison":true,"conditionResult":true}' \
--action '{"type":"email","data":{},"config":{"targetType":"team","targetIdentifier":"1"}}'
```

### List issue alert rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,10 @@ Create an issue alert rule
- `--name <value> - Rule name`
- `-c, --condition <value>... - Condition object JSON (repeatable, or pass one JSON array)`
- `-a, --action <value>... - Action object JSON (repeatable, or pass one JSON array)`
- `-m, --action-match <value> - Condition/action match mode: all or any`
- `--frequency <value> - Frequency in minutes (default: 30) - (default: 30)`
- `--environment <value> - Environment filter`
- `--filter <value>... - Filter object JSON (repeatable, or pass one JSON array)`
- `--filter-match <value> - Filter match mode: all or any`
- `-m, --filter-match <value> - Filter match mode: all or any`
- `--owner <value> - Owner (team:user style value accepted by Sentry API)`
- `-n, --dry-run - Show what would happen without making changes`

Expand All @@ -71,9 +70,8 @@ Create an issue alert rule
# Create an issue alert rule with inline JSON condition/action
sentry alert issues create my-org/my-project \
--name "Error Spike" \
--condition '{"id":"sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"}' \
--action '{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}' \
--action-match any
--condition '{"type":"first_seen_event","comparison":true,"conditionResult":true}' \
--action '{"type":"email","data":{},"config":{"targetType":"team","targetIdentifier":"1"}}'
```

### `sentry alert issues delete <org/project/rule-id-or-name>`
Expand Down Expand Up @@ -101,11 +99,10 @@ Edit an issue alert rule
- `--status <value> - Rule status: active or disabled`
- `-c, --condition <value>... - Condition object JSON (repeatable, or pass one JSON array)`
- `-a, --action <value>... - Action object JSON (repeatable, or pass one JSON array)`
- `-m, --action-match <value> - Condition/action match mode: all or any`
- `--frequency <value> - Frequency in minutes`
- `--environment <value> - Environment value (pass empty string to clear)`
- `--filter <value>... - Filter object JSON (repeatable, or pass one JSON array)`
- `--filter-match <value> - Filter match mode: all or any`
- `-m, --filter-match <value> - Filter match mode: all or any`
- `--owner <value> - Owner value (pass empty string to clear)`

**Examples:**
Expand Down
88 changes: 47 additions & 41 deletions packages/cli/src/commands/alert/issues/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,31 @@
*/

import type { SentryContext } from "../../../context.js";
import { createIssueAlertRule } from "../../../lib/api-client.js";
import {
createIssueAlertRule,
resolveErrorDetectorId,
} from "../../../lib/api-client.js";
import { parseOrgProjectArg } from "../../../lib/arg-parsing.js";
import { buildCommand, numberParser } from "../../../lib/command.js";
import { ContextError, ValidationError } from "../../../lib/errors.js";
import { CommandOutput } from "../../../lib/formatters/output.js";
import { DRY_RUN_ALIASES, DRY_RUN_FLAG } from "../../../lib/mutate-command.js";
import { resolveTargetsFromParsedArg } from "../../../lib/resolve-target.js";
import {
matchToLogicType,
parseJsonObjectList,
parseMatchMode,
triggerLogicType,
validateIssueRuleArrays,
} from "../mutation-utils.js";

const USAGE_HINT =
"sentry alert issues create <target> --name <name> --condition <json> --action <json> --action-match all|any";
"sentry alert issues create <target> --name <name> --condition <json> --action <json>";

type CreateFlags = {
readonly name: string;
readonly condition?: string[];
readonly action?: string[];
readonly "action-match"?: "all" | "any";
readonly frequency: number;
readonly environment?: string;
readonly filter?: string[];
Expand Down Expand Up @@ -61,18 +65,25 @@ export const createCommand = buildCommand({
"<org>/<project>, an auto-detected project, or a bare project search when " +
"it resolves to exactly one project.\n\n" +
"Required fields:\n" +
" --name, --condition (>=1), --action (>=1), --action-match all|any\n\n" +
" --name, --condition (>=1), --action (>=1)\n\n" +
"Optional fields:\n" +
" --frequency, --environment, --filter, --filter-match, --owner\n\n" +
"Conditions and actions are workflow-native JSON (this targets the\n" +
"org-scoped workflows endpoint):\n" +
" --condition a trigger data-condition: {type, comparison, conditionResult}\n" +
" --action an action: {type, data, config}\n" +
" --filter an action-filter condition (same shape as --condition)\n\n" +
"Match mode: --filter-match all|any controls how the action-filter\n" +
"conditions combine. Issue-alert triggers always evaluate as 'any-short'\n" +
"(they fire on a single error detector), so there is no trigger match flag.\n\n" +
"Examples:\n" +
" sentry alert issues create my-org/my-app --name 'Error Spike' \\\n" +
' --condition \'{"id":"sentry.rules.conditions.first_seen_event.FirstSeenEventCondition"}\' \\\n' +
' --action \'{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}\' \\\n' +
" --action-match any\n\n" +
" sentry alert issues create my-org/my-app --name 'Prod Errors' \\\n" +
' --condition \'[{"id":"sentry.rules.conditions.every_event.EveryEventCondition"}]\' \\\n' +
' --action \'[{"id":"sentry.mail.actions.NotifyEmailAction","targetType":"Team","targetIdentifier":1}]\' \\\n' +
" --action-match all --frequency 30 --dry-run",
" sentry alert issues create my-org/my-app --name 'New Issues' \\\n" +
' --condition \'{"type":"first_seen_event","comparison":true,"conditionResult":true}\' \\\n' +
' --action \'{"type":"email","data":{},"config":{"targetType":"team","targetIdentifier":"1"}}\'\n\n' +
" sentry alert issues create my-org/my-app --name 'High Priority' \\\n" +
' --condition \'{"type":"new_high_priority_issue","comparison":true,"conditionResult":true}\' \\\n' +
' --action \'{"type":"email","data":{},"config":{"targetType":"user","targetIdentifier":"56789"}}\' \\\n' +
" --frequency 30 --dry-run",
},
output: {
human: formatCreated,
Expand Down Expand Up @@ -111,12 +122,6 @@ export const createCommand = buildCommand({
optional: true,
brief: "Action object JSON (repeatable, or pass one JSON array)",
},
"action-match": {
kind: "parsed",
parse: (value: string) => parseMatchMode(value, "action-match"),
optional: true,
brief: "Condition/action match mode: all or any",
},
frequency: {
kind: "parsed",
parse: numberParser,
Expand Down Expand Up @@ -154,7 +159,7 @@ export const createCommand = buildCommand({
...DRY_RUN_ALIASES,
c: "condition",
a: "action",
m: "action-match",
m: "filter-match",
},
},
async *func(
Expand All @@ -172,12 +177,6 @@ export const createCommand = buildCommand({
"frequency"
);
}
if (!flags["action-match"]) {
throw new ValidationError(
"Pass --action-match with one of: all, any.",
"action-match"
);
}

const conditions = parseJsonObjectList(flags.condition, "condition");
const actions = parseJsonObjectList(flags.action, "action");
Expand All @@ -201,22 +200,31 @@ export const createCommand = buildCommand({
}
const target = targets[0] as (typeof targets)[number];

// Issue alerts fire on a project's error detector; connect via detector_ids.
const detectorId = await resolveErrorDetectorId(target.org, target.project);

// Assemble the workflow-shaped body. The user supplies workflow-native
// conditions/actions; the CLI only builds the envelope (triggers +
// action_filters + logic types), mirroring the backend dual-write mapping.
const body: Record<string, unknown> = {
name: flags.name,
conditions,
actions,
actionMatch: flags["action-match"],
frequency: flags.frequency,
detectorIds: [detectorId],
config: { frequency: flags.frequency },
triggers: {
logicType: triggerLogicType(),
conditions,
},
Comment thread
jared-outpost[bot] marked this conversation as resolved.
actionFilters: [
{
logicType: matchToLogicType(flags["filter-match"]),
conditions: filters ?? [],
actions,
},
],
};
if (flags.environment !== undefined) {
body.environment = flags.environment;
}
if (filters && filters.length > 0) {
body.filters = filters;
body.filterMatch = flags["filter-match"] ?? "all";
} else if (flags["filter-match"] !== undefined) {
body.filterMatch = flags["filter-match"];
}
if (flags.owner !== undefined) {
body.owner = flags.owner;
}
Expand All @@ -232,17 +240,15 @@ export const createCommand = buildCommand({
return { hint: "Dry run - no issue alert rule was created." };
}

const created = await createIssueAlertRule(
target.org,
target.project,
body
);
const created = await createIssueAlertRule(target.org, body);
yield new CommandOutput({
org: target.org,
project: target.project,
id: String(created.id ?? ""),
name: String(created.name ?? flags.name),
status: String(created.status ?? "active"),
status: String(
created.status ?? (created.enabled === false ? "disabled" : "active")
),
} satisfies CreateResult);
},
});
86 changes: 51 additions & 35 deletions packages/cli/src/commands/alert/issues/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,20 @@

import type { SentryContext } from "../../../context.js";
import {
getIssueAlertRuleDocument,
putIssueAlertRule,
getIssueAlertWorkflowDocument,
updateIssueAlertRule,
} from "../../../lib/api-client.js";
import { parseOrgProjectArg } from "../../../lib/arg-parsing.js";
import { buildCommand, numberParser } from "../../../lib/command.js";
import { ContextError, ValidationError } from "../../../lib/errors.js";
import { CommandOutput } from "../../../lib/formatters/output.js";
import { resolveTargetsFromParsedArg } from "../../../lib/resolve-target.js";
import {
matchToLogicType,
parseJsonObjectList,
parseMatchMode,
parseStatusFlag,
triggerLogicType,
validateIssueRuleArrays,
} from "../mutation-utils.js";
import { parseIssueRuleArg, resolveIssueAlertRule } from "./rule-resolve.js";
Expand All @@ -31,7 +33,6 @@ type EditFlags = {
readonly status?: "active" | "disabled" | undefined;
readonly condition?: string[];
readonly action?: string[];
readonly "action-match"?: "all" | "any";
readonly frequency?: number;
readonly environment?: string;
readonly filter?: string[];
Expand All @@ -53,7 +54,6 @@ function hasIssueMutations(flags: EditFlags): boolean {
flags.status !== undefined ||
flags.condition !== undefined ||
flags.action !== undefined ||
flags["action-match"] !== undefined ||
flags.frequency !== undefined ||
flags.environment !== undefined ||
flags.filter !== undefined ||
Expand Down Expand Up @@ -87,34 +87,56 @@ function applyIssueEdits(
body.name = flags.name;
}
if (flags.status !== undefined) {
body.status = flags.status;
}
if (conditions !== undefined) {
body.conditions = conditions;
}
if (actions !== undefined) {
body.actions = actions;
}
if (flags["action-match"] !== undefined) {
body.actionMatch = flags["action-match"];
// Workflows use an `enabled` boolean rather than a status string.
body.enabled = flags.status === "active";
}
if (flags.frequency !== undefined) {
body.frequency = flags.frequency;
const config = (body.config as Record<string, unknown> | undefined) ?? {};
config.frequency = flags.frequency;
body.config = config;
}
if (flags.environment !== undefined) {
body.environment =
flags.environment.trim() === "" ? null : flags.environment;
}
if (filters !== undefined) {
body.filters = filters;
}
if (flags["filter-match"] !== undefined) {
body.filterMatch = flags["filter-match"];
}
if (flags.owner !== undefined) {
body.owner = flags.owner.trim() === "" ? null : flags.owner;
}

// Triggers: the "when" data-condition group. Issue-alert triggers always use
// the 'any-short' logic type (see triggerLogicType), so we pin it whenever the
// conditions change rather than exposing a trigger match flag.
if (conditions !== undefined) {
const triggers =
(body.triggers as Record<string, unknown> | undefined) ?? {};
triggers.conditions = conditions;
triggers.logicType = triggerLogicType();
body.triggers = triggers;
}

// Action filter: the "if" group plus its actions. Issue alerts use one filter.
if (
actions !== undefined ||
filters !== undefined ||
flags["filter-match"] !== undefined
) {
const actionFilters = Array.isArray(body.actionFilters)
? (body.actionFilters as Record<string, unknown>[])
: [];
const filter = (actionFilters[0] as Record<string, unknown>) ?? {};
Comment on lines +123 to +126

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Partially editing an issue alert can overwrite and destroy the existing filter configuration if the API response for the alert rule is missing the actionFilters array.
Severity: HIGH

Suggested Fix

Before mutating actionFilters, validate that it was successfully loaded from the document returned by getIssueAlertWorkflowDocument. If actionFilters is missing from the API response, ensure the user's partial edits are merged with the existing state rather than creating a new, incomplete object. This will prevent accidental data loss during partial updates.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: packages/cli/src/commands/alert/issues/edit.ts#L123-L126

Potential issue: When a partial edit is applied to an issue alert, such as using only
the `--action` flag, the code may construct a new, incomplete filter object. This occurs
if the response from `getIssueAlertWorkflowDocument` does not include an `actionFilters`
array. The code at lines 123-126 defaults to an empty object (`{}`) in this scenario. A
subsequent PUT request then replaces the entire existing action filter with this
incomplete object, silently removing other configured properties like `logicType` and
`conditions` from the alert rule, effectively destroying the filter configuration.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jared, follow up with this

if (actions !== undefined) {
filter.actions = actions;
}
if (filters !== undefined) {
filter.conditions = filters;
}
if (flags["filter-match"] !== undefined) {
filter.logicType = matchToLogicType(flags["filter-match"]);
}
actionFilters[0] = filter;
body.actionFilters = actionFilters;
}

if (conditions !== undefined) {
validateIssueRuleArrays(conditions, actions, "conditions");
}
Expand Down Expand Up @@ -183,12 +205,6 @@ export const editCommand = buildCommand({
optional: true,
brief: "Action object JSON (repeatable, or pass one JSON array)",
},
"action-match": {
kind: "parsed",
parse: (value: string) => parseMatchMode(value, "action-match"),
optional: true,
brief: "Condition/action match mode: all or any",
},
frequency: {
kind: "parsed",
parse: numberParser,
Expand Down Expand Up @@ -224,7 +240,7 @@ export const editCommand = buildCommand({
aliases: {
c: "condition",
a: "action",
m: "action-match",
m: "filter-match",
},
},
async *func(this: SentryContext, flags: EditFlags, arg: string) {
Expand All @@ -248,21 +264,21 @@ export const editCommand = buildCommand({
);

const body = {
...(await getIssueAlertRuleDocument(target.org, target.project, rule.id)),
...(await getIssueAlertWorkflowDocument(target.org, rule.id)),
} as Record<string, unknown>;
applyIssueEdits(body, flags);

const updated = await putIssueAlertRule(
target.org,
target.project,
rule.id,
body
);
const updated = await updateIssueAlertRule(target.org, rule.id, body);
Comment thread
BYK marked this conversation as resolved.
yield new CommandOutput({
...updated,
org: target.org,
project: target.project,
id: String(updated.id ?? rule.id),
// The workflows endpoint returns `enabled` rather than `status`; map it
// back to a status label so human/JSON output matches the create path.
status: String(
updated.status ?? (updated.enabled === false ? "disabled" : "active")
),
} satisfies EditResult);
},
});
Loading
Loading