Skip to content

ref(cli): Parse every kind of ID argument with one grammar - #1714

Merged
BYK merged 13 commits into
mainfrom
byk/cli-bound-arguments
Oct 29, 2026
Merged

BYK merged 13 commits into
mainfrom
byk/cli-bound-arguments

Conversation

@sentry-junior

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

Copy link
Copy Markdown
Contributor

Follow-up to #1655 for #1564. Some positionals mean different things depending on how many arguments were given, like [<org>/<project>] <replay-id> or log list's optional trailing trace ID. Those kept parsing in func, because a Stricli parse only sees one argument at a time. Each ID-taking command also had its own rules for them.

The new parser type: boundArguments (src/lib/positional-parse.ts). It's an array positional with a pure parse over all the arguments, bound to a resolve that runs after the auth guard, the same contract as boundPositional. func takes one BoundArguments<Parsed, Resolved>, reads .parsed and awaits .resolved. Each argument still completes as its completion kind.

No Stricli patch. buildCommand already runs applyBoundPositionals before func, so boundArguments positionals are parsed there for every caller, CLI and SDK alike.

  • A parse error becomes Stricli's own ArgumentParseError. getExitCode maps it to InvalidArgument, and exceptionWhileRunningCommand formats it as an argument error. It still prints Failed to parse "…" for <placeholder> and exits 252, before the auth guard, just like a scanner error.
  • If an argument is help, the error is thrown inside the wrapper's try instead, so sentry replay view help still prints the command's help.
  • An earlier revision added a combine hook to our @stricli/core patch. I've removed it and proposed the hook upstream instead. If it lands, the parse can move into Stricli's scanner.

One grammar for every kind of ID (grammar/id-arguments.pegjs, src/lib/id-arguments.ts). The grammar doesn't know about any kind of ID. It has one start rule and only decides which token is the target and which are IDs. The command passes its kind from ID_KINDS (trace, replay, log, span): what an ID looks like, how to validate it, and which part of a Sentry URL holds one. A bare 32-hex value never tells the parser which kind it is. Errors come from parseIdArguments, never from Peggy.

Every ID-taking command now accepts the same forms. Each one takes the friendliest behavior any command had before:

  • <id>, <org>/<id>, <org>/<project>/<id>, <target> <id>, or a URL with the command's kind of ID.
  • <id> <org>/<project> is read as reversed, with a warning. Before, only trace view, replay and log view did this.
  • With multiple, more IDs can follow any of those, and each can repeat the target (<target>/<id>). Before, only log view did this.
  • One argument can hold several lines, or IDs after a space. This replaces span view's whitespace split and log view's newline split.
  • Extra arguments are ignored with a warning, and the first real ID is kept. Replay used to reject them.
  • Slashes pasted after an ID are ignored. An empty org or project in a path is an error.

Three options cover the differences between commands that are real:

  • optional: a command can run without an ID, and a lone argument is the target. Used by log list and span list (a project's items or a trace's), and by span view, where the target of the span IDs is the trace.
  • needsProject: <x>/<id> means project <x> instead of org <x>. Used by commands that need a project: log view, span view and span list. log view already read it that way.
  • validate: false: IDs come back raw, so the command can recover a malformed ID once it knows the target. Used by log view and span view. Trace recovery uses it too, instead of working out again which argument held the ID.

Keeping it consistent: the new sentry-cli/no-ad-hoc-id-arguments lint rule rejects hand-written ID parsing in commands (parseSlashSeparatedArg, detectSwappedViewArgs, HEX_ID_RE, SPAN_ID_RE). It also rejects a parseIdArguments call whose kind isn't from ID_KINDS. event view is exempt because it also takes issue IDs, which keep their own syntax. The grammar tests run one table against every kind, so the commands can't drift apart.

