Skip to content

ref(cli): Parse issue arguments with their own grammar, before the command runs - #1729

Merged
BYK merged 1 commit into
mainfrom
byk/cli-issue-argument-grammar
Oct 29, 2026
Merged

ref(cli): Parse issue arguments with their own grammar, before the command runs#1729
BYK merged 1 commit into
mainfrom
byk/cli-issue-argument-grammar

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

Part of #1564, following #1714. Issue arguments (numeric IDs, short IDs, @ selectors, # references, issue URLs) get their own grammar. Commands that take one issue now parse it before func runs.

Why a separate grammar. Issue IDs aren't hex IDs, so they stay out of grammar/id-arguments.pegjs, which doesn't care what kind of ID it parses. The forms used to live in about 500 lines of hand-written branches in arg-parsing.ts. They are now:

  • grammar/issue-argument.pegjs lists every form and splits an argument into its parts. Like the ID grammar, it accepts any input and never throws: a malformed argument comes back as invalid with a reason.
  • parseIssueArgument (src/lib/issue-arguments.ts) validates and normalizes those parts into the same ParsedIssueArg as before. It has no side effects: it returns a pasted URL's server as baseUrl instead of applying it.
  • parseIssueArg keeps its behavior (it applies the URL's server) on top of parseIssueArgument. issue list, issue view, issue explain and issue merge still use it through resolveIssue.

The existing parseIssueArg suites (unit, property and URL-poisoning) pass unchanged, apart from where they import it from. test/lib/issue-arguments.test.ts adds the error reasons, the # forms, and a check that a pasted URL's server isn't applied at parse time.

Parse before run. src/commands/issue/positional.ts adds issuePositional({ command }) and issueParameter. Each is a boundPositional whose target.resolved calls resolveIssue. They're used by issue archive, events, plan, resolve, unresolve, tags, link, unlink and event list. feedback view, resolve, spam and unresolve get the same treatment through feedback/positional.ts. resolveIssue (and resolveFeedback) now accept the parsed argument as well as a raw string.

  • tags keeps the tag key and --environment in its recovery hints. plan, link and unlink need the stricter resolveOrgAndIssueId. So these commands pass requireIssueArgument(target.parsed, …) themselves instead of awaiting target.resolved.
  • event list still bails out when its plural-alias interception drops the positional.

Behavior changes (intentional):

  • A malformed issue argument in these commands now fails the parse, like other bound positionals: Failed to parse "…" for issue: …, exit 252, before the auth guard.
  • A pasted issue URL's server is applied when the issue resolves, not while parsing. An attacker host still fails in applySentryUrlContext before any request.

Not in this PR:

  • issue view, issue explain and issue merge take several issues and warn on the ones that fail, both bad input and failed lookups. Parsing them up front would turn one bad argument into a failure for the whole command, so they need their own boundArguments design.
  • issue list belongs to ref(cli): Parse list command targets in their positional #1711.
  • event view is stacked on this PR.

via BYK.

--

View Junior Session in Sentry

@vercel

vercel Bot commented Oct 29, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated
sentry-local Skipped Skipped Oct 29, 2026 7:10pm UTC

Request Review

@github-actions github-actions Bot added the risk: medium PR risk score: medium label Oct 29, 2026
@sentry-junior @BYK
ref(cli): Parse issue arguments with their own grammar, before the co…
a600175 
…mmand runs

Issue IDs, short IDs, selectors, and issue URLs keep their own syntax:
they are not hex IDs, so they stay out of the kind-agnostic ID grammar.
grammar/issue-argument.pegjs now defines their forms, and
parseIssueArgument (src/lib/issue-arguments.ts) validates and normalizes
the parts without side effects, returning a pasted URL's server instead
of applying it. parseIssueArg keeps its behavior on top of it.

Commands that take one issue (archive, events, plan, resolve, unresolve,
tags, link, unlink, event list) and the Feedback commands parse it in
their positional, so a malformed issue fails before the auth guard.
resolveIssue takes the parsed argument and applies the URL's server.

Part of #1564

Co-Authored-By: BYK <burak.kaya@sentry.io>

Co-Authored-By: BYK <burak.kaya@sentry.io>
@sentry-junior
sentry-junior Bot force-pushed the byk/cli-issue-argument-grammar branch from 1472687 to a600175 Compare October 29, 2026 19:10
@BYK
BYK merged commit 31529fd into main Oct 29, 2026
26 checks passed
@BYK
BYK deleted the byk/cli-issue-argument-grammar branch October 29, 2026 19:30
@sentry-junior sentry-junior Bot mentioned this pull request Oct 29, 2026
Merged
BYK added a commit that referenced this pull request Oct 29, 2026
@sentry-junior @BYK
ref(cli): Parse event view arguments with the shared ID grammar (#1733)
8c6d0de 
Part of #1564. Follows #1729.

`event view` was the last command that parsed IDs itself. It was exempt
from `sentry-cli/no-ad-hoc-id-arguments` because it also takes issues.
It now uses the shared grammar and parses its arguments before `func`
runs.

**How it parses now** (`parseEventViewInput`, wired with
`boundArguments`):

- Issue forms are checked first, using the same `looksLikeIssueShortId`
and numeric checks as before:
- `[<org>/]<short-id>`, `[<org>/]<numeric-id>`, and issue URLs without
an event show the issue's latest event.
- `<short-id>/<event-id>` and `[<org>/]<short-id> <event-id>` show that
event of the issue.
- Everything else goes through `parseIdArguments` with a new
`ID_KINDS.event` (`fromUrl` → `url.eventId`), using `multiple`,
`needsProject` and `validate: false`. A short ID or numeric ID in the
event-ID slot still redirects to that issue's latest event.
- Parsing has no side effects. `resolveEventViewInput` applies a pasted
URL's server, logs warnings (such as reversed arguments), and validates
or recovers the event ID, like `trace view` does.

**Removed:** `parseSingleArg`, `parseSingleSlashArg` and
`parsePositionalArgs`, plus the uses of `parseSlashSeparatedArg`,
`detectSwappedViewArgs` and `HEX_ID_RE`. `AD_HOC_ID_PARSING_ALLOWED` is
gone from the lint plugin, so the rule now covers every command.
`EventViewArguments` joins the `require-target-resolution` type list.

**Shared grammar change:** the reversed form now accepts more IDs after
it (`<id> <org>/<project> <id>...`). The grammar's header already says
more IDs may follow any form, and `event view` handled this before, so
the "collects extras when args are swapped" test keeps passing. Nothing
else in the grammar changes. There's a new case in
`test/lib/id-arguments.test.ts`.

**Kept behavior** (existing tests pass; only the parse function's name
changed):

- CLI-1HT newline expansion
- the swapped-argument warning
- rejecting a bare `latest`
- issue URL → latest event
- an explicit event ID with an issue short ID: it's validated and never
falls back to latest
- `org/SHORT-ID`, `SHORT-ID/EVENT-ID`, `project/EVENT-ID`,
`org/NUMERIC-ID`, and multiple event IDs

**Behavior changes (intentional):**

- Bad input such as `CAM-82X 95fd7f5a` now fails the parse like other
bound positionals: `Failed to parse … for org/project/event-id`, exit
252, before auth and before any request.
- A pasted URL's server is applied when the arguments resolve, not while
parsing. An attacker host still fails in `applySentryUrlContext` before
any request; `url-arg-poison.test.ts` now checks that through the
command.
- `<target> ""` is reported as a missing event ID, not an invalid empty
one.
- `<short-id> <org>/<project>` is read as reversed arguments: that
issue's latest event in that project, with a warning. It used to fail
validation as an event ID.
- `<org>/<project>/<short-id>` now shows that issue's latest event. It
used to fail as a malformed event ID.

**Open question:** issue detection still uses `looksLikeIssueShortId`,
not the #1729 issue grammar. That grammar accepts any token as a
short-ID suffix, so it can't tell an issue from an event ID; it only
parses an argument already known to be an issue.

**Tests:**

- new parse cases: reversed short ID, `org/project/SHORT-ID`, and a
self-hosted issue URL returning `baseUrl`
- a func test showing bad input fails before any API call
- `ID_KINDS.event` in the shared grammar table, plus an event-URL case

**Verification:** `tsc` and repo lint (oxlint, oxfmt, ast-grep) pass.
The command, security and parser suites pass: 3,633 tests. The full CLI
unit suite only fails the 6 permission tests that can't pass in the
sandbox, which runs as root.

<!-- junior-request-attribution:start -->
via **BYK**.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3Ab47afb146f99778033f77dec53fa2175 -->

--

[View Junior Session in
Sentry](https://sentry.sentry.io/explore/conversations/agent%3Ab47afb146f99778033f77dec53fa2175/?project=4510944073809921)

<!-- junior-session-footer:end -->

---------

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: BYK <burak.kaya@sentry.io>
@sentry-junior sentry-junior Bot mentioned this pull request Oct 29, 2026
Merged
6 of 13 tasks
BYK added a commit that referenced this pull request Oct 29, 2026
@sentry-junior @BYK
ref(cli): Ratchet hand-parsed positionals and test argument conventio…
cfb568c 
…ns (#1739)

Part of #1564.

Every command that still parses a positional inside `func` is now named
on a lint allow-list with its follow-up, so the rest can be migrated one
by one and no new one can appear. Release commands, the small straggler,
move to a bound positional. A new table-driven test checks that commands
taking IDs read the same inputs the same way.

## Lint

- `no-ad-hoc-id-arguments` now also rejects `parseIssueTarget` and
`parseDashboardTarget` in commands. These parsers only tag a positional
for completion and leave the parsing to `func`.
- `require-target-resolution` now also rejects `parseOrgTarget`,
`parseOrgProjectTarget`, `parseOrgProjectArg`, and the
`resolve{OrgOnly,OrgOptional,ProjectBound}FromArg` helpers in commands.
- Both rules use allow-lists (`AD_HOC_ID_PARSING_ALLOWED`,
`UNBOUND_TARGET_PARSING_ALLOWED`), and each entry names its follow-up.
The lists only shrink: an allow-listed file that stops using the names
gets "Remove this file from the allow-list".
- `event view` is no longer exempt, since #1733 merged.
- `ReleaseTarget` was added to `BOUND_TARGET_TYPES`.
- New rule tests are in `scripts/cli-lint-rules.test.mjs`.

## Release positional

`releaseParameter(USAGE_HINT, brief)` in `release/parse.ts` returns a
bound `ReleaseTarget`. `func` reads `{ version, org, detectedFrom }`
from `await target.resolved`. A blank version now fails the parse (exit
252) instead of failing inside `func`. Migrated commands: archive,
create, delete, deploy, deploys, finalize, restore, set-commits, view.

## Cross-command conventions test

`test/lib/argument-conventions.test.ts` runs the same inputs through
each command's own positional with `applyBoundPositionals`, the way
Stricli does. The inputs are:
- a bare ID
- `<x>/<id>`
- `<org>/<project>/<id>`
- `<org>/<project> <id>`
- reversed arguments (expects a warning)
- extra or multiple IDs
- a missing ID (expects `ArgumentParseError`)
- a pasted URL (its server must be applied only when `.resolved` runs)

Commands covered: trace view/logs, span view/list, log view/list, event
view, replay view/download, the single-issue commands (issue and
feedback), and all release commands. Where a command legitimately
differs, its row says how:
- `slashTarget: "project"` for span view, log view and event view
- `multiple` for span view, log view and event view
- `optional` for span list and log list
- `separateTarget: false` for span view
- no URL for log view

## Audit: positional parsing in `func`

Excludes `event view` (#1733, merged) and `dashboard/*` (#1730).

**Already on bound parsers (verified)**
- [x] `trace view`, `trace logs` (#1711, #1728)
- [x] `span view`, `span list` (#1711, #1728)
- [x] `log view`, `log list`: fully migrated in #1714. The remaining
`parseOrgProjectInput` call is inside the bound `parseLogListInput`, not
in `func`.
- [x] `replay view`, `replay download`: fully migrated in #1714
(`parseIdArgument` is called inside the bound parser).
- [x] Single-issue commands (`issuePositional`, #1729) and `event view`
(#1733)
- [x] `release *` (9 commands): **migrated in this PR**

**Allow-listed, still parsing in `func`**
- [ ] `issue view`, `issue explain` (`issue/utils.ts`), `issue merge`:
these take several issues and need an array issue positional (#1564)
- [ ] `alert issues create/delete/edit/view`:
`[<org>/[<project>/]]<rule>`, resolving to every matching project
(#1564: alert rule target)
- [ ] `alert metrics create/delete/edit/list/view`: `[<org>/]<rule>`,
org-scoped (#1564: alert rule target)
- [ ] `agent-conversation view`, `integration delete`: `[<org>/]<id>`
with a non-hex ID, the same shape as `releaseParameter` (#1564:
org-scoped ID)
- [ ] `org view`, `trial list`, `trial start`: a bare org positional
(#1564: org positional)
- [ ] `init`, `project create`: a target followed by other positionals
(#1564)
- [ ] `dashboard create`, `dashboard list/restore/revisions/view`,
`dashboard widget add/delete/edit`: covered by #1730. These entries will
fail the ratchet once #1730 lands, and whichever PR merges second
removes them.

**Not target parsing, and not covered by these rules**
- `monitor run`: `args[0]` is a monitor slug followed by a passthrough
command. This is arity handling, not target parsing.
- Paths, endpoints, versions, selectors and doc parts: `api`, `auth
login/switch`, `build/debug-files/snapshots upload`, `debug-files find`
(debug IDs), `docs *`, `help`, `local *`, `schema`, `cli upgrade`, `org
list`, `react-native xcode`, `wasm-split`.

## Checks

`pnpm run tsc` and `pnpm run lint` pass, and the lint-rule tests pass
(70/70). The new conventions test and the release and issue tests pass.
The only full-suite failures locally are sandbox noise: tests that need
to run without root, e2e and init-eval tests that need a build or keys,
and timing-sensitive tests.

<!-- junior-request-attribution:start -->
via **BYK**.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3Af950a2e3aa4d0471d8b1b07a64bfa60d -->

--

[View Junior Session in
Sentry](https://sentry.sentry.io/explore/conversations/agent%3Af950a2e3aa4d0471d8b1b07a64bfa60d/?project=4510944073809921)

<!-- junior-session-footer:end -->

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: BYK <burak.kaya@sentry.io>
sentry-junior Bot added a commit that referenced this pull request Oct 29, 2026
@sentry-junior @BYK
ref(cli): Parse dashboard and issue arguments with the shared ID grammar
72e999d 
`id-arguments.pegjs` now has kinds that name items without an ID
(`IdKind.names`): what is not, and may not be, an ID is a name. Dashboards
(`ID_KINDS.dashboard`) are a numeric ID or a title, which may hold spaces
and slashes and, after a target, takes the rest of the arguments. Issues
(`ID_KINDS.issue`) are an identifier, one whole token that
`issue-identifier.ts` takes apart with the #1729 grammar, or search text.
The dashboard grammar is gone.

The parser makes no decision about a bare word before a dashboard title:
it returns both readings. `resolveDashboardInput` checks the word as an
org (cache first, then the API) and, on a 404/403, falls back to the whole
input as the title in the detected org, with a warning; when that misses
too, one error names both readings. A server error propagates.

Search text for an issue resolves through the org's issue search: the one
issue it matches, or an error listing none or several. Trace, span, log,
and replay IDs fail on text with a hint to search for it, from `validate()`
so recovery comes first. `event view` tells issues from events with
`ID_KINDS.issue.distinctForm`, keeping #1738's order.

Co-Authored-By: BYK <burak.kaya@sentry.io>
@sentry-junior sentry-junior Bot mentioned this pull request Oct 29, 2026
Merged
BYK added a commit that referenced this pull request Oct 29, 2026
@sentry-junior @BYK
ref(cli): Parse dashboard and issue arguments with the shared ID gram…
2ad8e4b 
…mar (#1730)

Part of #1564. Dashboard commands parsed their positionals by hand
inside `func`, and issue commands used their own grammar (#1729). This
moves both onto the shared ID grammar (`grammar/id-arguments.pegjs`,
#1714), parsed before `func` runs. Event view's issue checks (#1733,
#1738) use the new issue kind too.

**Shared grammar: kinds with names.** `IdKind.names` adds one idea to
`id-arguments.pegjs`: a kind may name an item without its ID, and
anything that is not, and may not be, an ID is a name. Kinds without
names keep their grammar paths exactly as before. Kinds with names keep
each line whole (no splitting at spaces), so `Top 10` stays one title
and never becomes title `Top` + ID `10`.
- `ID_KINDS.dashboard`: a numeric ID or a title, which may hold spaces
and slashes (`Latency/p95`). `<org>/[<project>/]<digits>` reads as a
path only when slugs come before the digits, so `Errors.v2/123` and
`/123` are titles. After a target, the title takes the rest of the
arguments (`my-org/ My Dashboard`). A bare first word names an org
(`orgScoped`).
- `ID_KINDS.issue`: an identifier (numeric ID, short ID, selector, issue
URL) is one whole token (`wholeIds`). `src/lib/issue-identifier.ts`
takes it apart with the #1729 grammar, which is unchanged. Text with
spaces is a name, meaning a search.
- `dashboard-arguments.pegjs` is gone. `dashboard list` keeps its thin
`parseDashboardListArgs`, because a bare word there is a title filter
(see #1711).

**Bare first word: org or title?** For `dashboard view My Dashboard`,
the parser makes no decision. It returns the structural reading (org
`My`, title `Dashboard`) and the whole input as one title
(`nameWithTarget`, surfaced as `orgOrTitle`). `resolveDashboardInput`
decides after auth:
- First it checks whether `My` is an org: the cached org list and region
cache, then `GET /organizations/My/` only on a cache miss.
- If the org exists, the title lookup in that org is final, even when it
misses.
- On a 404 or 403 it warns ``No org 'My'; treating "My Dashboard" as the
dashboard title. Use my-org/ to name an org.`` and looks up "My
Dashboard" in the auto-detected org.
- If that misses too, one error names both readings (no org `My` to find
`Dashboard` in, and no `My Dashboard` in `<detected-org>`) and hints at
`<org>/ Dashboard` and at quoting.
- A 5xx or network error from the org check propagates, with no
fallback.
- It never falls back for `my-org/ …` (explicit org), a numeric ref, or
a URL.

**Issue search text.** `issue view "login timeout"` resolves through
`GET /organizations/{org}/issues/?query=…` in the detected org. One hit
is the issue. Zero or several hits raise a `ResolutionError` that lists
up to 5 candidates as runnable commands and points at `sentry issue list
<org>/ --query '…'`. I checked the endpoint against getsentry/sentry
(`organization_group_index.py`, `group_index/index.py`): `query` takes
free text, an exact event ID or short ID gets a fast lookup, and an
explicit query replaces the default `is:unresolved`.

**Search hints.** Text where a trace, span, log, or replay ID belongs
(`trace view login`) now fails with ``Not a trace ID: "login". To
search, try `sentry trace list --query 'login'`.``. The hint comes from
`validate()`, so commands that recover malformed IDs (`trace view`,
`span view`, `log view`) recover first and use the hint only as their
final error. Event IDs get no hint, because `event list` lists one
issue's events.

**event view.** `ID_KINDS.issue.distinctForm` (uppercase short ID or
numeric) replaces `looksLikeIssueShortId` + `isAllDigits` and keeps
#1738's order: `<org>/<short-id>`, then `<short-id>/<event-id>` (even an
all-digit event ID), then `<org>/<numeric-id>`. I added a regression
test for the all-digit event ID and one for that order.

**Behavior changes:**
- `dashboard view my-org My Dashboard` (unquoted title) looks up "My
Dashboard". Before, it used `My` and dropped `Dashboard`. The same goes
for `my-org/ My Dashboard` and `https://acme.sentry.io/ My Dashboard`.
- `dashboard view My Dashboard`: when there is no org `My` (404/403), it
falls back to the title "My Dashboard" in the detected org, with the
warning above. If neither reading finds a dashboard, one error names
both. Dashboard commands with one dashboard ref get this (`view`,
`restore`, `revisions`, `widget edit/delete`, and `widget add` after it
splits off its widget title).
- A numeric ref first is the dashboard, as with every ID kind:
`dashboard view 42 X`, `my-org/42 X`. Before, the first of two arguments
was always the target. After a target, a numeric ID keeps the rest as
extra (`my-org 123 extra` → ID 123, ignored `extra`) instead of joining
them into a title. Arguments after the ID go through the shared rules:
extra, reversed (`42 my-org/`), or the shared "reference different
projects" error when they name another target.
- A lone non-Sentry URL is no longer taken as a dashboard title.
- `issue view "login timeout"` (text with spaces) searches instead of
failing as a malformed identifier. Control characters and URLs are still
rejected.
- Trace/span/log/replay IDs that are plain text get a search hint
instead of "Invalid trace ID".
- Parse errors (no dashboard, an org URL without a dashboard, a
dashboard URL passed to `list`, an invalid target slug) fail before the
auth guard, as in the earlier commits on this branch. They print `Failed
to parse "…" for <placeholder>: …` and exit 252.
- `widget add/edit/delete`: an argument error comes before flag errors
like "Specify --index or --title". `widget add`'s positional completes
cached dashboards. `parseDashboardTarget` had no callers left and is
removed.

**Lint ratchet (#1739):** the dashboard entries are off both
allow-lists, since these commands now parse before `func`. The issue
entries (`issue/utils.ts`, `issue/merge.ts`, which need an array issue
positional) stay.

**Syncing:** this branch merges `main` (no rebase). 54ebfa2 and 7f52f19
merge #1729, #1732–#1734, #1736, #1738 and #1739.

Verification:
- `tsc`, `pnpm run lint` (oxlint, format, ast-grep) and
`scripts/cli-lint-rules.test.mjs` (70/70) pass.
- vitest on `test/lib`, `test/grammar` and the dashboard, issue, event,
trace, span, log, replay and feedback command tests: 10,158 passed. The
6 failures are the sandbox-only permission tests (completions
EACCES/EPERM, code-scanner, file-changes, telemetry readonly, upgrade
lock), which also fail on `main`. The full CLI suite shows the same 6,
plus e2e/eval files that need a build or credentials.
- New tests:
- The bare-org fallback: the org from cache and from the API (no
fallback), org missing then fallback hit with the warning, a 403, org
missing then fallback miss with the combined error, a 5xx (propagates,
no fallback), `my-org/ My Dashboard`, and a numeric ref.
  - Issue search with 0, 1 and several hits.
  - Search hints, including `validate: false` deferral.
  - #1738's event view order.
- Differential runs against this PR's previous parsers:
- Issue arguments: 8,575 cases. 1,388 former errors are now searches
(text with spaces), with 0 other differences.
- Dashboard view: 32,552 cases over 0–3 arguments. Every difference is
in one of the categories above: a numeric ID first (2,600), arguments
after a dashboard URL that name another target (688), a numeric ID after
a target with more words (475), and a lone non-Sentry URL (1).

<!-- junior-request-attribution:start -->
via **BYK**.
<!-- junior-request-attribution:end -->

<!-- junior-session-footer:start -->
<!-- junior-conversation-id:agent%3A5a5e7e4543124325a44d731c08dd6089 -->

--

[View Junior Session in
Sentry](https://sentry.sentry.io/explore/conversations/agent%3A5a5e7e4543124325a44d731c08dd6089/?project=4510944073809921)

<!-- junior-session-footer:end -->

---------

Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com>
Co-authored-by: BYK <burak.kaya@sentry.io>
@sentry-junior sentry-junior Bot mentioned this pull request Oct 30, 2026
Closed

This branch was previously deployed

1 inactive deployment
Preview — a6001750 Deployed Oct 29, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BYK BYK BYK approved these changes

Assignees

No one assigned

Labels

risk: medium PR risk score: medium

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@BYK