Repository navigation
ref(cli): Parse trace logs and span list arguments before they run - #1728
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Move `trace logs` and `span list` onto `boundArguments`, like `trace view`, `span view` and `log list`. The parse is pure and runs before the auth guard; the resolver applies a pasted URL's server and the warnings, then resolves the org (and project). With no callers left, remove `parseTraceTarget` and `parseDualModeArgs`; tests use the side-effect-free `parseTraceTargetInput` and `parseDualModeInput`. Part of #1564. Co-Authored-By: BYK <burak.kaya@sentry.io>
BYK
approved these changes
Oct 29, 2026
BYK
enabled auto-merge (squash)
October 29, 2026 19:00
sentry-junior
Bot
force-pushed
the
junior/cli-trace-logs-span-list-bound
branch
from
October 29, 2026 19:02
184b84b to
e5211d8
Compare
6 of 13 tasks
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>
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. Follow-up to #1714, which left
trace logsandspan liston the shared ID grammar but still parsing insidefunc.Both now declare their positional with
boundArguments, liketrace view,span viewandlog list:trace logsparses withparseTraceTargetInput(pure). Its resolver applies a pasted URL's server and the warnings, then resolves the org withresolveTraceOrg; an explicit project still becomes theproject:filter.span listparses withparseDualModeInput(..., { needsProject: true }), plusparseOrgProjectInputin project mode, the same shape aslog 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.parseTraceTargetandparseDualModeArgshad no callers left, so they're removed. Their tests now go throughparseTraceTargetInput/parseDualModeInput, keeping the same coverage.SpanListArgumentsandTraceLogsArgumentsare added to therequire-target-resolutionlint rule's type list.Behavior changes (input that already failed):
Failed to parse "…" for org/project/trace-id: …, exit 252. Before, it failed insidefunc. For example,trace logs not-hex,trace logs my-org/web,span list my-org/web not-hex.ArgumentParseErrorwhose.exceptionis the parser's error, as with the otherboundArgumentscommands.sentry trace logs helpstill 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
parseSpanListInputtables (including<x>/<trace-id>→ project<x>and a URL'sbaseUrlnot being applied),trace logsparse failures before any lookup, andtrace logs helpinhelp-positional. Project-mode tests now mockresolveProjectBoundTarget.Verification:
tsc, CLI and repo lint (oxlint, oxfmt, ast-grep, lint-rule tests) pass.pnpm run cli: the three inputs above exit 252 with the parse error, andtrace logs helpprints help.via BYK.
--
View Junior Session in Sentry