Migrated:

  • With boundArguments (parsed before the command runs): log list, replay view, replay download, trace view, span view and log view.
  • With the shared parser, still parsing in func: trace logs and span list. All of them lose their own splitting, reversed-argument detection and full-path handling. trace view keeps only its issue-short-ID shortcut.
  • trace view, span view and log view recover malformed IDs, which needs the API. Their parse keeps IDs as typed, and resolve validates or recovers them. parseTraceTargetWithRecovery is split into a pure parseTraceArgumentsInput and an async resolveTraceArguments.
  • Parsing a pasted Sentry URL no longer applies its server. The parse returns baseUrl and warnings, and resolve applies them.
  • Malformed input now exits 252 in the migrated boundArguments commands: a bad replay ID, or a replay URL without an ID. In trace view, span view and log view, a missing ID or a bad target exits 252 before auth or any lookup; a malformed ID still goes through recovery.
  • SDK callers get an ArgumentParseError whose .exception is the parser's error, where they used to get that error directly.
  • replay download gets org/project completion, which replay view already had.

Behavior changes, all for input that used to fail, warn, or be ignored:

  • replay view <target> <id> extra warns and ignores extra instead of failing.
  • log list <project> not-a-trace-id and span list <project> not-a-trace-id fail with "Invalid trace ID" instead of silently listing the project.
  • span list <project>/<trace-id> and span view <project>/<trace-id> <span-id> look up project <project> instead of failing with "Specific project required".
  • span view org/project/not-a-trace/<span-id> goes through trace-ID recovery, like trace view, instead of failing right away.
  • <org>/<partial-id> (at least 8 hex chars, an ellipsized ID, or junk after an ID) is read as an ID to recover, as before. <org>/<project> is still a target missing its ID.

Subcommand-name interception (sentry log list view …): skipLeadingArgument drops the subcommand name before the arguments are parsed, so it can't change what the other arguments mean. buildListCommand then only prints the tip. This fixes both Bugbot findings.

Next: trace logs and span list can move onto boundArguments too. Issue IDs and event view stay separate. The list-target positional for dispatch-based lists is in #1711.

Verification (Node 24):

  • tsc, lint (including the new rule), format, ast-grep and the lint-rule tests pass.
  • CLI unit suite: 11,749 passed. The 7 failures are sandbox-only (permission tests and delta-upgrade) and also fail on main.
  • Smoke-tested on a bundled build:
    • Exit 252: replay view not-hex, replay view test-org/cli ("Replay ID is required"), a trace URL passed to replay view (missing replay ID, with an example replay URL), and log list web not-hex.
    • span view <span-id>, span view org/project/<span-id> and span view <trace-id> show the targeted "trace ID and span ID", "trace ID" and "span ID" required errors.
    • log view a/b/<id> c/d/<id> reports IDs from different projects.
  • The trace view / span view / log view migration (9a899d4) was checked with tsc, lint, format, the lint-rule tests, and the command and lib unit suites (failures only in sandbox-only permission, SQLite and subprocess tests). The generated SDK is unchanged. It has not been smoke-tested on a bundled build.

via BYK.

--

View Junior Session in Sentry

Follow-up to #1655 for #1564. Positionals whose meaning depends on how
many arguments were given (`[<org>/<project>] <replay-id>`, `log list`'s
trailing trace ID) kept parsing in `func`, because a per-element Stricli
`parse` can't see the count.

Our `@stricli/core` patch now lets an array positional declare
`combine`: it gets every parsed element and returns the one value the
command receives. A throw becomes an `ArgumentParseError`, so these
commands fail in the scanner with exit 252 like other arguments.
`boundArguments` (`src/lib/positional-parse.ts`) builds such a
positional: a pure `parse` over all the arguments, bound to a `resolve`
that runs after the auth guard, like `boundPositional`.

`log list`, `replay view` and `replay download` move onto it. Parsing a
pasted Sentry URL no longer applies its server; the parse returns
`baseUrl` (and trace-target warnings), and `resolve` applies them.
`parseTraceTargetInput` and `parseDualModeInput` are the side-effect-free
trace parsers; the existing ones wrap them, so span and trace commands
are unchanged and can migrate onto the same primitive next.

