Repository navigation
ref(cli): Parse alert rule positionals before the command runs - #1745
Merged
Merged
Conversation
Move the nine allow-listed alert commands off in-func target parsing: - alert issues view/delete/edit and alert metrics view/delete/edit take their rule argument through issueRuleParameter/metricRuleParameter, a boundPositional over the existing parseIssueRuleArg/parseMetricRuleArg splitters. Rule names may contain spaces, so they keep their own syntax rather than an ID_KINDS entry. - alert issues create and alert metrics create use orgProjectTarget with their existing resolvers; alert metrics list uses listTarget. A pasted URL's server is applied on resolve, after the auth guard. Drop the commands from UNBOUND_TARGET_PARSING_ALLOWED, register AlertRuleArgument with require-target-resolution, and cover the commands in argument-conventions.test.ts. Part of #1564 Co-Authored-By: BYK <burak.kaya@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
BYK
approved these changes
Oct 29, 2026
BYK
enabled auto-merge (squash)
October 29, 2026 22:46
Resolve conflicts with #1746: UNBOUND_TARGET_PARSING_ALLOWED is now empty, so drop its allow-list and ratchet test cases (they pointed at alert/metrics/view.ts); keep both sides' argument-conventions tests. Co-Authored-By: BYK <burak.kaya@sentry.io>
UNBOUND_TARGET_PARSING_ALLOWED is empty now that every command parses its positionals before run. Delete it with ratchetedNames and allowedFollowUp, and have require-target-resolution report every banned reference in command files through bannedNames. Update AGENTS.md: neither positional rule has an allow-list. Part of #1564 Co-Authored-By: BYK <burak.kaya@sentry.io>
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #1564
Moves the nine allow-listed alert commands off in-
functarget parsing. Their positionals are now parsed before the command runs.Rule commands (
alert issues view/delete/edit,alert metrics view/delete/edit): these useissueRuleParameter/metricRuleParameter. Both wrap the newalertRuleParameter(src/commands/alert/arguments.ts), which is aboundPositionalover the existingparseIssueRuleArg/parseMetricRuleArgsplitters.functakes anAlertRuleArgumentand reads{ ref, target }fromawait rule.resolved. Rule lookup by ID or name is unchanged. No API calls or endpoints changed.I didn't add an
ID_KINDSentry. Rule names are free text and can contain spaces, while the ID grammar splits IDs at spaces. The rule part also comes after a 1- or 2-segment target, which isn't the[<org>/[<project>/]]<id>shape. The splitters already handle this syntax, so I kept them.Target-only commands:
alert issues createusesorgProjectTargetwith its resolver, which still requires exactly one project.alert metrics createusesorgProjectTargetwithresolveOrgOnlyTarget.alert metrics listuseslistTarget(ORG_PROJECT_TARGET). It keeps its own brief ("project ignored").Lint and docs:
UNBOUND_TARGET_PARSING_ALLOWED, then the list itself (see below).AlertRuleArgumenttoBOUND_TARGET_TYPESsorequire-target-resolutioncovers it.packages/cli/AGENTS.md.Tests:
argument-conventions.test.tschecks that each rule command parses42,my-org/web/High error rateandmy-org/High error ratethe same way.my-org/webfor issue rules,my-org/,a/b/c, blank) and target parsing for the create and list commands.commandPrefix, like Stricli does, andmetrics deletespies onresolveOrgOnlyTarget.Behavior changes
alert metrics view a/b/c, oralert issues view my-org/webwithout a rule) now fails in Stricli's argument scanner. Before, it failed insidefunc. The message gets the usualFailed to parse "<input>" for <placeholder>: …prefix and the exit code changes from 21 to 252. The error text is otherwise the same. It also fails before the auth guard now, like other migrated commands.Lint allow-lists
Drops the last allow-list, per review on #1747; closes out #1564's ratchet. Once #1746 landed,
UNBOUND_TARGET_PARSING_ALLOWEDheld only these nine alert commands. This PR deletes it, along withratchetedNamesandallowedFollowUp.require-target-resolutionnow usesbannedNames, so it reports every banned reference in command files and has no exemption path. The allow-list and ratchet lint-rule test cases are removed, andpackages/cli/AGENTS.mdnow says neither positional rule has an allow-list.Not in scope: alert rule URLs (
/alerts/rules/...,/monitors/...) were not accepted before and still aren't.sentry-url-parserhas no alert-rule paths. That could be a follow-up.Verification:
tsc,pnpm run lint, lint-rule tests (scripts/cli-lint-rules.test.mjs, 69/69 after the cleanup) and the alert and argument-conventions tests pass. Before the merge, the full CLI unit suite had 12593 passing tests and only 6 failures, all known root-permission sandbox tests (EACCES/readonly).via BYK.
--
View Junior Session in Sentry