A parse error next to a `help` argument is deferred to `func`, so
`sentry replay view help` still shows the command's help. Plural alias
interception drops the first argument and parses the rest again.

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

vercel Bot commented Oct 29, 2026 •

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 5:28pm UTC

Request Review

@cursor cursor Bot left a comment •

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.

Stale Bugbot comment from a previous run.

Comment thread packages/cli/src/commands/log/list.ts
Comment thread packages/cli/src/lib/list-command.ts Outdated
Addresses Bugbot on #1714. `buildListCommand` dropped a subcommand name
(`sentry log list view …`) in `func` and parsed the other arguments
again, after the scanner had already parsed all of them:

- `log list view acme/web <trace-id>` failed in the scanner (exit 252),
  because "acme/web" was read as the trace ID, so the intercept never ran
- a parse error deferred next to `help` in the second parse was never
  thrown, so `func` read an undefined parse

`skipLeadingArgument` now wraps a `boundArguments` positional so the
name is left out in the scanner, before it changes what the other
arguments mean. `func` only prints the hint.
Drops the array positional `combine` hook from our `@stricli/core` patch.
`buildCommand` already runs `applyBoundPositionals` before `func`, so it
now parses `boundArguments` positionals there for every caller, CLI and
SDK alike.

A parse error becomes Stricli's own `ArgumentParseError`. `getExitCode`
maps it to `InvalidArgument` and `exceptionWhileRunningCommand` formats
it as an argument error, so it still prints `Failed to parse "…" for
<placeholder>` and exits 252, before the auth guard. Next to a `help`
argument it is thrown inside the wrapper's try instead, so the help
recovery catches it. That replaces the deferred-error bookkeeping.

The patch, its `check:patches` markers and the lockfile hash are back to
main's.

Co-Authored-By: BYK <burak.kaya@sentry.io>
@sentry-junior sentry-junior Bot changed the title ref(cli): Parse arity-dependent arguments in the scanner ref(cli): Parse arity-dependent arguments before the command runs Oct 29, 2026
The TypeScript that decided which argument is the ID and which is its
target (`replay view`'s `parsePositionalArgs`, `parseTraceTargetInput`,
`parseSlashSeparatedTraceTarget`, `parseDualModeInput`) counted
arguments and split on slashes by hand. `grammar/id-arguments.pegjs`
now does that, over every argument at once: each one, trimmed, follows
a NUL, so arity is part of the input.

Its start rules are `trace`, `tracePath`, `traceOrProject` (log list,
span list) and `replay`. They return what the arguments name
(`IdArguments`: the ID, its org/project scope, a URL's server, and
warnings), and callers map that onto their own target types. ID checks
and errors go through hooks, so messages and error classes match what
they were.

`generate-parsers.mjs` compiles the `startRule` literals of a grammar's
declarations as its allowed start rules.

Behavior changes, all for input that used to fail or misparse:
- `log list`/`span list`: trace mode and the trace ID now come from the
  same argument, so `<target> <extra> <trace-id>` warns about `<extra>`
  instead of rejecting it as the trace ID.
- `replay`: a slash after the replay ID and a leading `/` are accepted,
  like for trace IDs.

Co-Authored-By: BYK <burak.kaya@sentry.io>
Comment thread packages/cli/grammar/id-arguments.pegjs Outdated
Comment thread packages/cli/grammar/id-arguments.pegjs Outdated
Addresses Warden on #1714.

- A first argument that ends in a trace ID is the trace target, even
  when a second trace ID follows. `trace` and `traceOrProject` used to
  read it as the target of the second ID, so `trace logs acme/<id> <id2>`
  used `<id2>`. The later arguments are now ignored with a warning, as
  after any other form.
- `<org>//<id>` and `/<project>/<id>` have an ID; an empty org or project
  segment is a `ValidationError` ("Invalid format"), not a missing ID.
Comment thread packages/cli/grammar/id-arguments.pegjs Outdated
Addresses Warden on #1714. `replay view "" <replay-id>` (an unset
`$ORG`, say) failed with "Replay ID is required": the empty-first-argument
rule also matched when a replay ID followed. Only an empty single argument
is a missing ID now; an empty target auto-detects, as for trace IDs.
The ID-arguments grammar no longer knows trace or replay IDs. It has one
start rule, and the command says what its IDs look like through an
ID_KINDS entry (trace, replay, log, span): the predicate, the validator,
and which part of a Sentry URL holds one. A bare 32-hex value never
tells the parser its kind.

Every ID-taking command now takes the same forms: `<id>`, `<org>/<id>`,
`<org>/<project>/<id>`, `<target> <id>`, `<id> <org>/<project>`
(reversed, with a warning), a URL, and with `multiple`, more IDs that
may repeat the target. One argument may hold several lines, or IDs
after a space. Extra arguments are ignored with a warning, keeping the
first real ID. `optional` covers `log list` and `span list`;
`needsProject` makes `<x>/<id>` name project `<x>` for commands that
need one; `validate: false` leaves malformed IDs for recovery.

Migrated span view and log view as well. They drop their hand-written
splitting, swap detection, and full-path handling. Trace view drops its
swap detection, and trace recovery reuses the raw parse instead of
re-deriving the ID's slot.

A new sentry-cli/no-ad-hoc-id-arguments lint rule rejects ID parsing by
hand in commands (parseSlashSeparatedArg, detectSwappedViewArgs,
HEX_ID_RE, SPAN_ID_RE), with event view allowed since it takes issue
IDs, and rejects ID kinds defined outside ID_KINDS. The grammar tests
run one table against every kind.

Co-Authored-By: BYK <burak.kaya@sentry.io>
@sentry-junior sentry-junior Bot changed the title ref(cli): Parse arity-dependent arguments before the command runs ref(cli): Parse every kind of ID argument with one grammar Oct 29, 2026

@cursor cursor Bot left a comment •

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.

Stale Bugbot comment from a previous run.

Comment thread packages/cli/grammar/id-arguments.pegjs
`<org>/<id>` with a truncated, ellipsized, or junk-suffixed ID parsed
as an org/project target with no ID, so trace commands said the trace
ID was required instead of recovering it from the prefix.

Read the last segment as an ID when it has enough hex for recovery to
look up (the same check recovery uses) and is not a slug or a shell
sentinel. `<org>/<project>` is still a target without an ID, and so is
any token when IDs are optional.

@cursor cursor Bot left a comment •

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.

Stale Bugbot comment from a previous run.

Comment thread packages/cli/src/lib/hex-id-recovery.ts
`trace view`, `span view` and `log view` were the last ID-taking
commands parsing their positionals in `func`. They now use
`boundArguments`, like `replay view` and `log list`: the parse runs in
the argument scanner with no side effects, so a missing ID or a bad
target fails like any other argument (exit 252) before auth or any
lookup, and `func` awaits `target.resolved`.

These commands recover malformed IDs, which needs the API, so the
parse keeps IDs as typed and `resolve` validates or recovers them.
`parseTraceTargetWithRecovery` splits into the two halves:
`parseTraceArgumentsInput` (pure) and `resolveTraceArguments` (applies
a pasted URL's server and warnings, then validates or recovers the
trace ID). `log view` parses its target with `parseOrgProjectInput` and
applies the server when it resolves.

Co-Authored-By: BYK <burak.kaya@sentry.io>
`<org>/trace-<id>` (an ID copied with its URL fragment prefix) was read
as `<org>/<project>`, because the slug check saw the prefix's dash and
letters. Strip the `trace-`, `span-`, `event-` and `txn-` prefixes
before deciding the segment is a slug, as recovery does, so it reaches
recovery like a bare `trace-<id>`.

@cursor cursor Bot left a comment

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.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 099a650. Configure here.

Comment thread packages/cli/src/lib/hex-id-recovery.ts Outdated
Recovery's slug check ran on the input with its URL fragment prefix, so
`trace-<prefix>` or `span-<a>...<b>` failed as "looks like a project
slug" and never reached the fuzzy lookup, though `<org>/trace-<prefix>`
now gets to recovery. Share the prefix-stripping slug check between the
grammar's ID hint and recovery. `span-my-app` is still a slug.
…operty

`<org>/<x>` is now read as a log ID to recover when `x` has enough hex
for a fuzzy lookup, so a generated project slug like `aa000a00` no
longer makes the one-slash property throw ContextError. Generate only
project slugs that cannot be a partial ID there, and add a property
that `<org>/<partial-id>` parses as org target plus raw log ID.
@BYK
BYK merged commit 63e2ffc into main Oct 29, 2026
35 checks passed
@BYK
BYK deleted the byk/cli-bound-arguments branch October 29, 2026 18:13
BYK added a commit that referenced this pull request Oct 29, 2026
…1728)

Part of #1564. Follow-up to #1714, which left `trace logs` and `span
list` on the shared ID grammar but still parsing inside `func`.

Both now declare their positional with `boundArguments`, like `trace
view`, `span view` and `log list`:

- **`trace logs`** parses with `parseTraceTargetInput` (pure). Its
resolver applies a pasted URL's server and the warnings, then resolves
the org with `resolveTraceOrg`; an explicit project still becomes the
`project:` filter.
- **`span list`** parses with `parseDualModeInput(..., { needsProject:
true })`, plus `parseOrgProjectInput` in project mode, the same shape as
`log list`. The resolver applies the URL context and resolves the
project (`resolveProjectBoundTarget`) or the trace's org and project
(`resolveTraceOrgProject`). The mode handlers now take resolved values
instead of resolving themselves.
- `parseTraceTarget` and `parseDualModeArgs` had no callers left, so
they're removed. Their tests now go through `parseTraceTargetInput` /
`parseDualModeInput`, keeping the same coverage.
- `SpanListArguments` and `TraceLogsArguments` are added to the
`require-target-resolution` lint rule's type list.

**Behavior changes** (input that already failed):
- A missing or invalid trace ID, or a bad target, now fails before the
auth guard as an argument error: `Failed to parse "…" for
org/project/trace-id: …`, exit 252. Before, it failed inside `func`. For
example, `trace logs not-hex`, `trace logs my-org/web`, `span list
my-org/web not-hex`.
- SDK callers get an `ArgumentParseError` whose `.exception` is the
parser's error, as with the other `boundArguments` commands.
- `sentry trace logs help` still shows the command's help, now through
the parse-error path (a new test covers it).

Resolution order, URL-context timing (applied in the resolver, after
auth), help text and completion kind are otherwise unchanged.

**Tests:** new `parseSpanListInput` tables (including `<x>/<trace-id>` →
project `<x>` and a URL's `baseUrl` not being applied), `trace logs`
parse failures before any lookup, and `trace logs help` in
`help-positional`. Project-mode tests now mock
`resolveProjectBoundTarget`.

**Verification:**
- `tsc`, CLI and repo lint (oxlint, oxfmt, ast-grep, lint-rule tests)
pass.
- CLI unit suite: 11,976 passed. The 6 failures are permission tests
that only fail in the sandbox (running as root).
- Smoke-tested with `pnpm run cli`: the three inputs above exit 252 with
the parse error, and `trace logs help` prints help.

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

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

--

[View Junior Session in
Sentry](https://sentry.sentry.io/explore/conversations/agent%3A6afff18d4507cd4bc63a317180ceb0f7/?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>
BYK added a commit that referenced this pull request Oct 29, 2026
…mmand runs (#1729)

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 #1711.
- `event view` is stacked on this PR.

<!-- 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>
BYK added a commit that referenced this pull request Oct 29, 2026
…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>
BYK added a commit that referenced this pull request Oct 29, 2026
…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>

This branch was previously deployed

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

Labels

risk: medium PR risk score: medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant