From 0c7ea45d592d7b1b55048f58fc57e0ef579f9ec6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 18:59:19 +0000 Subject: [PATCH 1/4] ref(cli): Parse dashboard arguments with a grammar Dashboard commands parsed their positionals by hand inside `func` (`parseDashboardPositionalArgs`, `parseDashboardListArgs`). They now use `grammar/dashboard-arguments.pegjs` and parse before `func` runs with `boundArguments`, like the ID commands. Dashboards are named by numeric ID or title, and titles can hold spaces and slashes, so they can't share the hex-ID grammar. They follow its conventions instead: parsing has no side effects, a pasted URL's server and any warnings are applied on resolve, and extra arguments are ignored with a warning. Part of #1564 Co-Authored-By: BYK --- packages/cli/AGENTS.md | 6 +- packages/cli/grammar/dashboard-arguments.d.ts | 56 +++ .../cli/grammar/dashboard-arguments.pegjs | 127 ++++++ packages/cli/lint-rules/cli-oxlint-plugin.js | 3 + .../cli/src/commands/dashboard/arguments.ts | 171 ++++++++ packages/cli/src/commands/dashboard/list.ts | 36 +- .../cli/src/commands/dashboard/resolve.ts | 246 +----------- .../cli/src/commands/dashboard/restore.ts | 39 +- .../cli/src/commands/dashboard/revisions.ts | 39 +- packages/cli/src/commands/dashboard/view.ts | 39 +- .../cli/src/commands/dashboard/widget/add.ts | 65 ++-- .../src/commands/dashboard/widget/delete.ts | 41 +- .../cli/src/commands/dashboard/widget/edit.ts | 36 +- packages/cli/src/lib/complete.ts | 2 +- packages/cli/src/lib/completion-target.ts | 12 +- packages/cli/src/lib/dashboard-arguments.ts | 159 ++++++++ .../cli/test/commands/dashboard/list.test.ts | 20 + .../test/commands/dashboard/resolve.test.ts | 265 +------------ .../test/commands/dashboard/restore.test.ts | 37 ++ .../commands/dashboard/widget/add.test.ts | 12 +- .../cli/test/lib/dashboard-arguments.test.ts | 364 ++++++++++++++++++ packages/cli/test/lib/introspect.test.ts | 12 +- 22 files changed, 1081 insertions(+), 706 deletions(-) create mode 100644 packages/cli/grammar/dashboard-arguments.d.ts create mode 100644 packages/cli/grammar/dashboard-arguments.pegjs create mode 100644 packages/cli/src/commands/dashboard/arguments.ts create mode 100644 packages/cli/src/lib/dashboard-arguments.ts create mode 100644 packages/cli/test/lib/dashboard-arguments.test.ts diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index cf76e6c98..fb8d721d7 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -249,7 +249,9 @@ and the IDs. Add a kind to `ID_KINDS` rather than validating IDs in a command; the `sentry-cli/no-ad-hoc-id-arguments` lint rule rejects hand-written ID parsing (`parseSlashSeparatedArg`, `HEX_ID_RE`, ...) in commands and kinds defined outside `ID_KINDS`. Issue IDs (and `event view`, which takes them) -keep their own syntax. +keep their own syntax. Dashboards, named by numeric ID or title, have their +own grammar (`grammar/dashboard-arguments.pegjs`); dashboard commands take +`dashboardArguments` from `commands/dashboard/arguments.ts`. **Issue positionals:** issue IDs, short IDs, selectors, and issue URLs are parsed by `parseIssueArgument` (`src/lib/issue-arguments.ts`), whose forms @@ -343,7 +345,7 @@ input; errors come from `parseIdArguments`, never Peggy's own syntax errors. dashboard use a completion-aware parser from `src/lib/completion-target.ts` instead of `String`: `parseOrgProjectTarget` completes `org/`, `org/project`, and project aliases; `parseOrgTarget` completes org slugs only; -`parseIssueTarget` adds cached issue short IDs; `parseDashboardTarget` +`parseIssueTarget` adds cached issue short IDs; `dashboardArguments` completes cached dashboards. `issueIdsPositional` already uses them. A parser tagged with `withCompletionTarget`, such as the `target-positional.ts` parsers behind `listTargetPositional`, completes the same way. Shared value diff --git a/packages/cli/grammar/dashboard-arguments.d.ts b/packages/cli/grammar/dashboard-arguments.d.ts new file mode 100644 index 000000000..923f3edef --- /dev/null +++ b/packages/cli/grammar/dashboard-arguments.d.ts @@ -0,0 +1,56 @@ +// Types of the parser generated from dashboard-arguments.pegjs. The +// generator copies this file to src/generated/dashboard-arguments-grammar.d.ts +// and rewrites relative imports to resolve from there. + +/** The parts of a Sentry URL that dashboard arguments use. */ +export type DashboardUrl = { + readonly baseUrl: string; + readonly org?: string; + readonly dashboardId?: string; +}; + +/** What the caller passes the grammar. See `parseDashboardArgs()`. */ +export type DashboardArgumentsGrammarOptions = { + /** Whether these are `dashboard list` arguments: a target and a filter */ + list: boolean; + /** The Sentry URL `value` is, or `null` for anything else */ + sentryUrl: (value: string) => DashboardUrl | null; +}; + +/** Which argument is the dashboard and which its target. */ +export type DashboardSyntax = { + /** The numeric ID or title; absent when none was given */ + readonly ref?: string; + /** The org/project target, as typed, or `/` */ + readonly target?: string; + /** The URL the dashboard or org came from */ + readonly url?: DashboardUrl; + /** Arguments after the dashboard */ + readonly extra: readonly string[]; +}; + +/** Which argument is the target and which the title filter. */ +export type DashboardListSyntax = { + readonly target?: string; + /** The title filter, its words joined by spaces */ + readonly filter?: string; + /** The URL the org came from, or a dashboard URL, which lists nothing */ + readonly url?: DashboardUrl; +}; + +export declare class SyntaxError extends Error { + expected: unknown; + found: string | null; + location: unknown; +} + +/** Parse NUL-prefixed arguments into a dashboard and its target. */ +export declare function parse( + input: string, + options: DashboardArgumentsGrammarOptions & { list: false }, +): DashboardSyntax; +/** Parse NUL-prefixed arguments into a list target and title filter. */ +export declare function parse( + input: string, + options: DashboardArgumentsGrammarOptions & { list: true }, +): DashboardListSyntax; diff --git a/packages/cli/grammar/dashboard-arguments.pegjs b/packages/cli/grammar/dashboard-arguments.pegjs new file mode 100644 index 000000000..e64c5eec4 --- /dev/null +++ b/packages/cli/grammar/dashboard-arguments.pegjs @@ -0,0 +1,127 @@ +// Grammar for dashboard positionals. Dashboards are org-scoped, so these +// name a dashboard and, optionally, the org to find it in: +// +// the org is auto-detected +// / like a URL: the org, then a numeric ID +// // the project only selects the org +// any org/project target; a bare `` +// means `/` +// a Sentry dashboard URL +// a Sentry URL with only an org +// +// A title may hold slashes: one argument is `/[/]` only +// when slugs come before a numeric ID, and a title otherwise. Arguments +// after the dashboard are extra. +// +// With `list` (`dashboard list`), they name the org and a title filter: +// +// (none) the org is auto-detected, no filter +// an argument without a slash +// / or / a target, no filter +// // a target, then the filter +// the filter's words may be separate +// arguments +// [] a Sentry URL with only an org +// +// `pnpm run generate:grammar` compiles it, with the types in +// dashboard-arguments.d.ts, into the gitignored src/generated/; the build, +// typecheck, and test scripts run it. +// +// `parseDashboardArgs()` and `parseDashboardListArgs()` +// (src/lib/dashboard-arguments.ts) put a NUL, which no argument contains, +// before each argument. The grammar only decides which argument is what; it +// accepts any input and never throws. The caller reports a missing +// dashboard, turns a URL into warnings, and parses the target like any +// org/project target. + +{ + const { list, sentryUrl } = options; + + // A bare argument before the dashboard names an org + function orgTarget(value) { + return value.includes("/") ? value : `${value}/`; + } + + function filter(words) { + return words.length > 0 ? { filter: words.join(" ") } : {}; + } +} + +// Peggy runs a rule's action as soon as that rule matches, even when the +// enclosing alternative fails later and the parser backtracks. Every action +// here only builds values, and `sentryUrl` is pure, so that is harmless. + +arguments + = &{ return list; } @listArguments + / dashboardArguments + +dashboardArguments + = end { return { extra: [] }; } + // A dashboard URL; anything after it is extra + / sep u:dashboardUrl x:rest { + return { + ref: u.dashboardId, + ...(u.org && { target: `${u.org}/` }), + url: u, + extra: x, + }; + } + // A URL with only an org; the dashboard follows + / sep u:orgUrl sep r:token x:rest { + return { ref: r, target: `${u.org}/`, url: u, extra: x }; + } + / sep u:orgUrl end { return { url: u, extra: [] }; } + // `//` and `/` + / sep o:slug "/" p:slug "/" i:digits end { + return { ref: i, target: `${o}/${p}`, extra: [] }; + } + / sep o:slug "/" i:digits end { return { ref: i, target: `${o}/`, extra: [] }; } + // Anything else in one argument is an ID or a title, slashes and all + / sep r:token end { return { ref: r, extra: [] }; } + // ` ` + / sep t:token sep r:token x:rest { + return { ref: r, target: orgTarget(t), extra: x }; + } + +listArguments + = end { return {}; } + // A dashboard URL names no list; the caller suggests `dashboard view` + / sep u:dashboardUrl rest { return { url: u }; } + / sep u:orgUrl x:rest { return { target: `${u.org}/`, url: u, ...filter(x) }; } + // ` ` + / sep t:token x:rest &{ return x.length > 0; } { + return { target: orgTarget(t), ...filter(x) }; + } + // `//`: the filter is everything after the second + // slash. With nothing after it, the whole argument is the target. + / sep t:$([^/\0]* "/" [^/\0]*) "/" f:$[^\0]+ end { return { target: t, filter: f }; } + / sep t:$([^/\0]* "/" [^\0]*) end { return { target: t }; } + / sep f:token end { return { filter: f }; } + +// The arguments after the ones a form names +rest + = (sep @token)* + +// A Sentry URL naming a dashboard +dashboardUrl + = u:token &{ return sentryUrl(u)?.dashboardId !== undefined; } + { return sentryUrl(u); } + +// A Sentry URL naming an org but no dashboard +orgUrl + = u:token &{ + const url = sentryUrl(u); + return url?.org !== undefined && url.dashboardId === undefined; + } + { return sentryUrl(u); } + +slug = $[a-zA-Z0-9_-]+ + +digits = $[0-9]+ + +// Possibly empty: an empty argument is still one +token = $[^\0]* + +sep = "\0" + +end = !. diff --git a/packages/cli/lint-rules/cli-oxlint-plugin.js b/packages/cli/lint-rules/cli-oxlint-plugin.js index 97a95a26b..33141c3fc 100644 --- a/packages/cli/lint-rules/cli-oxlint-plugin.js +++ b/packages/cli/lint-rules/cli-oxlint-plugin.js @@ -66,6 +66,8 @@ const BOUND_TARGET_TYPES = new Set([ "OrgProjectTarget", "ProjectBoundTarget", "BoundArguments", + "DashboardArguments", + "DashboardListArguments", "LogListArguments", "LogViewArguments", "ReplayArguments", @@ -73,6 +75,7 @@ const BOUND_TARGET_TYPES = new Set([ "SpanViewArguments", "TraceLogsArguments", "TraceViewArguments", + "WidgetAddArguments", ]); function boundTargetParams(fn) { diff --git a/packages/cli/src/commands/dashboard/arguments.ts b/packages/cli/src/commands/dashboard/arguments.ts new file mode 100644 index 000000000..f52e02a33 --- /dev/null +++ b/packages/cli/src/commands/dashboard/arguments.ts @@ -0,0 +1,171 @@ +/** + * Dashboard positionals: `[] ` and + * `dashboard list`'s `[] []`, parsed by + * `grammar/dashboard-arguments.pegjs` before `func` runs and resolved to + * the org (and dashboard ID) on the first `.resolved` read. + */ + +import type { SentryContext } from "../../context.js"; +import { + type ParsedOrgProject, + parseOrgProjectInput, +} from "../../lib/arg-parsing.js"; +import { + DASHBOARD_TARGET, + ORG_PROJECT_TARGET, +} from "../../lib/completion-target.js"; +import { + parseDashboardArgs, + parseDashboardListArgs, +} from "../../lib/dashboard-arguments.js"; +import { logger } from "../../lib/logger.js"; +import { + type BoundArguments, + boundArguments, +} from "../../lib/positional-parse.js"; +import { applySentryUrlContext } from "../../lib/sentry-url-parser.js"; +import { resolveDashboardId, resolveOrgFromTarget } from "./resolve.js"; + +const log = logger.withTag("dashboard"); + +/** A dashboard argument's dashboard and org target, parsed. */ +export type DashboardInput = { + /** The numeric ID or title, as typed */ + readonly ref: string; + readonly target: ParsedOrgProject; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + readonly warnings: readonly string[]; +}; + +/** A dashboard, resolved to its org and numeric ID. */ +export type ResolvedDashboard = { + readonly orgSlug: string; + readonly dashboardId: string; +}; + +/** The arguments of a command that takes one dashboard. */ +export type DashboardArguments = BoundArguments< + DashboardInput, + ResolvedDashboard +>; + +/** + * Parse dashboard arguments with {@link parseDashboardArgs}, and their + * target, without side effects. + * + * @throws {ValidationError} When no dashboard is given or the target is + * invalid + */ +export function parseDashboardInput( + args: readonly string[], + usageHint: string, +): DashboardInput { + const { dashboardRef, targetArg, baseUrl, warnings } = parseDashboardArgs( + args, + usageHint, + ); + const target = parseOrgProjectInput(targetArg); + return { + ref: dashboardRef, + target: target.parsed, + baseUrl: baseUrl ?? target.baseUrl, + warnings, + }; +} + +/** + * Resolve parsed dashboard arguments: apply the URL's server, warn, find + * the org, and look a title up. + */ +export async function resolveDashboardInput( + input: DashboardInput, + cwd: string, + usageHint: string, +): Promise { + applyInputContext(input); + const orgSlug = await resolveOrgFromTarget(input.target, cwd, usageHint); + const dashboardId = await resolveDashboardId(orgSlug, input.ref); + return { orgSlug, dashboardId }; +} + +/** + * The `[] ` positional of a command that + * takes one dashboard (see `grammar/dashboard-arguments.pegjs`). + * + * @param options.usageHint - Usage shown in errors and warnings + */ +export function dashboardArguments(options: { + readonly placeholder: string; + readonly usageHint: string; +}) { + const { placeholder, usageHint } = options; + return boundArguments({ + placeholder, + brief: "[] ", + kind: DASHBOARD_TARGET, + parse: (args) => parseDashboardInput(args, usageHint), + resolve: (input: DashboardInput, context: SentryContext) => + resolveDashboardInput(input, context.cwd, usageHint), + }); +} + +/** `dashboard list` arguments: the org target and title filter, parsed. */ +export type DashboardListInput = { + readonly target: ParsedOrgProject; + /** Title glob to filter by */ + readonly titleFilter?: string; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + readonly warnings: readonly string[]; +}; + +/** The arguments of `dashboard list`; they resolve to the org slug. */ +export type DashboardListArguments = BoundArguments; + +/** + * Parse `dashboard list` arguments with {@link parseDashboardListArgs}, and + * their target, without side effects. + */ +function parseDashboardListInput(args: readonly string[]): DashboardListInput { + const { targetArg, titleFilter, baseUrl, warnings } = + parseDashboardListArgs(args); + const target = parseOrgProjectInput(targetArg); + return { + target: target.parsed, + titleFilter, + baseUrl: baseUrl ?? target.baseUrl, + warnings, + }; +} + +/** + * The `[] []` positional of `dashboard list`. + * + * @param usageHint - Usage shown when the org cannot be found + */ +export function dashboardListArguments(usageHint: string) { + return boundArguments({ + placeholder: "org/title-filter", + brief: "[] []", + kind: ORG_PROJECT_TARGET, + parse: parseDashboardListInput, + resolve: (input: DashboardListInput, context: SentryContext) => { + applyInputContext(input); + return resolveOrgFromTarget(input.target, context.cwd, usageHint); + }, + }); +} + +/** Apply a pasted URL's server and log the parse's warnings. */ +function applyInputContext(input: { + readonly baseUrl?: string; + readonly warnings: readonly string[]; +}): void { + if (input.baseUrl) { + applySentryUrlContext(input.baseUrl); + } + for (const warning of input.warnings) { + log.warn(warning); + } +} diff --git a/packages/cli/src/commands/dashboard/list.ts b/packages/cli/src/commands/dashboard/list.ts index 0c08fb31e..8e8a68c2d 100644 --- a/packages/cli/src/commands/dashboard/list.ts +++ b/packages/cli/src/commands/dashboard/list.ts @@ -12,9 +12,7 @@ import { API_MAX_PER_PAGE, listDashboardsPaginated, } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { advancePaginationState, buildPaginationContextKey, @@ -39,10 +37,10 @@ import { import type { DashboardListItem } from "../../types/dashboard.js"; import type { Writer } from "../../types/index.js"; import { - withDashboardErrors, - parseDashboardListArgs, - resolveOrgFromTarget, -} from "./resolve.js"; + type DashboardListArguments, + dashboardListArguments, +} from "./arguments.js"; +import { withDashboardErrors } from "./resolve.js"; /** Command key for pagination cursor storage */ export const PAGINATION_KEY = "dashboard-list"; @@ -418,14 +416,7 @@ export const listCommand = buildListCommand("dashboard", { jsonTransform: jsonTransformDashboardList, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/title-filter", - brief: "[] []", - parse: parseOrgProjectTarget, - }, - }, + positional: dashboardListArguments("sentry dashboard list /"), flags: { web: { kind: "boolean", @@ -436,16 +427,13 @@ export const listCommand = buildListCommand("dashboard", { }, aliases: { w: "web", n: "limit" }, }, - async *func(this: SentryContext, flags: ListFlags, ...args: string[]) { - const { cwd } = this; - - const { targetArg, titleFilter } = parseDashboardListArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard list /", - ); + async *func( + this: SentryContext, + flags: ListFlags, + target: DashboardListArguments, + ) { + const { titleFilter } = target.parsed; + const orgSlug = await target.resolved; if (flags.web) { await openInBrowser(buildDashboardsListUrl(orgSlug), "dashboards"); diff --git a/packages/cli/src/commands/dashboard/resolve.ts b/packages/cli/src/commands/dashboard/resolve.ts index 48f6120b0..3299a768f 100644 --- a/packages/cli/src/commands/dashboard/resolve.ts +++ b/packages/cli/src/commands/dashboard/resolve.ts @@ -11,7 +11,7 @@ import { API_MAX_PER_PAGE, listDashboardsPaginated, } from "../../lib/api-client.js"; -import type { parseOrgProjectArg } from "../../lib/arg-parsing.js"; +import type { ParsedOrgProject } from "../../lib/arg-parsing.js"; import { ApiError, ResolutionError, @@ -20,10 +20,6 @@ import { import { fuzzyMatch } from "../../lib/fuzzy.js"; import { logger } from "../../lib/logger.js"; import { resolveOrgOnlyTarget } from "../../lib/resolve-target.js"; -import { - applySentryUrlContext, - parseSentryUrl, -} from "../../lib/sentry-url-parser.js"; import { isAllDigits } from "../../lib/utils.js"; import { type DashboardWidget, @@ -36,9 +32,6 @@ import { WIDGET_TYPES, } from "../../types/dashboard.js"; -/** Recognize slug-shaped targets without treating arbitrary slash titles as paths. */ -const DASHBOARD_REF_PATH_RE = /^([a-zA-Z0-9_-]+(?:\/[a-zA-Z0-9_-]+)?)\/(.+)$/; - /** Shared widget query flags used by `add` and `edit` commands */ export type WidgetQueryFlags = { readonly display?: string; @@ -63,248 +56,13 @@ export type WidgetQueryFlags = { * @returns Organization slug */ export function resolveOrgFromTarget( - parsed: ReturnType, + parsed: ParsedOrgProject, cwd: string, usageHint: string, ): Promise { return resolveOrgOnlyTarget(parsed, cwd, "dashboard", usageHint); } -/** Result of URL-based dashboard arg extraction */ -type DashboardArgResult = { - dashboardRef: string; - targetArg: string | undefined; -}; - -/** - * Try to extract dashboard ref + org from a Sentry URL. - * - * Calls `applySentryUrlContext` for host-scoping trust checks on non-SaaS URLs. - * Returns null if the input isn't a recognized Sentry URL. - */ -function tryExtractDashboardUrl( - first: string, - args: string[], -): DashboardArgResult | null { - const urlParsed = parseSentryUrl(first); - if (!urlParsed) { - return null; - } - applySentryUrlContext(urlParsed.baseUrl); - if (urlParsed.dashboardId) { - log.warn( - `Extracted dashboard ID ${urlParsed.dashboardId} from URL` + - (urlParsed.org ? ` (org: ${urlParsed.org})` : ""), - ); - return { - dashboardRef: urlParsed.dashboardId, - targetArg: urlParsed.org ? `${urlParsed.org}/` : undefined, - }; - } - // URL recognized but no dashboardId — use org context if available - if (urlParsed.org) { - log.warn(`Extracted org '${urlParsed.org}' from URL`); - if (args.length >= 2) { - return { - dashboardRef: args[1] as string, - targetArg: `${urlParsed.org}/`, - }; - } - throw new ValidationError( - "Dashboard ID or title is required.\n\n" + - "The URL provided contains an org but no dashboard ID.\n" + - `Try: sentry dashboard ${urlParsed.org}/ `, - "dashboard", - ); - } - return null; -} - -/** - * Parse a dashboard reference and optional target from array positional args. - * - * Handles: - * - `` — single arg (auto-detect org) - * - `/` or `//` — qualified numeric ID - * - ` ` — explicit target + dashboard ref - * - Full Sentry dashboard URL — extracts org + dashboard ID - * - * When two args are provided and the first is a bare slug (no `/`), it is - * normalized to `slug/` so `parseOrgProjectArg` treats it as an org-all - * target. Dashboards are org-scoped so the project component is irrelevant. - * - * @param args - Raw positional arguments - * @returns Dashboard reference string and optional target arg - */ -export function parseDashboardPositionalArgs( - args: string[], -): DashboardArgResult { - if (args.length === 0) { - throw new ValidationError( - "Dashboard ID or title is required.", - "dashboard", - ); - } - - const first = args[0] as string; - - // URL detection — extract org + dashboard ID from pasted Sentry URLs - const urlResult = tryExtractDashboardUrl(first, args); - if (urlResult) { - return urlResult; - } - - if (args.length === 1) { - const [, target, dashboardRef] = DASHBOARD_REF_PATH_RE.exec(first) ?? []; - if (target && dashboardRef && isAllDigits(dashboardRef)) { - return { - dashboardRef, - targetArg: target.includes("/") ? target : `${target}/`, - }; - } - return { - dashboardRef: first, - targetArg: undefined, - }; - } - // Normalize bare org slug → org/ (dashboards are org-scoped) - const target = first.includes("/") ? first : `${first}/`; - return { - dashboardRef: args[1] as string, - targetArg: target, - }; -} - -/** - * Parse dashboard list positional args into a target and optional title filter. - * - * Handles: - * - (empty) — auto-detect org, no filter - * - `` or `` — target, no filter - * - `'CLI'` or `'Error*'` or `'*API*'` — auto-detect org, title filter - * - ` 'Error*'` or ` 'Error*'` — target + glob filter - * - * A single arg without `/` is always treated as a title filter, not a - * project-search target. Dashboards are org-scoped so project-search - * doesn't apply — `resolveOrgFromTarget` ignores the slug anyway. - * To specify an org, use `org/` or pass two args: `org 'filter'`. - * - * When two args are provided and the first is a bare slug (no `/`), it is - * normalized to `slug/` so `parseOrgProjectArg` treats it as an org-all target. - * - * @param args - Raw positional arguments - * @returns Target arg for org resolution and optional title filter glob - */ -/** Result of URL-based list arg extraction */ -type ListArgResult = { - targetArg: string | undefined; - titleFilter: string | undefined; -}; - -/** - * Try to extract org context from a Sentry URL in dashboard list args. - * - * When the URL contains a dashboard ID, throws a helpful error suggesting - * `sentry dashboard view` instead. When only an org is present, extracts - * it as the target. - */ -function tryExtractListUrl( - first: string, - remaining: string[], -): ListArgResult | null { - const urlParsed = parseSentryUrl(first); - if (!urlParsed) { - return null; - } - applySentryUrlContext(urlParsed.baseUrl); - if (urlParsed.dashboardId) { - const orgPrefix = urlParsed.org ? `${urlParsed.org}/ ` : ""; - const orgSuffix = urlParsed.org ? ` ${urlParsed.org}/` : ""; - const orgNote = urlParsed.org ? ` in '${urlParsed.org}'` : ""; - throw new ValidationError( - "This looks like a dashboard URL. To view a specific dashboard:\n\n" + - ` sentry dashboard view ${orgPrefix}${urlParsed.dashboardId}\n\n` + - `To list dashboards${orgNote}:\n\n` + - ` sentry dashboard list${orgSuffix}`, - "dashboard", - ); - } - if (urlParsed.org) { - log.warn(`Extracted org '${urlParsed.org}' from URL`); - const titleFilter = remaining.length > 0 ? remaining.join(" ") : undefined; - return { targetArg: `${urlParsed.org}/`, titleFilter }; - } - return null; -} - -export function parseDashboardListArgs(args: string[]): ListArgResult { - // buildListCommand's interceptSubcommand may replace args[0] with undefined - // when the first positional matches a subcommand name (e.g. "view", "create"). - // Filter those out so we don't crash on .includes("/"). - const filtered = args.filter( - (a): a is string => a !== null && a !== undefined && a !== "", - ); - if (filtered.length === 0) { - return { targetArg: undefined, titleFilter: undefined }; - } - - // URL detection — extract org or suggest `view` for dashboard-specific URLs - const urlResult = tryExtractListUrl(filtered[0] as string, filtered.slice(1)); - if (urlResult) { - return urlResult; - } - - if (filtered.length >= 2) { - // First arg is the target, remaining args are joined as the filter. - // This handles unquoted multi-word titles: `my-org/ CLI Health` arrives - // as ["my-org/", "CLI", "Health"] and becomes filter "CLI Health". - // Normalize bare org slug to org/ format so parseOrgProjectArg treats - // it as org-all (dashboards are org-scoped, project is irrelevant). - const raw = filtered[0] as string; - const target = raw.includes("/") ? raw : `${raw}/`; - const titleFilter = filtered.slice(1).join(" "); - return { targetArg: target, titleFilter }; - } - // 1 arg: if it contains "/" it may be a target, or an org/project/name combo. - // Without "/" it's always a title filter (dashboards are org-scoped). - const arg = filtered[0] as string; - if (arg.includes("/")) { - return splitOrgProjectName(arg); - } - return { targetArg: undefined, titleFilter: arg }; -} - -/** - * Split a slash-containing single arg into target and optional title filter. - * - * - `org/` or `org/project` (≤1 slash) → target only, no filter - * - `org/project/name` (2+ slashes) → target is `org/project`, filter is the rest - * - * This lets users type `sentry dashboard list my-org/my-project/CLI` as a - * single arg instead of requiring two separate args. - */ -function splitOrgProjectName(arg: string): { - targetArg: string | undefined; - titleFilter: string | undefined; -} { - const firstSlash = arg.indexOf("/"); - const secondSlash = arg.indexOf("/", firstSlash + 1); - - if (secondSlash === -1) { - // Only one slash: "org/" or "org/project" — target only - return { targetArg: arg, titleFilter: undefined }; - } - - // Two+ slashes: split into target + name filter - const target = arg.slice(0, secondSlash); - const name = arg.slice(secondSlash + 1); - if (!name) { - // Trailing slash after project: "org/project/" → target only - return { targetArg: arg, titleFilter: undefined }; - } - return { targetArg: target, titleFilter: name }; -} - /** * Resolve a dashboard reference (numeric ID or title) to a numeric ID string. * diff --git a/packages/cli/src/commands/dashboard/restore.ts b/packages/cli/src/commands/dashboard/restore.ts index fc9d71432..c07badee1 100644 --- a/packages/cli/src/commands/dashboard/restore.ts +++ b/packages/cli/src/commands/dashboard/restore.ts @@ -6,9 +6,7 @@ import type { SentryContext } from "../../context.js"; import { restoreDashboardRevision } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; -import { parseDashboardTarget } from "../../lib/completion-target.js"; import { ValidationError } from "../../lib/errors.js"; import { colorTag, escapeMarkdownCell } from "../../lib/formatters/markdown.js"; import { CommandOutput } from "../../lib/formatters/output.js"; @@ -16,12 +14,8 @@ import { formatRelativeTime } from "../../lib/formatters/time-utils.js"; import { withProgress } from "../../lib/polling.js"; import { buildDashboardUrl } from "../../lib/sentry-web-urls.js"; import type { DashboardDetail } from "../../types/dashboard.js"; -import { - withDashboardErrors, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, -} from "./resolve.js"; +import { type DashboardArguments, dashboardArguments } from "./arguments.js"; +import { withDashboardErrors } from "./resolve.js"; type RestoreFlags = { readonly revision: string; @@ -82,14 +76,10 @@ export const restoreCommand = buildCommand({ human: formatRestoreHuman, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/dashboard", - brief: "[] ", - parse: parseDashboardTarget, - }, - }, + positional: dashboardArguments({ + placeholder: "org/dashboard", + usageHint: "sentry dashboard restore / --revision ", + }), flags: { revision: { kind: "parsed", @@ -108,17 +98,12 @@ export const restoreCommand = buildCommand({ }, aliases: { r: "revision" }, }, - async *func(this: SentryContext, flags: RestoreFlags, ...args: string[]) { - const { cwd } = this; - - const { dashboardRef, targetArg } = parseDashboardPositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard restore / --revision ", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + async *func( + this: SentryContext, + flags: RestoreFlags, + target: DashboardArguments, + ) { + const { orgSlug, dashboardId } = await target.resolved; const dashboard = await withDashboardErrors( { diff --git a/packages/cli/src/commands/dashboard/revisions.ts b/packages/cli/src/commands/dashboard/revisions.ts index b15a7751d..713777449 100644 --- a/packages/cli/src/commands/dashboard/revisions.ts +++ b/packages/cli/src/commands/dashboard/revisions.ts @@ -7,9 +7,7 @@ import type { SentryContext } from "../../context.js"; import { paginate } from "../../lib/api/infrastructure.js"; import { listDashboardRevisionsPaginated } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; -import { parseDashboardTarget } from "../../lib/completion-target.js"; import { advancePaginationState, buildPaginationContextKey, @@ -30,12 +28,8 @@ import { withProgress } from "../../lib/polling.js"; import { buildDashboardUrl } from "../../lib/sentry-web-urls.js"; import type { DashboardRevision } from "../../types/dashboard.js"; import type { Writer } from "../../types/index.js"; -import { - withDashboardErrors, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, -} from "./resolve.js"; +import { type DashboardArguments, dashboardArguments } from "./arguments.js"; +import { withDashboardErrors } from "./resolve.js"; const PAGINATION_KEY = "dashboard-revisions"; @@ -145,31 +139,22 @@ export const revisionsCommand = buildCommand({ jsonTransform: jsonTransformRevisions, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/dashboard", - brief: "[] ", - parse: parseDashboardTarget, - }, - }, + positional: dashboardArguments({ + placeholder: "org/dashboard", + usageHint: "sentry dashboard revisions / ", + }), flags: { limit: buildListLimitFlag("revisions"), cursor: LIST_CURSOR_FLAG, }, aliases: { n: "limit", c: "cursor" }, }, - async *func(this: SentryContext, flags: RevisionsFlags, ...args: string[]) { - const { cwd } = this; - - const { dashboardRef, targetArg } = parseDashboardPositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard revisions / ", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + async *func( + this: SentryContext, + flags: RevisionsFlags, + target: DashboardArguments, + ) { + const { orgSlug, dashboardId } = await target.resolved; const contextKey = buildPaginationContextKey( "dashboard-revisions", diff --git a/packages/cli/src/commands/dashboard/view.ts b/packages/cli/src/commands/dashboard/view.ts index 977e998f6..d6f37a886 100644 --- a/packages/cli/src/commands/dashboard/view.ts +++ b/packages/cli/src/commands/dashboard/view.ts @@ -7,10 +7,8 @@ import type { SentryContext } from "../../context.js"; import { getDashboard, queryAllWidgets } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; -import { parseDashboardTarget } from "../../lib/completion-target.js"; import type { DashboardViewData } from "../../lib/formatters/dashboard.js"; import { createDashboardViewRenderer } from "../../lib/formatters/dashboard.js"; import { ClearScreen, CommandOutput } from "../../lib/formatters/output.js"; @@ -36,12 +34,8 @@ import type { DashboardWidget, WidgetDataResult, } from "../../types/dashboard.js"; -import { - withDashboardErrors, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, -} from "./resolve.js"; +import { type DashboardArguments, dashboardArguments } from "./arguments.js"; +import { withDashboardErrors } from "./resolve.js"; /** Default auto-refresh interval in seconds */ const DEFAULT_REFRESH_INTERVAL = 60; @@ -218,14 +212,10 @@ export const viewCommand = buildCommand({ jsonExclude: ["rendererPreference", "graphicsCap"], }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/project/dashboard", - brief: "[] ", - parse: parseDashboardTarget, - }, - }, + positional: dashboardArguments({ + placeholder: "org/project/dashboard", + usageHint: "sentry dashboard view / ", + }), flags: { web: { kind: "boolean", @@ -266,18 +256,13 @@ export const viewCommand = buildCommand({ t: "period", }, }, - async *func(this: SentryContext, flags: ViewFlags, ...args: string[]) { + async *func( + this: SentryContext, + flags: ViewFlags, + target: DashboardArguments, + ) { applyFreshFlag(flags); - const { cwd } = this; - - const { dashboardRef, targetArg } = parseDashboardPositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard view / ", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + const { orgSlug, dashboardId } = await target.resolved; const url = buildDashboardUrl(orgSlug, dashboardId); if (flags.web) { diff --git a/packages/cli/src/commands/dashboard/widget/add.ts b/packages/cli/src/commands/dashboard/widget/add.ts index 2f5d8d95b..ab1c960f3 100644 --- a/packages/cli/src/commands/dashboard/widget/add.ts +++ b/packages/cli/src/commands/dashboard/widget/add.ts @@ -6,11 +6,15 @@ import type { SentryContext } from "../../../context.js"; import { getDashboard, updateDashboard } from "../../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../../lib/arg-parsing.js"; import { buildCommand, numberParser } from "../../../lib/command.js"; +import { DASHBOARD_TARGET } from "../../../lib/completion-target.js"; import { ValidationError } from "../../../lib/errors.js"; import { formatWidgetAdded } from "../../../lib/formatters/human.js"; import { CommandOutput } from "../../../lib/formatters/output.js"; +import { + type BoundArguments, + boundArguments, +} from "../../../lib/positional-parse.js"; import { buildDashboardUrl } from "../../../lib/sentry-web-urls.js"; import { assignDefaultLayout, @@ -22,13 +26,16 @@ import { type WidgetLayoutFlags, type WidgetLayoutMode, } from "../../../types/dashboard.js"; +import { + type DashboardInput, + parseDashboardInput, + type ResolvedDashboard, + resolveDashboardInput, +} from "../arguments.js"; import { buildWidgetFromFlags, withDashboardErrors, normalizeDataset, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, validateWidgetEnums, type WidgetQueryFlags, } from "../resolve.js"; @@ -47,12 +54,21 @@ type AddResult = { url: string; }; +const USAGE_HINT = + "sentry dashboard widget add / --display <type>"; + +/** `widget add` arguments: the dashboard, then the widget title. */ +type WidgetAddInput = DashboardInput & { readonly title: string }; + +/** The arguments of `widget add`. */ +type WidgetAddArguments = BoundArguments<WidgetAddInput, ResolvedDashboard>; + /** * Parse positional args for widget add. * Last arg is widget title, rest go to dashboard resolution. */ -function parseAddPositionalArgs(args: string[]): { - dashboardArgs: string[]; +function parseAddPositionalArgs(args: readonly string[]): { + dashboardArgs: readonly string[]; title: string; } { if (args.length < 2) { @@ -138,14 +154,17 @@ export const addCommand = buildCommand({ human: formatWidgetAdded, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/project/dashboard/title", - brief: "[<org/project>] <dashboard> <title>", - parse: String, + positional: boundArguments({ + placeholder: "org/project/dashboard/title", + brief: "[<org/project>] <dashboard> <title>", + kind: DASHBOARD_TARGET, + parse: (args): WidgetAddInput => { + const { dashboardArgs, title } = parseAddPositionalArgs(args); + return { ...parseDashboardInput(dashboardArgs, USAGE_HINT), title }; }, - }, + resolve: (input: WidgetAddInput, context: SentryContext) => + resolveDashboardInput(input, context.cwd, USAGE_HINT), + }), flags: { display: { kind: "parsed", @@ -235,10 +254,12 @@ export const addCommand = buildCommand({ y: "row", }, }, - async *func(this: SentryContext, flags: AddFlags, ...args: string[]) { - const { cwd } = this; - - const { dashboardArgs, title } = parseAddPositionalArgs(args); + async *func( + this: SentryContext, + flags: AddFlags, + target: WidgetAddArguments, + ) { + const { title } = target.parsed; // Resolve dataset aliases (e.g. "errors" → "error-events") once, up front. // Every downstream consumer — enum validation, dataset-aware aggregate @@ -249,15 +270,7 @@ export const addCommand = buildCommand({ // Validate enums before any network calls (fail fast) validateWidgetEnums(flags.display, dataset); - const { dashboardRef, targetArg } = - parseDashboardPositionalArgs(dashboardArgs); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard widget add <org>/ <dashboard> <title> --display <type>", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + const { orgSlug, dashboardId } = await target.resolved; let newWidget = buildWidgetFromFlags({ title, diff --git a/packages/cli/src/commands/dashboard/widget/delete.ts b/packages/cli/src/commands/dashboard/widget/delete.ts index 2ff2518a9..fe686bfd6 100644 --- a/packages/cli/src/commands/dashboard/widget/delete.ts +++ b/packages/cli/src/commands/dashboard/widget/delete.ts @@ -12,9 +12,7 @@ import type { SentryContext } from "../../../context.js"; import { getDashboard, updateDashboard } from "../../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../../lib/arg-parsing.js"; import { numberParser } from "../../../lib/command.js"; -import { parseDashboardTarget } from "../../../lib/completion-target.js"; import { ValidationError } from "../../../lib/errors.js"; import { formatWidgetDeleted } from "../../../lib/formatters/human.js"; import { CommandOutput } from "../../../lib/formatters/output.js"; @@ -24,13 +22,8 @@ import { type DashboardDetail, prepareDashboardForUpdate, } from "../../../types/dashboard.js"; -import { - withDashboardErrors, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, - resolveWidgetIndex, -} from "../resolve.js"; +import { type DashboardArguments, dashboardArguments } from "../arguments.js"; +import { withDashboardErrors, resolveWidgetIndex } from "../resolve.js"; type DeleteFlags = { readonly index?: number; @@ -93,14 +86,11 @@ export const deleteCommand = buildDeleteCommand( }, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/project/dashboard", - brief: "[<org/project>] <dashboard-id-or-title>", - parse: parseDashboardTarget, - }, - }, + positional: dashboardArguments({ + placeholder: "org/project/dashboard", + usageHint: + "sentry dashboard widget delete <org>/ <id> (--index <n> | --title <name>)", + }), flags: { index: { kind: "parsed", @@ -117,9 +107,11 @@ export const deleteCommand = buildDeleteCommand( }, aliases: { i: "index", t: "title" }, }, - async *func(this: SentryContext, flags: DeleteFlags, ...args: string[]) { - const { cwd } = this; - + async *func( + this: SentryContext, + flags: DeleteFlags, + target: DashboardArguments, + ) { if (flags.index === undefined && !flags.title) { throw new ValidationError( "Specify --index or --title to identify the widget to delete.", @@ -127,14 +119,7 @@ export const deleteCommand = buildDeleteCommand( ); } - const { dashboardRef, targetArg } = parseDashboardPositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard widget delete <org>/ <id> (--index <n> | --title <name>)", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + const { orgSlug, dashboardId } = await target.resolved; // GET current dashboard → find widget const current = await withDashboardErrors( diff --git a/packages/cli/src/commands/dashboard/widget/edit.ts b/packages/cli/src/commands/dashboard/widget/edit.ts index 9c360c7b0..ddea9d817 100644 --- a/packages/cli/src/commands/dashboard/widget/edit.ts +++ b/packages/cli/src/commands/dashboard/widget/edit.ts @@ -6,9 +6,7 @@ import type { SentryContext } from "../../../context.js"; import { getDashboard, updateDashboard } from "../../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../../lib/arg-parsing.js"; import { buildCommand, numberParser } from "../../../lib/command.js"; -import { parseDashboardTarget } from "../../../lib/completion-target.js"; import { ValidationError } from "../../../lib/errors.js"; import { formatWidgetEdited } from "../../../lib/formatters/human.js"; import { CommandOutput } from "../../../lib/formatters/output.js"; @@ -27,13 +25,11 @@ import { validateWidgetLayout, type WidgetLayoutFlags, } from "../../../types/dashboard.js"; +import { type DashboardArguments, dashboardArguments } from "../arguments.js"; import { applyGroupLimitAutoDefault, withDashboardErrors, normalizeDataset, - parseDashboardPositionalArgs, - resolveDashboardId, - resolveOrgFromTarget, resolveWidgetIndex, validateSortReferencesAggregate, validateWidgetEnums, @@ -226,14 +222,11 @@ export const editCommand = buildCommand({ human: formatWidgetEdited, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/project/dashboard", - brief: "[<org/project>] <dashboard-id-or-title>", - parse: parseDashboardTarget, - }, - }, + positional: dashboardArguments({ + placeholder: "org/project/dashboard", + usageHint: + "sentry dashboard widget edit <org>/ <dashboard> --title <name> --display <type>", + }), flags: { index: { kind: "parsed", @@ -337,9 +330,11 @@ export const editCommand = buildCommand({ y: "row", }, }, - async *func(this: SentryContext, flags: EditFlags, ...args: string[]) { - const { cwd } = this; - + async *func( + this: SentryContext, + flags: EditFlags, + target: DashboardArguments, + ) { if (flags.index === undefined && !flags.title) { throw new ValidationError( "Specify --index or --title to identify the widget to edit.\n\n" + @@ -363,14 +358,7 @@ export const editCommand = buildCommand({ validateWidgetEnums(normalizedFlags.display, normalizedFlags.dataset); - const { dashboardRef, targetArg } = parseDashboardPositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const orgSlug = await resolveOrgFromTarget( - parsed, - cwd, - "sentry dashboard widget edit <org>/ <dashboard> --title <name> --display <type>", - ); - const dashboardId = await resolveDashboardId(orgSlug, dashboardRef); + const { orgSlug, dashboardId } = await target.resolved; // GET current dashboard → find widget → merge changes → PUT const current = await withDashboardErrors( diff --git a/packages/cli/src/lib/complete.ts b/packages/cli/src/lib/complete.ts index 59dd28bc4..e2bba5579 100644 --- a/packages/cli/src/lib/complete.ts +++ b/packages/cli/src/lib/complete.ts @@ -195,7 +195,7 @@ const COMPLETERS: Record<CompletionTarget, Completer> = { "org/project": (partial) => completeOrgSlashProject(partial), issue: entityCompleter("issue", completeOrgSlashProject), // Only the first word: in `<org/project> <id-or-title>`, the org of the - // second word is for `parseDashboardPositionalArgs` to decide, and this + // second word is for the dashboard grammar to decide, and this // fast path cannot load it dashboard: (partial, slot) => slot.positionals.length > 0 ? [] : completeDashboards(partial), diff --git a/packages/cli/src/lib/completion-target.ts b/packages/cli/src/lib/completion-target.ts index 066af0027..cb4855114 100644 --- a/packages/cli/src/lib/completion-target.ts +++ b/packages/cli/src/lib/completion-target.ts @@ -8,9 +8,9 @@ * `__complete` fast path cannot boot Stricli to walk the route tree, and * `src/lib/complete.ts` maps each kind to its completer. * - * {@link parseOrgTarget}, {@link parseOrgProjectTarget}, - * {@link parseIssueTarget}, and {@link parseDashboardTarget} return their - * input unchanged and only mark the kind. Parsers that also parse the value, + * {@link parseOrgTarget}, {@link parseOrgProjectTarget}, and + * {@link parseIssueTarget} return their input unchanged and only mark the + * kind. Parsers that also parse the value, * such as those in `target-positional.ts`, use the same tag. */ @@ -83,12 +83,6 @@ export const parseIssueTarget = withCompletionTarget( (input: string): string => input, ); -/** Parser for `[<org/project>] <dashboard-id-or-title>` positionals. */ -export const parseDashboardTarget = withCompletionTarget( - DASHBOARD_TARGET, - (input: string): string => input, -); - /** * Shared value parsers and what their arguments complete. Listed here, not * tagged where they are defined, so those modules know nothing about diff --git a/packages/cli/src/lib/dashboard-arguments.ts b/packages/cli/src/lib/dashboard-arguments.ts new file mode 100644 index 000000000..2136a14b7 --- /dev/null +++ b/packages/cli/src/lib/dashboard-arguments.ts @@ -0,0 +1,159 @@ +/** + * Dashboard positionals, parsed by `grammar/dashboard-arguments.pegjs`. + * + * Dashboards are org-scoped and named by a numeric ID or a title, which may + * hold spaces and slashes. That is a different syntax from the hex IDs of + * `id-arguments.ts`, so dashboards have their own grammar, but follow the + * same conventions: the result is a `targetArg` for `parseOrgProjectInput`, + * and parsing has no side effects. A pasted URL's server comes back as + * `baseUrl` and anything to tell the user as `warnings`, for the caller to + * apply. + */ + +import { + type DashboardUrl, + parse, +} from "../generated/dashboard-arguments-grammar.js"; +import { ValidationError } from "./errors.js"; +import { parseSentryUrl } from "./sentry-url-parser.js"; + +/** Dashboard arguments, parsed. */ +export type DashboardArgs = { + /** The numeric ID or title, as typed */ + readonly dashboardRef: string; + /** The org/project target; `undefined` to auto-detect */ + readonly targetArg?: string; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + /** Warnings to log, such as for an ID taken from a URL */ + readonly warnings: readonly string[]; +}; + +/** + * Parse the arguments of a command that takes one dashboard: + * `[<org/project>] <id-or-title>`, `<org>/[<project>/]<id>`, or a Sentry + * URL (see the grammar). + * + * @param usageHint - Usage shown when arguments are ignored + * @throws {ValidationError} When no dashboard is given + */ +export function parseDashboardArgs( + args: readonly string[], + usageHint: string, +): DashboardArgs { + const syntax = parse(grammarInput(args), { list: false, sentryUrl }); + const { ref, url } = syntax; + if (ref === undefined) { + if (url?.org) { + throw new ValidationError( + "Dashboard ID or title is required.\n\n" + + "The URL provided contains an org but no dashboard ID.\n" + + `Try: sentry dashboard <command> ${url.org}/ <id-or-title>`, + "dashboard", + ); + } + throw new ValidationError( + "Dashboard ID or title is required.", + "dashboard", + ); + } + const warnings: string[] = []; + if (url) { + warnings.push(urlWarning(url)); + } + if (syntax.extra.length > 0) { + warnings.push( + `Extra arguments ignored: ${syntax.extra.join(" ")}. Expected: ${usageHint}`, + ); + } + return { + dashboardRef: ref, + targetArg: syntax.target, + baseUrl: url?.baseUrl, + warnings, + }; +} + +/** `dashboard list` arguments, parsed. */ +export type DashboardListArgs = { + /** The org/project target; `undefined` to auto-detect */ + readonly targetArg?: string; + /** Title glob to filter by */ + readonly titleFilter?: string; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + /** Warnings to log, such as for an org taken from a URL */ + readonly warnings: readonly string[]; +}; + +/** + * Parse `dashboard list` arguments: an optional target and title filter. + * One argument without a slash is a filter (dashboards are org-scoped, so a + * project search would only pick an org), and a filter may span several + * arguments (see the grammar). + * + * @throws {ValidationError} For a dashboard URL, suggesting `dashboard view` + */ +export function parseDashboardListArgs( + args: readonly (string | undefined)[], +): DashboardListArgs { + const syntax = parse( + grammarInput(args.filter((arg): arg is string => !!arg)), + { list: true, sentryUrl }, + ); + const { url } = syntax; + if (url?.dashboardId) { + const orgPrefix = url.org ? `${url.org}/ ` : ""; + const orgSuffix = url.org ? ` ${url.org}/` : ""; + const orgNote = url.org ? ` in '${url.org}'` : ""; + throw new ValidationError( + "This looks like a dashboard URL. To view a specific dashboard:\n\n" + + ` sentry dashboard view ${orgPrefix}${url.dashboardId}\n\n` + + `To list dashboards${orgNote}:\n\n` + + ` sentry dashboard list${orgSuffix}`, + "dashboard", + ); + } + return { + targetArg: syntax.target, + titleFilter: syntax.filter, + baseUrl: url?.baseUrl, + warnings: url ? [urlWarning(url)] : [], + }; +} + +/** + * The grammar's input: a NUL before each argument. + * + * @throws {ValidationError} When an argument holds a NUL + */ +function grammarInput(args: readonly string[]): string { + if (args.some((arg) => arg.includes("\0"))) { + throw new ValidationError( + "Arguments cannot contain NUL characters.", + "positional", + ); + } + return args.map((arg) => `\0${arg}`).join(""); +} + +function sentryUrl(value: string): DashboardUrl | null { + const url = parseSentryUrl(value); + return ( + url && { + baseUrl: url.baseUrl, + ...(url.org && { org: url.org }), + ...(url.dashboardId && { dashboardId: url.dashboardId }), + } + ); +} + +function urlWarning(url: DashboardUrl): string { + if (url.dashboardId) { + return ( + `Extracted dashboard ID ${url.dashboardId} from URL` + + (url.org ? ` (org: ${url.org})` : "") + ); + } + return `Extracted org '${url.org}' from URL`; +} diff --git a/packages/cli/test/commands/dashboard/list.test.ts b/packages/cli/test/commands/dashboard/list.test.ts index 1d6f322e7..90fd2d317 100644 --- a/packages/cli/test/commands/dashboard/list.test.ts +++ b/packages/cli/test/commands/dashboard/list.test.ts @@ -10,6 +10,7 @@ * integration tests focus on end-to-end behavior through the Stricli func(). */ +import { ArgumentParseError } from "@stricli/core"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { @@ -435,6 +436,25 @@ describe("dashboard list command", () => { expect(parsed.data[0].title).toBe("API Monitoring"); }); + test("a dashboard URL fails to parse with a view suggestion, before any request", async () => { + const { context } = createMockContext(); + const func = await listCommand.loader(); + + const error = await func + .call( + context, + defaultFlags({ json: true }), + "https://my-org.sentry.io/dashboard/4326879/", + ) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ArgumentParseError); + expect(String((error as ArgumentParseError).exception)).toContain( + "sentry dashboard view my-org/ 4326879", + ); + expect(listDashboardsPaginatedSpy).not.toHaveBeenCalled(); + }); + test("glob filter is case-insensitive", async () => { resolveOrgSpy.mockResolvedValue("test-org"); listDashboardsPaginatedSpy.mockResolvedValue({ diff --git a/packages/cli/test/commands/dashboard/resolve.test.ts b/packages/cli/test/commands/dashboard/resolve.test.ts index a85a813ad..3d86e0444 100644 --- a/packages/cli/test/commands/dashboard/resolve.test.ts +++ b/packages/cli/test/commands/dashboard/resolve.test.ts @@ -1,8 +1,9 @@ /** * Dashboard Resolution Utility Tests * - * Tests for positional argument parsing, dashboard ID resolution, - * and org resolution in src/commands/dashboard/resolve.ts. + * Tests for dashboard ID resolution and org resolution in + * src/commands/dashboard/resolve.ts. Argument parsing is tested in + * test/lib/dashboard-arguments.test.ts. */ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; @@ -12,8 +13,6 @@ import { DEFAULT_GROUP_BY_LIMIT, enrichDashboardError, normalizeDataset, - parseDashboardListArgs, - parseDashboardPositionalArgs, resolveDashboardId, resolveOrgFromTarget, resolveWidgetIndex, @@ -70,264 +69,6 @@ vi.mock("../../../src/lib/resolve-target.js", async (importOriginal) => { // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as resolveTarget from "../../../src/lib/resolve-target.js"; -// --------------------------------------------------------------------------- -// parseDashboardPositionalArgs -// --------------------------------------------------------------------------- - -describe("parseDashboardPositionalArgs", () => { - test("throws ValidationError for empty args", () => { - expect(() => parseDashboardPositionalArgs([])).toThrow(ValidationError); - }); - - test("error message contains 'Dashboard ID or title'", () => { - try { - parseDashboardPositionalArgs([]); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - expect((error as ValidationError).message).toContain( - "Dashboard ID or title", - ); - } - }); - - test("single arg returns dashboardRef only", () => { - const result = parseDashboardPositionalArgs(["123"]); - expect(result.dashboardRef).toBe("123"); - expect(result.targetArg).toBeUndefined(); - }); - - test.each([ - ["my-org/12345", "my-org/"], - ["my-org/web_app/12345", "my-org/web_app"], - ])("extracts a numeric dashboard ID from %s", (input, targetArg) => { - expect(parseDashboardPositionalArgs([input])).toEqual({ - dashboardRef: "12345", - targetArg, - }); - }); - - test.each([ - "Latency/p95", - "Errors / 2026", - "My Dashboard/123", - "Errors.v2/123", - "org/project/title", - "org/", - "/123", - "org//123", - "org/a/b/123", - ])("preserves %s as a dashboard title", (title) => { - expect(parseDashboardPositionalArgs([title])).toEqual({ - dashboardRef: title, - targetArg: undefined, - }); - }); - - test("preserves a numeric slash title with a separate target", () => { - expect(parseDashboardPositionalArgs(["my-org/", "incidents/2026"])).toEqual( - { - dashboardRef: "incidents/2026", - targetArg: "my-org/", - }, - ); - }); - - test("two args returns target + dashboardRef", () => { - const result = parseDashboardPositionalArgs(["my-org/", "My Dashboard"]); - expect(result.dashboardRef).toBe("My Dashboard"); - expect(result.targetArg).toBe("my-org/"); - }); - - test("two args with bare org slug auto-appends /", () => { - const result = parseDashboardPositionalArgs(["my-org", "12345"]); - expect(result.dashboardRef).toBe("12345"); - expect(result.targetArg).toBe("my-org/"); - }); - - test("two args with org/project is unchanged", () => { - const result = parseDashboardPositionalArgs(["my-org/my-project", "12345"]); - expect(result.dashboardRef).toBe("12345"); - expect(result.targetArg).toBe("my-org/my-project"); - }); - - // -- URL auto-recovery -- - - test("extracts dashboard ID and org from SaaS subdomain URL", () => { - const result = parseDashboardPositionalArgs([ - "https://sentry-sdks.sentry.io/dashboard/4326879/", - ]); - expect(result.dashboardRef).toBe("4326879"); - expect(result.targetArg).toBe("sentry-sdks/"); - }); - - test("extracts dashboard ID and org from organizations-path URL", () => { - const result = parseDashboardPositionalArgs([ - "https://sentry.io/organizations/my-org/dashboard/12345/", - ]); - expect(result.dashboardRef).toBe("12345"); - expect(result.targetArg).toBe("my-org/"); - }); - - test("URL with org but no dashboard ID + second arg uses second arg as ref", () => { - const result = parseDashboardPositionalArgs([ - "https://my-org.sentry.io/", - "My Dashboard", - ]); - expect(result.dashboardRef).toBe("My Dashboard"); - expect(result.targetArg).toBe("my-org/"); - }); - - test("URL with org but no dashboard ID and no second arg throws", () => { - expect(() => - parseDashboardPositionalArgs(["https://my-org.sentry.io/"]), - ).toThrow(ValidationError); - }); -}); - -// --------------------------------------------------------------------------- -// parseDashboardListArgs -// --------------------------------------------------------------------------- - -describe("parseDashboardListArgs", () => { - test("empty args returns undefined for both", () => { - const result = parseDashboardListArgs([]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBeUndefined(); - }); - - test("single arg with trailing slash is target", () => { - const result = parseDashboardListArgs(["my-org/"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBeUndefined(); - }); - - test("single arg with glob * is title filter", () => { - const result = parseDashboardListArgs(["Error*"]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBe("Error*"); - }); - - test("single arg with space is title filter", () => { - const result = parseDashboardListArgs(["My Dashboard"]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBe("My Dashboard"); - }); - - test("single arg with glob ? is title filter", () => { - const result = parseDashboardListArgs(["?something"]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBe("?something"); - }); - - test("single arg with glob [ is title filter", () => { - const result = parseDashboardListArgs(["[abc]"]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBe("[abc]"); - }); - - test("single bare word is title filter (dashboards are org-scoped)", () => { - const result = parseDashboardListArgs(["performance"]); - expect(result.targetArg).toBeUndefined(); - expect(result.titleFilter).toBe("performance"); - }); - - test("two args with trailing slash target and glob filter", () => { - const result = parseDashboardListArgs(["my-org/", "Error*"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBe("Error*"); - }); - - test("two args with bare org slug auto-appends /", () => { - const result = parseDashboardListArgs(["my-org", "Error*"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBe("Error*"); - }); - - test("two args with org/project and glob filter", () => { - const result = parseDashboardListArgs(["my-org/my-project", "*API*"]); - expect(result.targetArg).toBe("my-org/my-project"); - expect(result.titleFilter).toBe("*API*"); - }); - - test("multi-word unquoted filter: remaining args joined with spaces", () => { - const result = parseDashboardListArgs(["sentry/cli", "CLI", "Health"]); - expect(result.targetArg).toBe("sentry/cli"); - expect(result.titleFilter).toBe("CLI Health"); - }); - - test("multi-word unquoted filter with bare org", () => { - const result = parseDashboardListArgs(["my-org", "Error", "Overview"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBe("Error Overview"); - }); - - test("single arg org/project/name splits into target + filter", () => { - const result = parseDashboardListArgs(["sentry/cli/CLI"]); - expect(result.targetArg).toBe("sentry/cli"); - expect(result.titleFilter).toBe("CLI"); - }); - - test("single arg org/project/name with spaces in name", () => { - const result = parseDashboardListArgs(["sentry/cli/My Dashboard"]); - expect(result.targetArg).toBe("sentry/cli"); - expect(result.titleFilter).toBe("My Dashboard"); - }); - - test("single arg org/ is target only (one slash, trailing)", () => { - const result = parseDashboardListArgs(["my-org/"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBeUndefined(); - }); - - test("single arg org/project is target only (one slash)", () => { - const result = parseDashboardListArgs(["my-org/my-project"]); - expect(result.targetArg).toBe("my-org/my-project"); - expect(result.titleFilter).toBeUndefined(); - }); - - test("single arg org/project/ is target only (trailing slash after project)", () => { - const result = parseDashboardListArgs(["my-org/my-project/"]); - expect(result.targetArg).toBe("my-org/my-project/"); - expect(result.titleFilter).toBeUndefined(); - }); - - // -- URL handling -- - - test("dashboard URL throws ValidationError suggesting view command", () => { - expect(() => - parseDashboardListArgs([ - "https://sentry-sdks.sentry.io/dashboard/4326879/", - ]), - ).toThrow(ValidationError); - try { - parseDashboardListArgs([ - "https://sentry-sdks.sentry.io/dashboard/4326879/", - ]); - } catch (error) { - const msg = (error as ValidationError).message; - expect(msg).toContain("sentry dashboard view"); - expect(msg).toContain("4326879"); - expect(msg).toContain("sentry-sdks"); - } - }); - - test("org-only URL extracts org as target", () => { - const result = parseDashboardListArgs(["https://my-org.sentry.io/"]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBeUndefined(); - }); - - test("org-only URL with second arg uses it as title filter", () => { - const result = parseDashboardListArgs([ - "https://my-org.sentry.io/", - "Error*", - ]); - expect(result.targetArg).toBe("my-org/"); - expect(result.titleFilter).toBe("Error*"); - }); -}); - // --------------------------------------------------------------------------- // resolveDashboardId // --------------------------------------------------------------------------- diff --git a/packages/cli/test/commands/dashboard/restore.test.ts b/packages/cli/test/commands/dashboard/restore.test.ts index 05707ed9f..3515496c5 100644 --- a/packages/cli/test/commands/dashboard/restore.test.ts +++ b/packages/cli/test/commands/dashboard/restore.test.ts @@ -5,6 +5,7 @@ * Uses spyOn pattern to mock API client, resolve, and polling modules. */ +import { ArgumentParseError } from "@stricli/core"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as resolve from "../../../src/commands/dashboard/resolve.js"; @@ -13,6 +14,7 @@ import { restoreCommand } from "../../../src/commands/dashboard/restore.js"; import * as apiClient from "../../../src/lib/api-client.js"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- needed for spyOn mocking import * as polling from "../../../src/lib/polling.js"; +import { ValidationError } from "../../../src/lib/errors.js"; import type { DashboardDetail } from "../../../src/types/dashboard.js"; // --------------------------------------------------------------------------- @@ -203,6 +205,41 @@ describe("dashboard restore command", () => { ); }); + test("takes the org and dashboard ID from a pasted dashboard URL", async () => { + const { context } = createMockContext(); + const func = await restoreCommand.loader(); + await func.call( + context, + defaultFlags({ json: true, revision: "3" }), + "https://my-org.sentry.io/dashboard/4326879/", + ); + + expect(resolveOrgFromTargetSpy).toHaveBeenCalledWith( + { type: "org-all", org: "my-org" }, + "/tmp", + expect.any(String), + ); + expect(resolveDashboardIdSpy).toHaveBeenCalledWith("test-org", "4326879"); + }); + + test("fails to parse a missing dashboard before any request", async () => { + const { context } = createMockContext(); + const func = await restoreCommand.loader(); + + const error = await func + .call(context, defaultFlags({ revision: "3" })) + .catch((caught: unknown) => caught); + + expect(error).toBeInstanceOf(ArgumentParseError); + const { exception } = error as ArgumentParseError; + expect(exception).toBeInstanceOf(ValidationError); + expect((exception as ValidationError).message).toContain( + "Dashboard ID or title is required", + ); + expect(resolveOrgFromTargetSpy).not.toHaveBeenCalled(); + expect(restoreDashboardRevisionSpy).not.toHaveBeenCalled(); + }); + // ------------------------------------------------------------------------- // Error handling // ------------------------------------------------------------------------- diff --git a/packages/cli/test/commands/dashboard/widget/add.test.ts b/packages/cli/test/commands/dashboard/widget/add.test.ts index 5d9de84bc..ac783e818 100644 --- a/packages/cli/test/commands/dashboard/widget/add.test.ts +++ b/packages/cli/test/commands/dashboard/widget/add.test.ts @@ -5,6 +5,7 @@ * Uses spyOn pattern to mock API client and resolve-target. */ +import { ArgumentParseError } from "@stricli/core"; import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { addCommand } from "../../../../src/commands/dashboard/widget/add.js"; @@ -239,15 +240,20 @@ describe("dashboard widget add", () => { expect(output).toContain("Error Rate"); }); - test("throws ValidationError when title is missing (less than 2 positional args)", async () => { + test("fails to parse the arguments when the title is missing (less than 2 positional args)", async () => { const { context } = createMockContext(); const func = await addCommand.loader(); const err = await func .call(context, { json: false, display: "line" }, "123") .catch((e: Error) => e); - expect(err).toBeInstanceOf(ValidationError); - expect(err.message).toContain("Widget title is required"); + expect(err).toBeInstanceOf(ArgumentParseError); + const { exception } = err as ArgumentParseError; + expect(exception).toBeInstanceOf(ValidationError); + expect((exception as ValidationError).message).toContain( + "Widget title is required", + ); + expect(getDashboardSpy).not.toHaveBeenCalled(); }); test("throws ValidationError for invalid display type", async () => { diff --git a/packages/cli/test/lib/dashboard-arguments.test.ts b/packages/cli/test/lib/dashboard-arguments.test.ts new file mode 100644 index 000000000..76179fa94 --- /dev/null +++ b/packages/cli/test/lib/dashboard-arguments.test.ts @@ -0,0 +1,364 @@ +/** + * Dashboard argument parsing tests (src/lib/dashboard-arguments.ts and + * grammar/dashboard-arguments.pegjs). + */ + +import { describe, expect, test } from "vitest"; +import { + parseDashboardArgs, + parseDashboardListArgs, +} from "../../src/lib/dashboard-arguments.js"; +import { ValidationError } from "../../src/lib/errors.js"; + +const USAGE_HINT = "sentry dashboard view <org>/ <id>"; + +function parseRef(args: string[]) { + return parseDashboardArgs(args, USAGE_HINT); +} + +// --------------------------------------------------------------------------- +// parseDashboardArgs +// --------------------------------------------------------------------------- + +describe("parseDashboardArgs", () => { + test("throws ValidationError for empty args", () => { + expect(() => parseRef([])).toThrow(ValidationError); + }); + + test("error message contains 'Dashboard ID or title'", () => { + try { + parseRef([]); + expect.unreachable("Should have thrown"); + } catch (error) { + expect(error).toBeInstanceOf(ValidationError); + expect((error as ValidationError).message).toContain( + "Dashboard ID or title", + ); + } + }); + + test("single arg returns dashboardRef only", () => { + const result = parseRef(["123"]); + expect(result.dashboardRef).toBe("123"); + expect(result.targetArg).toBeUndefined(); + }); + + test.each([ + ["my-org/12345", "my-org/"], + ["my-org/web_app/12345", "my-org/web_app"], + ])("extracts a numeric dashboard ID from %s", (input, targetArg) => { + expect(parseRef([input])).toEqual({ + dashboardRef: "12345", + targetArg, + warnings: [], + }); + }); + + test.each([ + "Latency/p95", + "Errors / 2026", + "My Dashboard/123", + "Errors.v2/123", + "org/project/title", + "org/", + "/123", + "org//123", + "org/a/b/123", + ])("preserves %s as a dashboard title", (title) => { + expect(parseRef([title])).toEqual({ + dashboardRef: title, + targetArg: undefined, + warnings: [], + }); + }); + + test("preserves a numeric slash title with a separate target", () => { + expect(parseRef(["my-org/", "incidents/2026"])).toEqual({ + dashboardRef: "incidents/2026", + targetArg: "my-org/", + warnings: [], + }); + }); + + test("two args returns target + dashboardRef", () => { + const result = parseRef(["my-org/", "My Dashboard"]); + expect(result.dashboardRef).toBe("My Dashboard"); + expect(result.targetArg).toBe("my-org/"); + }); + + test("two args with bare org slug auto-appends /", () => { + const result = parseRef(["my-org", "12345"]); + expect(result.dashboardRef).toBe("12345"); + expect(result.targetArg).toBe("my-org/"); + }); + + test("two args with org/project is unchanged", () => { + const result = parseRef(["my-org/my-project", "12345"]); + expect(result.dashboardRef).toBe("12345"); + expect(result.targetArg).toBe("my-org/my-project"); + }); + + // -- URL auto-recovery -- + + test("extracts dashboard ID and org from SaaS subdomain URL", () => { + const result = parseRef([ + "https://sentry-sdks.sentry.io/dashboard/4326879/", + ]); + expect(result.dashboardRef).toBe("4326879"); + expect(result.targetArg).toBe("sentry-sdks/"); + }); + + test("extracts dashboard ID and org from organizations-path URL", () => { + const result = parseRef([ + "https://sentry.io/organizations/my-org/dashboard/12345/", + ]); + expect(result.dashboardRef).toBe("12345"); + expect(result.targetArg).toBe("my-org/"); + }); + + test("URL with org but no dashboard ID + second arg uses second arg as ref", () => { + const result = parseRef(["https://my-org.sentry.io/", "My Dashboard"]); + expect(result.dashboardRef).toBe("My Dashboard"); + expect(result.targetArg).toBe("my-org/"); + }); + + test("URL with org but no dashboard ID and no second arg throws", () => { + expect(() => parseRef(["https://my-org.sentry.io/"])).toThrow( + ValidationError, + ); + }); +}); + +// --------------------------------------------------------------------------- +// parseDashboardListArgs +// --------------------------------------------------------------------------- + +describe("parseDashboardListArgs", () => { + test("empty args returns undefined for both", () => { + const result = parseDashboardListArgs([]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBeUndefined(); + }); + + test("single arg with trailing slash is target", () => { + const result = parseDashboardListArgs(["my-org/"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBeUndefined(); + }); + + test("single arg with glob * is title filter", () => { + const result = parseDashboardListArgs(["Error*"]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBe("Error*"); + }); + + test("single arg with space is title filter", () => { + const result = parseDashboardListArgs(["My Dashboard"]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBe("My Dashboard"); + }); + + test("single arg with glob ? is title filter", () => { + const result = parseDashboardListArgs(["?something"]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBe("?something"); + }); + + test("single arg with glob [ is title filter", () => { + const result = parseDashboardListArgs(["[abc]"]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBe("[abc]"); + }); + + test("single bare word is title filter (dashboards are org-scoped)", () => { + const result = parseDashboardListArgs(["performance"]); + expect(result.targetArg).toBeUndefined(); + expect(result.titleFilter).toBe("performance"); + }); + + test("two args with trailing slash target and glob filter", () => { + const result = parseDashboardListArgs(["my-org/", "Error*"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBe("Error*"); + }); + + test("two args with bare org slug auto-appends /", () => { + const result = parseDashboardListArgs(["my-org", "Error*"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBe("Error*"); + }); + + test("two args with org/project and glob filter", () => { + const result = parseDashboardListArgs(["my-org/my-project", "*API*"]); + expect(result.targetArg).toBe("my-org/my-project"); + expect(result.titleFilter).toBe("*API*"); + }); + + test("multi-word unquoted filter: remaining args joined with spaces", () => { + const result = parseDashboardListArgs(["sentry/cli", "CLI", "Health"]); + expect(result.targetArg).toBe("sentry/cli"); + expect(result.titleFilter).toBe("CLI Health"); + }); + + test("multi-word unquoted filter with bare org", () => { + const result = parseDashboardListArgs(["my-org", "Error", "Overview"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBe("Error Overview"); + }); + + test("single arg org/project/name splits into target + filter", () => { + const result = parseDashboardListArgs(["sentry/cli/CLI"]); + expect(result.targetArg).toBe("sentry/cli"); + expect(result.titleFilter).toBe("CLI"); + }); + + test("single arg org/project/name with spaces in name", () => { + const result = parseDashboardListArgs(["sentry/cli/My Dashboard"]); + expect(result.targetArg).toBe("sentry/cli"); + expect(result.titleFilter).toBe("My Dashboard"); + }); + + test("single arg org/ is target only (one slash, trailing)", () => { + const result = parseDashboardListArgs(["my-org/"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBeUndefined(); + }); + + test("single arg org/project is target only (one slash)", () => { + const result = parseDashboardListArgs(["my-org/my-project"]); + expect(result.targetArg).toBe("my-org/my-project"); + expect(result.titleFilter).toBeUndefined(); + }); + + test("single arg org/project/ is target only (trailing slash after project)", () => { + const result = parseDashboardListArgs(["my-org/my-project/"]); + expect(result.targetArg).toBe("my-org/my-project/"); + expect(result.titleFilter).toBeUndefined(); + }); + + // -- URL handling -- + + test("dashboard URL throws ValidationError suggesting view command", () => { + expect(() => + parseDashboardListArgs([ + "https://sentry-sdks.sentry.io/dashboard/4326879/", + ]), + ).toThrow(ValidationError); + try { + parseDashboardListArgs([ + "https://sentry-sdks.sentry.io/dashboard/4326879/", + ]); + } catch (error) { + const msg = (error as ValidationError).message; + expect(msg).toContain("sentry dashboard view"); + expect(msg).toContain("4326879"); + expect(msg).toContain("sentry-sdks"); + } + }); + + test("org-only URL extracts org as target", () => { + const result = parseDashboardListArgs(["https://my-org.sentry.io/"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBeUndefined(); + }); + + test("org-only URL with second arg uses it as title filter", () => { + const result = parseDashboardListArgs([ + "https://my-org.sentry.io/", + "Error*", + ]); + expect(result.targetArg).toBe("my-org/"); + expect(result.titleFilter).toBe("Error*"); + }); +}); + +describe("parseDashboardArgs warnings and edge cases", () => { + test("a dashboard URL warns which ID it took, and returns its server", () => { + expect( + parseRef(["https://sentry.example.com/organizations/acme/dashboard/42/"]), + ).toEqual({ + dashboardRef: "42", + targetArg: "acme/", + baseUrl: "https://sentry.example.com", + warnings: ["Extracted dashboard ID 42 from URL (org: acme)"], + }); + }); + + test("an org URL warns which org it took", () => { + expect(parseRef(["https://acme.sentry.io/", "Errors"])).toEqual({ + dashboardRef: "Errors", + targetArg: "acme/", + baseUrl: "https://acme.sentry.io", + warnings: ["Extracted org 'acme' from URL"], + }); + }); + + test("arguments after the dashboard are ignored with a warning", () => { + expect(parseRef(["my-org", "My", "Dashboard"])).toEqual({ + dashboardRef: "My", + targetArg: "my-org/", + warnings: [`Extra arguments ignored: Dashboard. Expected: ${USAGE_HINT}`], + }); + }); + + test("arguments after a dashboard URL are ignored with a warning", () => { + const { dashboardRef, warnings } = parseRef([ + "https://acme.sentry.io/dashboard/7/", + "extra", + ]); + expect(dashboardRef).toBe("7"); + expect(warnings).toContain( + `Extra arguments ignored: extra. Expected: ${USAGE_HINT}`, + ); + }); + + test("an empty argument is still a dashboard reference", () => { + expect(parseRef([""])).toEqual({ dashboardRef: "", warnings: [] }); + expect(parseRef(["my-org", ""])).toEqual({ + dashboardRef: "", + targetArg: "my-org/", + warnings: [], + }); + }); + + test("an org URL without a dashboard suggests the org form", () => { + expect(() => parseRef(["https://acme.sentry.io/"])).toThrow( + "Try: sentry dashboard <command> acme/ <id-or-title>", + ); + }); + + test("rejects NUL characters", () => { + expect(() => parseRef(["a\0b"])).toThrow(ValidationError); + }); +}); + +describe("parseDashboardListArgs edge cases", () => { + test("skips omitted and empty arguments", () => { + expect(parseDashboardListArgs([undefined, "", "Error*"])).toEqual({ + titleFilter: "Error*", + warnings: [], + }); + }); + + test("an org URL warns which org it took", () => { + expect(parseDashboardListArgs(["https://acme.sentry.io/"])).toEqual({ + targetArg: "acme/", + baseUrl: "https://acme.sentry.io", + warnings: ["Extracted org 'acme' from URL"], + }); + }); + + test("an org URL joins the filter words after it", () => { + expect( + parseDashboardListArgs(["https://acme.sentry.io/", "CLI", "Health"]), + ).toMatchObject({ targetArg: "acme/", titleFilter: "CLI Health" }); + }); + + test("everything after the second slash is the filter", () => { + expect(parseDashboardListArgs(["a/b/c/d"])).toEqual({ + targetArg: "a/b", + titleFilter: "c/d", + warnings: [], + }); + }); +}); diff --git a/packages/cli/test/lib/introspect.test.ts b/packages/cli/test/lib/introspect.test.ts index 1787b1f5c..ee4c3e856 100644 --- a/packages/cli/test/lib/introspect.test.ts +++ b/packages/cli/test/lib/introspect.test.ts @@ -12,10 +12,10 @@ import { ISSUE_TARGET, ORG_PROJECT_TARGET, ORG_TARGET, - parseDashboardTarget, parseIssueTarget, parseOrgProjectTarget, parseOrgTarget, + withCompletionTarget, } from "../../src/lib/completion-target.js"; import type { Command, @@ -476,7 +476,15 @@ describe("extractCommandCompletions", () => { parameters: { positional: { kind: "tuple", - parameters: [{ placeholder: "target", parse: parseDashboardTarget }], + parameters: [ + { + placeholder: "target", + parse: withCompletionTarget( + DASHBOARD_TARGET, + (input: string) => input, + ), + }, + ], }, flags: { web: { kind: "boolean" }, From b2d10e3daa6028a160d2ad65d952702df42919b4 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 19:20:46 +0000 Subject: [PATCH 2/4] fix(cli): Don't import grammar types where lint runs before generation CI lints before the grammar is generated, so a type imported from src/generated/ is an error type there. Infer the URL shape instead, like id-arguments.ts does. --- packages/cli/src/lib/dashboard-arguments.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/lib/dashboard-arguments.ts b/packages/cli/src/lib/dashboard-arguments.ts index 2136a14b7..63dd04322 100644 --- a/packages/cli/src/lib/dashboard-arguments.ts +++ b/packages/cli/src/lib/dashboard-arguments.ts @@ -10,10 +10,7 @@ * apply. */ -import { - type DashboardUrl, - parse, -} from "../generated/dashboard-arguments-grammar.js"; +import { parse } from "../generated/dashboard-arguments-grammar.js"; import { ValidationError } from "./errors.js"; import { parseSentryUrl } from "./sentry-url-parser.js"; @@ -137,7 +134,8 @@ function grammarInput(args: readonly string[]): string { return args.map((arg) => `\0${arg}`).join(""); } -function sentryUrl(value: string): DashboardUrl | null { +/** The parts of the Sentry URL `value` is, or `null` for anything else. */ +function sentryUrl(value: string) { const url = parseSentryUrl(value); return ( url && { @@ -148,7 +146,10 @@ function sentryUrl(value: string): DashboardUrl | null { ); } -function urlWarning(url: DashboardUrl): string { +function urlWarning(url: { + readonly org?: string; + readonly dashboardId?: string; +}): string { if (url.dashboardId) { return ( `Extracted dashboard ID ${url.dashboardId} from URL` + From 73869451bfa9a525cf991feeb96c08d1e6a8d5db Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 20:04:33 +0000 Subject: [PATCH 3/4] feat(cli): Join an unquoted dashboard title after a target `dashboard view my-org My Dashboard` took "My" and ignored "Dashboard" with a warning. After a target or an org URL, the remaining arguments now form one title, like the `dashboard list` filter. Arguments after a dashboard URL are still extra. --- packages/cli/grammar/dashboard-arguments.d.ts | 2 +- .../cli/grammar/dashboard-arguments.pegjs | 20 +++++++++----- packages/cli/src/lib/dashboard-arguments.ts | 5 ++-- .../test/commands/dashboard/restore.test.ts | 17 ++++++++++++ .../cli/test/lib/dashboard-arguments.test.ts | 27 ++++++++++++++++--- 5 files changed, 58 insertions(+), 13 deletions(-) diff --git a/packages/cli/grammar/dashboard-arguments.d.ts b/packages/cli/grammar/dashboard-arguments.d.ts index 923f3edef..09d23af70 100644 --- a/packages/cli/grammar/dashboard-arguments.d.ts +++ b/packages/cli/grammar/dashboard-arguments.d.ts @@ -25,7 +25,7 @@ export type DashboardSyntax = { readonly target?: string; /** The URL the dashboard or org came from */ readonly url?: DashboardUrl; - /** Arguments after the dashboard */ + /** Arguments after a dashboard URL */ readonly extra: readonly string[]; }; diff --git a/packages/cli/grammar/dashboard-arguments.pegjs b/packages/cli/grammar/dashboard-arguments.pegjs index e64c5eec4..1da0fee26 100644 --- a/packages/cli/grammar/dashboard-arguments.pegjs +++ b/packages/cli/grammar/dashboard-arguments.pegjs @@ -4,14 +4,15 @@ // <id-or-title> the org is auto-detected // <org>/<id> like a URL: the org, then a numeric ID // <org>/<project>/<id> the project only selects the org -// <target> <id-or-title> any org/project target; a bare `<org>` +// <target> <id-or-title...> any org/project target; a bare `<org>` // means `<org>/` // <dashboard-url> a Sentry dashboard URL -// <org-url> <id-or-title> a Sentry URL with only an org +// <org-url> <id-or-title...> a Sentry URL with only an org // // A title may hold slashes: one argument is `<org>/[<project>/]<id>` only -// when slugs come before a numeric ID, and a title otherwise. Arguments -// after the dashboard are extra. +// when slugs come before a numeric ID, and a title otherwise. After a +// target, a title's words may be separate arguments (an unquoted title). +// Arguments after a dashboard URL are extra. // // With `list` (`dashboard list`), they name the org and a title filter: // @@ -45,6 +46,11 @@ function filter(words) { return words.length > 0 ? { filter: words.join(" ") } : {}; } + + // An unquoted title arrives as one argument per word + function title(first, more) { + return [first, ...more].join(" "); + } } // Peggy runs a rule's action as soon as that rule matches, even when the @@ -68,7 +74,7 @@ dashboardArguments } // A URL with only an org; the dashboard follows / sep u:orgUrl sep r:token x:rest { - return { ref: r, target: `${u.org}/`, url: u, extra: x }; + return { ref: title(r, x), target: `${u.org}/`, url: u, extra: [] }; } / sep u:orgUrl end { return { url: u, extra: [] }; } // `<org>/<project>/<id>` and `<org>/<id>` @@ -78,9 +84,9 @@ dashboardArguments / sep o:slug "/" i:digits end { return { ref: i, target: `${o}/`, extra: [] }; } // Anything else in one argument is an ID or a title, slashes and all / sep r:token end { return { ref: r, extra: [] }; } - // `<target> <id-or-title>` + // `<target> <id-or-title...>` / sep t:token sep r:token x:rest { - return { ref: r, target: orgTarget(t), extra: x }; + return { ref: title(r, x), target: orgTarget(t), extra: [] }; } listArguments diff --git a/packages/cli/src/lib/dashboard-arguments.ts b/packages/cli/src/lib/dashboard-arguments.ts index 63dd04322..6c6df4e17 100644 --- a/packages/cli/src/lib/dashboard-arguments.ts +++ b/packages/cli/src/lib/dashboard-arguments.ts @@ -29,9 +29,10 @@ export type DashboardArgs = { /** * Parse the arguments of a command that takes one dashboard: * `[<org/project>] <id-or-title>`, `<org>/[<project>/]<id>`, or a Sentry - * URL (see the grammar). + * URL (see the grammar). After a target, the remaining arguments are one + * title: `my-org/ My Dashboard` names "My Dashboard". * - * @param usageHint - Usage shown when arguments are ignored + * @param usageHint - Usage shown when arguments after a URL are ignored * @throws {ValidationError} When no dashboard is given */ export function parseDashboardArgs( diff --git a/packages/cli/test/commands/dashboard/restore.test.ts b/packages/cli/test/commands/dashboard/restore.test.ts index 3515496c5..3a2f33980 100644 --- a/packages/cli/test/commands/dashboard/restore.test.ts +++ b/packages/cli/test/commands/dashboard/restore.test.ts @@ -205,6 +205,23 @@ describe("dashboard restore command", () => { ); }); + test("joins an unquoted title after a target", async () => { + const { context } = createMockContext(); + const func = await restoreCommand.loader(); + await func.call( + context, + defaultFlags({ json: true, revision: "3" }), + "my-org/", + "Backend", + "Performance", + ); + + expect(resolveDashboardIdSpy).toHaveBeenCalledWith( + "test-org", + "Backend Performance", + ); + }); + test("takes the org and dashboard ID from a pasted dashboard URL", async () => { const { context } = createMockContext(); const func = await restoreCommand.loader(); diff --git a/packages/cli/test/lib/dashboard-arguments.test.ts b/packages/cli/test/lib/dashboard-arguments.test.ts index 76179fa94..a6154a6a9 100644 --- a/packages/cli/test/lib/dashboard-arguments.test.ts +++ b/packages/cli/test/lib/dashboard-arguments.test.ts @@ -293,11 +293,32 @@ describe("parseDashboardArgs warnings and edge cases", () => { }); }); - test("arguments after the dashboard are ignored with a warning", () => { + test("after a target, an unquoted title's words form one title", () => { expect(parseRef(["my-org", "My", "Dashboard"])).toEqual({ - dashboardRef: "My", + dashboardRef: "My Dashboard", targetArg: "my-org/", - warnings: [`Extra arguments ignored: Dashboard. Expected: ${USAGE_HINT}`], + warnings: [], + }); + expect(parseRef(["my-org/", "Backend", "Performance"])).toEqual({ + dashboardRef: "Backend Performance", + targetArg: "my-org/", + warnings: [], + }); + expect(parseRef(["my-org/web", "Errors", "/", "2026"])).toEqual({ + dashboardRef: "Errors / 2026", + targetArg: "my-org/web", + warnings: [], + }); + }); + + test("after an org URL, an unquoted title's words form one title", () => { + expect( + parseRef(["https://acme.sentry.io/", "Backend", "Performance"]), + ).toEqual({ + dashboardRef: "Backend Performance", + targetArg: "acme/", + baseUrl: "https://acme.sentry.io", + warnings: ["Extracted org 'acme' from URL"], }); }); From 72e999dc22b33074e0317d64ac3a6126fcb6bcd8 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:40:03 +0000 Subject: [PATCH 4/4] ref(cli): Parse dashboard and issue arguments with the shared ID grammar `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> --- packages/cli/AGENTS.md | 32 +- packages/cli/grammar/dashboard-arguments.d.ts | 56 --- .../cli/grammar/dashboard-arguments.pegjs | 133 ------- packages/cli/grammar/id-arguments.d.ts | 11 + packages/cli/grammar/id-arguments.pegjs | 74 +++- .../cli/src/commands/dashboard/arguments.ts | 71 +++- .../cli/src/commands/dashboard/resolve.ts | 31 ++ packages/cli/src/commands/event/view.ts | 38 +- packages/cli/src/commands/issue/utils.ts | 63 ++++ packages/cli/src/commands/log/view.ts | 8 +- packages/cli/src/commands/span/view.ts | 2 +- packages/cli/src/lib/dashboard-arguments.ts | 167 +++++---- packages/cli/src/lib/id-arguments.ts | 224 +++++++++++- packages/cli/src/lib/issue-arguments.ts | 318 ++--------------- packages/cli/src/lib/issue-identifier.ts | 327 ++++++++++++++++++ .../test/commands/dashboard/arguments.test.ts | 179 ++++++++++ packages/cli/test/commands/event/view.test.ts | 12 + .../cli/test/commands/issue/utils.test.ts | 65 ++++ .../cli/test/lib/arg-parsing.property.test.ts | 3 +- packages/cli/test/lib/arg-parsing.test.ts | 18 +- .../cli/test/lib/dashboard-arguments.test.ts | 73 +++- packages/cli/test/lib/id-arguments.test.ts | 154 ++++++++- 22 files changed, 1441 insertions(+), 618 deletions(-) delete mode 100644 packages/cli/grammar/dashboard-arguments.d.ts delete mode 100644 packages/cli/grammar/dashboard-arguments.pegjs create mode 100644 packages/cli/src/lib/issue-identifier.ts create mode 100644 packages/cli/test/commands/dashboard/arguments.test.ts diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index b1568b829..24595dcd5 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -248,21 +248,33 @@ target is known. The result is a `target` string for `parseOrgProjectInput` and the IDs. Add a kind to `ID_KINDS` rather than validating IDs in a command; the `sentry-cli/no-ad-hoc-id-arguments` lint rule rejects hand-written ID parsing (`parseSlashSeparatedArg`, `HEX_ID_RE`, ...) in commands and kinds -defined outside `ID_KINDS`. Issue IDs keep their own syntax; `event view`, -which also takes them, checks for an issue first (`parseEventViewInput`). -Dashboards, named by numeric ID or title, have their own grammar -(`grammar/dashboard-arguments.pegjs`); dashboard commands take -`dashboardArguments` from `commands/dashboard/arguments.ts`. +defined outside `ID_KINDS`. A kind without names gets a hint to search +(`search`) when text is passed for an ID: `validate()` throws it, so it comes +after any recovery. + +Some kinds name items without an ID (`IdKind.names`): a dashboard by its +title (`ID_KINDS.dashboard`; unquoted words after a target are one title), +an issue by search text (`ID_KINDS.issue`). Then what is not an ID +comes back as `name`. A bare first word before a name comes back as +`nameWithTarget` too: the parser does not decide whether it names an org, so +dashboard commands check once they resolve (`resolveDashboardInput`), and +fall back to the whole input as the title. Dashboard commands take +`dashboardArguments` from `commands/dashboard/arguments.ts`; `dashboard list` +reads a target and a title filter instead (`parseDashboardListArgs`). +`event view` checks for an issue before an event ID, with +`ID_KINDS.issue.distinctForm` (`parseEventViewInput`). `test/lib/argument-conventions.test.ts` feeds the same inputs to every command that takes IDs, issues, or releases, through the command's own positional; add a new command to its tables, and encode a legitimate difference (such as `needsProject`) in its row. -**Issue positionals:** issue IDs, short IDs, selectors, and issue URLs are -parsed by `parseIssueArgument` (`src/lib/issue-arguments.ts`), whose forms -are defined by `grammar/issue-argument.pegjs`. A command that takes one -issue declares `issuePositional({ command })` (or `issueParameter` inside a -tuple) from `src/commands/issue/positional.ts`: `func` takes an +**Issue positionals:** issue IDs, short IDs, selectors, issue URLs, and +search text are parsed by `parseIssueArgument` (`src/lib/issue-arguments.ts`) +with `ID_KINDS.issue`. An identifier is one whole token, which +`src/lib/issue-identifier.ts` takes apart with `grammar/issue-argument.pegjs`; +text with spaces is a search, which resolves to the one issue it matches. +A command that takes one issue declares `issuePositional({ command })` (or +`issueParameter` inside a tuple) from `src/commands/issue/positional.ts`: `func` takes an `IssueTarget` and awaits `target.resolved` for the org and issue. A command that resolves it another way (`resolveOrgAndIssueId`, or recovery hints that keep its other arguments) passes `requireIssueArgument(target.parsed, ...)` diff --git a/packages/cli/grammar/dashboard-arguments.d.ts b/packages/cli/grammar/dashboard-arguments.d.ts deleted file mode 100644 index 09d23af70..000000000 --- a/packages/cli/grammar/dashboard-arguments.d.ts +++ /dev/null @@ -1,56 +0,0 @@ -// Types of the parser generated from dashboard-arguments.pegjs. The -// generator copies this file to src/generated/dashboard-arguments-grammar.d.ts -// and rewrites relative imports to resolve from there. - -/** The parts of a Sentry URL that dashboard arguments use. */ -export type DashboardUrl = { - readonly baseUrl: string; - readonly org?: string; - readonly dashboardId?: string; -}; - -/** What the caller passes the grammar. See `parseDashboardArgs()`. */ -export type DashboardArgumentsGrammarOptions = { - /** Whether these are `dashboard list` arguments: a target and a filter */ - list: boolean; - /** The Sentry URL `value` is, or `null` for anything else */ - sentryUrl: (value: string) => DashboardUrl | null; -}; - -/** Which argument is the dashboard and which its target. */ -export type DashboardSyntax = { - /** The numeric ID or title; absent when none was given */ - readonly ref?: string; - /** The org/project target, as typed, or `<org>/` */ - readonly target?: string; - /** The URL the dashboard or org came from */ - readonly url?: DashboardUrl; - /** Arguments after a dashboard URL */ - readonly extra: readonly string[]; -}; - -/** Which argument is the target and which the title filter. */ -export type DashboardListSyntax = { - readonly target?: string; - /** The title filter, its words joined by spaces */ - readonly filter?: string; - /** The URL the org came from, or a dashboard URL, which lists nothing */ - readonly url?: DashboardUrl; -}; - -export declare class SyntaxError extends Error { - expected: unknown; - found: string | null; - location: unknown; -} - -/** Parse NUL-prefixed arguments into a dashboard and its target. */ -export declare function parse( - input: string, - options: DashboardArgumentsGrammarOptions & { list: false }, -): DashboardSyntax; -/** Parse NUL-prefixed arguments into a list target and title filter. */ -export declare function parse( - input: string, - options: DashboardArgumentsGrammarOptions & { list: true }, -): DashboardListSyntax; diff --git a/packages/cli/grammar/dashboard-arguments.pegjs b/packages/cli/grammar/dashboard-arguments.pegjs deleted file mode 100644 index 1da0fee26..000000000 --- a/packages/cli/grammar/dashboard-arguments.pegjs +++ /dev/null @@ -1,133 +0,0 @@ -// Grammar for dashboard positionals. Dashboards are org-scoped, so these -// name a dashboard and, optionally, the org to find it in: -// -// <id-or-title> the org is auto-detected -// <org>/<id> like a URL: the org, then a numeric ID -// <org>/<project>/<id> the project only selects the org -// <target> <id-or-title...> any org/project target; a bare `<org>` -// means `<org>/` -// <dashboard-url> a Sentry dashboard URL -// <org-url> <id-or-title...> a Sentry URL with only an org -// -// A title may hold slashes: one argument is `<org>/[<project>/]<id>` only -// when slugs come before a numeric ID, and a title otherwise. After a -// target, a title's words may be separate arguments (an unquoted title). -// Arguments after a dashboard URL are extra. -// -// With `list` (`dashboard list`), they name the org and a title filter: -// -// (none) the org is auto-detected, no filter -// <filter> an argument without a slash -// <org>/ or <org>/<project> a target, no filter -// <org>/<project>/<filter> a target, then the filter -// <target> <filter...> the filter's words may be separate -// arguments -// <org-url> [<filter...>] a Sentry URL with only an org -// -// `pnpm run generate:grammar` compiles it, with the types in -// dashboard-arguments.d.ts, into the gitignored src/generated/; the build, -// typecheck, and test scripts run it. -// -// `parseDashboardArgs()` and `parseDashboardListArgs()` -// (src/lib/dashboard-arguments.ts) put a NUL, which no argument contains, -// before each argument. The grammar only decides which argument is what; it -// accepts any input and never throws. The caller reports a missing -// dashboard, turns a URL into warnings, and parses the target like any -// org/project target. - -{ - const { list, sentryUrl } = options; - - // A bare argument before the dashboard names an org - function orgTarget(value) { - return value.includes("/") ? value : `${value}/`; - } - - function filter(words) { - return words.length > 0 ? { filter: words.join(" ") } : {}; - } - - // An unquoted title arrives as one argument per word - function title(first, more) { - return [first, ...more].join(" "); - } -} - -// Peggy runs a rule's action as soon as that rule matches, even when the -// enclosing alternative fails later and the parser backtracks. Every action -// here only builds values, and `sentryUrl` is pure, so that is harmless. - -arguments - = &{ return list; } @listArguments - / dashboardArguments - -dashboardArguments - = end { return { extra: [] }; } - // A dashboard URL; anything after it is extra - / sep u:dashboardUrl x:rest { - return { - ref: u.dashboardId, - ...(u.org && { target: `${u.org}/` }), - url: u, - extra: x, - }; - } - // A URL with only an org; the dashboard follows - / sep u:orgUrl sep r:token x:rest { - return { ref: title(r, x), target: `${u.org}/`, url: u, extra: [] }; - } - / sep u:orgUrl end { return { url: u, extra: [] }; } - // `<org>/<project>/<id>` and `<org>/<id>` - / sep o:slug "/" p:slug "/" i:digits end { - return { ref: i, target: `${o}/${p}`, extra: [] }; - } - / sep o:slug "/" i:digits end { return { ref: i, target: `${o}/`, extra: [] }; } - // Anything else in one argument is an ID or a title, slashes and all - / sep r:token end { return { ref: r, extra: [] }; } - // `<target> <id-or-title...>` - / sep t:token sep r:token x:rest { - return { ref: title(r, x), target: orgTarget(t), extra: [] }; - } - -listArguments - = end { return {}; } - // A dashboard URL names no list; the caller suggests `dashboard view` - / sep u:dashboardUrl rest { return { url: u }; } - / sep u:orgUrl x:rest { return { target: `${u.org}/`, url: u, ...filter(x) }; } - // `<target> <filter...>` - / sep t:token x:rest &{ return x.length > 0; } { - return { target: orgTarget(t), ...filter(x) }; - } - // `<org>/<project>/<filter>`: the filter is everything after the second - // slash. With nothing after it, the whole argument is the target. - / sep t:$([^/\0]* "/" [^/\0]*) "/" f:$[^\0]+ end { return { target: t, filter: f }; } - / sep t:$([^/\0]* "/" [^\0]*) end { return { target: t }; } - / sep f:token end { return { filter: f }; } - -// The arguments after the ones a form names -rest - = (sep @token)* - -// A Sentry URL naming a dashboard -dashboardUrl - = u:token &{ return sentryUrl(u)?.dashboardId !== undefined; } - { return sentryUrl(u); } - -// A Sentry URL naming an org but no dashboard -orgUrl - = u:token &{ - const url = sentryUrl(u); - return url?.org !== undefined && url.dashboardId === undefined; - } - { return sentryUrl(u); } - -slug = $[a-zA-Z0-9_-]+ - -digits = $[0-9]+ - -// Possibly empty: an empty argument is still one -token = $[^\0]* - -sep = "\0" - -end = !. diff --git a/packages/cli/grammar/id-arguments.d.ts b/packages/cli/grammar/id-arguments.d.ts index e891f0631..31fd0c502 100644 --- a/packages/cli/grammar/id-arguments.d.ts +++ b/packages/cli/grammar/id-arguments.d.ts @@ -22,6 +22,13 @@ export type IdArgumentsGrammarOptions = { mayBeId: (value: string) => boolean; /** The ID in a Sentry URL, or `null` for anything else. */ idUrl: (value: string) => IdUrl | null; + /** Whether the kind names items without an ID, too */ + names: boolean; + /** + * Whether an ID is a whole token, with its own syntax for the org, + * project, and URL. Only kinds with names use it. + */ + wholeIds: boolean; }; /** A token that names an ID, maybe after a target: `[<target>/]<id>`. */ @@ -45,6 +52,10 @@ export type IdArgumentsSyntax = { readonly baseUrl?: string; /** Whether the ID came before the target */ readonly reversed?: boolean; + /** The name, when the tokens name no ID (kinds with names only) */ + readonly name?: string; + /** Whether `target` is a bare word before `name`, maybe its first word */ + readonly bareTarget?: boolean; }; export declare class SyntaxError extends Error { diff --git a/packages/cli/grammar/id-arguments.pegjs b/packages/cli/grammar/id-arguments.pegjs index cb02204f5..74de968f3 100644 --- a/packages/cli/grammar/id-arguments.pegjs +++ b/packages/cli/grammar/id-arguments.pegjs @@ -15,6 +15,20 @@ // (`<target>/<id>`). The ID is always the last path segment, and slashes // pasted after it are ignored. // +// A kind may also name an item without its ID (`options.names`), such as a +// dashboard by its title. Then anything that is not an ID is a name rather +// than a target or a malformed ID: +// +// <name> one token that is no `[<target>/]<id>`, +// slashes and all +// <target> <name> after a target; the name takes all the +// tokens left, joined with spaces +// +// A token after a target is an ID when the kind says it is one, or may be +// one; the rest of the tokens are then IDs too. A kind whose IDs carry their +// own org, project, and URL syntax (`options.wholeIds`, such as issue short +// IDs) has its IDs as whole tokens, for the kind to take apart. +// // `pnpm run generate:grammar` compiles it, with the types in // id-arguments.d.ts, into the gitignored src/generated/; the build, // typecheck, and test scripts run it. @@ -26,7 +40,12 @@ // the IDs, validates them, and parses the target like any org/project target. { - const { optional, isId, mayBeId, idUrl } = options; + const { optional, isId, mayBeId, idUrl, names, wholeIds } = options; + + // Whether a token is, or may be, an ID rather than a name + function idLike(value) { + return isId(value) || mayBeId(value); + } // `[<org>/[<project>/]]` before an ID; the caller makes it a target function pathTarget(prefix) { @@ -51,6 +70,7 @@ arguments ...(u.baseUrl && { baseUrl: u.baseUrl }), }; } + / &{ return names; } @namedArguments // Any other URL is a target / sep t:url x:more { return { target: t, ids: x }; } // `<id> <org>/<project> [<id>...]`: the arguments are reversed. When IDs @@ -83,6 +103,48 @@ arguments // `<target> [<id>...]` / sep t:token x:more { return { target: t, ids: x }; } +// The arguments of a kind with names. A name is never a malformed ID: what +// may be an ID is one. +namedArguments + // An ID with its own syntax: the whole token, and whole tokens after it + = &{ return wholeIds; } sep i:token &{ return idLike(i); } + x:(sep t:token { return { text: t, id: t }; })* + { return { ids: [{ text: i, id: i }, ...x] }; } + // Any other URL is a target + / sep t:url n:afterTarget { return { target: t, ...n }; } + // `<id> <org>/<project> [<id>...]`: reversed; only a real ID can come + // first + / !{ return wholeIds; } sep i:segment &{ return isId(i); } sep !idPath + t:$([^/\0]* "/" [^\0]*) x:more + { return { target: t, ids: [{ text: i, id: i }, ...x], reversed: true }; } + // `[<org>/[<project>/]]<id> [<id>...]`, with slugs before the ID: a + // name may hold slashes too (`Latency/p95`, `Errors.v2/2026`) + / !{ return wholeIds; } sep p:slugPath x:more { + return { + ...pathTarget(p.target ?? ""), + ids: [{ text: p.text, id: p.id }, ...x], + }; + } + // Anything else in one token is a name, slashes and all + / sep n:token end { return { ids: [], name: n }; } + // `<target> <id> [<id>...]` or `<target> <name>`; a bare word as the + // target may instead be the name's first word, which the caller can + // check once it knows what the word names + / sep t:token n:afterTarget { + return { + target: t, + ...n, + ...(n.name !== undefined && !t.includes("/") && { bareTarget: true }), + }; + } + +// What follows a target: IDs, when the first token is or may be one, or a +// name, which takes every token left. +afterTarget + = end { return { ids: [] }; } + / &(sep i:token &{ return idLike(i); }) x:more { return { ids: x }; } + / x:(sep @token)+ { return { ids: [], name: x.join(" ") }; } + // The IDs after the first token. Each may repeat the target: // `<target>/<id>`. more @@ -95,6 +157,16 @@ idPath { return { text: text(), target: t, id: i }; } / i:idSegment argEnd { return { text: text(), id: i }; } +// `[<org>/[<project>/]]<id>` in one token, where the org and project are +// slugs +slugPath + = t:$(slug "/" slug) "/" i:idSegment argEnd + { return { text: text(), target: t, id: i }; } + / t:slug "/" i:idSegment argEnd { return { text: text(), target: t, id: i }; } + / i:idSegment argEnd { return { text: text(), id: i }; } + +slug = $[a-zA-Z0-9_-]+ + // An ID, and the slashes after it idSegment = i:segment "/"* &{ return isId(i); } { return i; } diff --git a/packages/cli/src/commands/dashboard/arguments.ts b/packages/cli/src/commands/dashboard/arguments.ts index f52e02a33..8fa6a302e 100644 --- a/packages/cli/src/commands/dashboard/arguments.ts +++ b/packages/cli/src/commands/dashboard/arguments.ts @@ -1,8 +1,8 @@ /** * Dashboard positionals: `[<org/project>] <dashboard-id-or-title>` and * `dashboard list`'s `[<org/project>] [<name-glob>]`, parsed by - * `grammar/dashboard-arguments.pegjs` before `func` runs and resolved to - * the org (and dashboard ID) on the first `.resolved` read. + * `lib/dashboard-arguments.ts` before `func` runs and resolved to the org + * (and dashboard ID) on the first `.resolved` read. */ import type { SentryContext } from "../../context.js"; @@ -18,13 +18,18 @@ import { parseDashboardArgs, parseDashboardListArgs, } from "../../lib/dashboard-arguments.js"; +import { ValidationError } from "../../lib/errors.js"; import { logger } from "../../lib/logger.js"; import { type BoundArguments, boundArguments, } from "../../lib/positional-parse.js"; import { applySentryUrlContext } from "../../lib/sentry-url-parser.js"; -import { resolveDashboardId, resolveOrgFromTarget } from "./resolve.js"; +import { + orgExists, + resolveDashboardId, + resolveOrgFromTarget, +} from "./resolve.js"; const log = logger.withTag("dashboard"); @@ -36,6 +41,12 @@ export type DashboardInput = { /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ readonly baseUrl?: string; readonly warnings: readonly string[]; + /** + * When the first argument is a bare word before a title (`My + * Dashboard`): the word, which names an org only if there is one, and the + * whole input as a title to fall back to + */ + readonly orgOrTitle?: { readonly org: string; readonly title: string }; }; /** A dashboard, resolved to its org and numeric ID. */ @@ -61,16 +72,15 @@ export function parseDashboardInput( args: readonly string[], usageHint: string, ): DashboardInput { - const { dashboardRef, targetArg, baseUrl, warnings } = parseDashboardArgs( - args, - usageHint, - ); + const { dashboardRef, targetArg, baseUrl, warnings, orgOrTitle } = + parseDashboardArgs(args, usageHint); const target = parseOrgProjectInput(targetArg); return { ref: dashboardRef, target: target.parsed, baseUrl: baseUrl ?? target.baseUrl, warnings, + ...(orgOrTitle && { orgOrTitle }), }; } @@ -84,14 +94,59 @@ export async function resolveDashboardInput( usageHint: string, ): Promise<ResolvedDashboard> { applyInputContext(input); + // `<word> <title...>`: the word names an org only if there is one. When + // there is, its title lookup is final. + if (input.orgOrTitle && !(await orgExists(input.orgOrTitle.org))) { + return await resolveWholeTitle(input.ref, input.orgOrTitle, cwd, usageHint); + } const orgSlug = await resolveOrgFromTarget(input.target, cwd, usageHint); const dashboardId = await resolveDashboardId(orgSlug, input.ref); return { orgSlug, dashboardId }; } +/** + * `<word> <title...>` where `<word>` names no org: look the whole input up + * as one title in the auto-detected org instead (`dashboard view My + * Dashboard`). + * + * @throws {ValidationError} When that title is not found either, naming + * both readings + */ +async function resolveWholeTitle( + ref: string, + orgOrTitle: NonNullable<DashboardInput["orgOrTitle"]>, + cwd: string, + usageHint: string, +): Promise<ResolvedDashboard> { + const { org, title } = orgOrTitle; + log.warn( + `No org '${org}'; treating "${title}" as the dashboard title. Use my-org/ to name an org.`, + ); + const orgSlug = await resolveOrgFromTarget( + { type: "auto-detect" }, + cwd, + usageHint, + ); + try { + const dashboardId = await resolveDashboardId(orgSlug, title); + return { orgSlug, dashboardId }; + } catch (error) { + if (!(error instanceof ValidationError)) { + throw error; + } + throw new ValidationError( + `No dashboard found: there is no org '${org}' to find '${ref}' in, ` + + `and no dashboard '${title}' in '${orgSlug}'.\n\n` + + `To name an org, end it with a slash: <org>/ ${ref}\n` + + `To name a title with spaces, quote it: '${title}'`, + "dashboard", + ); + } +} + /** * The `[<org/project>] <dashboard-id-or-title>` positional of a command that - * takes one dashboard (see `grammar/dashboard-arguments.pegjs`). + * takes one dashboard (see `parseDashboardArgs()`). * * @param options.usageHint - Usage shown in errors and warnings */ diff --git a/packages/cli/src/commands/dashboard/resolve.ts b/packages/cli/src/commands/dashboard/resolve.ts index 3299a768f..891a7931b 100644 --- a/packages/cli/src/commands/dashboard/resolve.ts +++ b/packages/cli/src/commands/dashboard/resolve.ts @@ -9,6 +9,7 @@ import { parseFunction } from "@sentry/toolkit-core/events-fields"; import { MAX_PAGINATION_PAGES } from "../../lib/api/infrastructure.js"; import { API_MAX_PER_PAGE, + getOrganization, listDashboardsPaginated, } from "../../lib/api-client.js"; import type { ParsedOrgProject } from "../../lib/arg-parsing.js"; @@ -17,6 +18,7 @@ import { ResolutionError, ValidationError, } from "../../lib/errors.js"; +import { getCachedOrganizations, getOrgRegion } from "../../lib/db/regions.js"; import { fuzzyMatch } from "../../lib/fuzzy.js"; import { logger } from "../../lib/logger.js"; import { resolveOrgOnlyTarget } from "../../lib/resolve-target.js"; @@ -63,6 +65,35 @@ export function resolveOrgFromTarget( return resolveOrgOnlyTarget(parsed, cwd, "dashboard", usageHint); } +/** + * Whether an org exists and is visible to the user: in the cached org list + * (or the region cache, which only orgs the API found are in), or else by + * fetching it. + * + * @returns `false` when the API cannot find the org or denies access to it + * @throws {ApiError} For any other failure, such as a server error + */ +export async function orgExists(orgSlug: string): Promise<boolean> { + if ( + getCachedOrganizations().some((org) => org.slug === orgSlug) || + getOrgRegion(orgSlug) !== undefined + ) { + return true; + } + try { + await getOrganization(orgSlug); + return true; + } catch (error) { + if ( + error instanceof ApiError && + (error.status === 404 || error.status === 403) + ) { + return false; + } + throw error; + } +} + /** * Resolve a dashboard reference (numeric ID or title) to a numeric ID string. * diff --git a/packages/cli/src/commands/event/view.ts b/packages/cli/src/commands/event/view.ts index c6b6eb6ce..410fbb616 100644 --- a/packages/cli/src/commands/event/view.ts +++ b/packages/cli/src/commands/event/view.ts @@ -18,7 +18,6 @@ import { resolveEventInOrg, } from "../../lib/api-client.js"; import { - looksLikeIssueShortId, type ParsedOrgProject, ProjectSpecificationType, parseOrgProjectInput, @@ -77,7 +76,6 @@ import { import { buildEventSearchUrl } from "../../lib/sentry-web-urls.js"; import { getSpanTreeLines } from "../../lib/span-tree.js"; import { setOrgProjectContext } from "../../lib/telemetry.js"; -import { isAllDigits } from "../../lib/utils.js"; import { EventViewOutputSchema, type SentryEvent } from "../../types/index.js"; type ViewFlags = { @@ -379,11 +377,17 @@ function issueForms(tokens: readonly string[]): IssueInput | null { if (second === undefined) { // `<issue-short-id>/<event-id>`: the event of an issue. Short IDs are - // uppercase, and project slugs never are. `<org>/<issue-short-id>` and - // `<org>/<numeric-issue-id>` take precedence, but not over an event ID. + // uppercase, and project slugs never are. `<org>/<issue-short-id>` + // comes first, then this, even for an all-digit event ID, and then + // `<org>/<numeric-issue-id>`. const afterShortId = - prefix && looksLikeIssueShortId(prefix) && !looksLikeIssueShortId(id); - if (afterShortId && (ID_KINDS.event.isId(id) || !isAllDigits(id))) { + prefix !== undefined && + issueForm(prefix) === "short-id" && + issueForm(id) !== "short-id"; + if ( + afterShortId && + (ID_KINDS.event.isId(id) || issueForm(id) !== "numeric") + ) { return { eventId: ID_KINDS.event.validate(id), targetArg: undefined, @@ -394,7 +398,7 @@ function issueForms(tokens: readonly string[]): IssueInput | null { } // `[<target>/]<issue-short-id> <event-id>`: the event of an issue. A // second argument with a slash is a target: the arguments are reversed. - if (looksLikeIssueShortId(id) && !second.includes("/")) { + if (issueForm(id) === "short-id" && !second.includes("/")) { return { // Issue shortcuts skip the event ID's recovery eventId: ID_KINDS.event.validate(second), @@ -413,13 +417,19 @@ function issueFromEventId( id: string, targetArg: string | undefined, ): IssueInput | null { - if (looksLikeIssueShortId(id)) { - return { eventId: LATEST_EVENT_SENTINEL, targetArg, issueShortId: id }; + switch (issueForm(id)) { + case "short-id": + return { eventId: LATEST_EVENT_SENTINEL, targetArg, issueShortId: id }; + case "numeric": + return { eventId: LATEST_EVENT_SENTINEL, targetArg, issueId: id }; + default: + return null; } - if (isAllDigits(id)) { - return { eventId: LATEST_EVENT_SENTINEL, targetArg, issueId: id }; - } - return null; +} + +/** The issue ID forms `event view` tells apart from event IDs */ +function issueForm(value: string): "short-id" | "numeric" | null { + return ID_KINDS.issue.distinctForm(value); } /** @@ -818,7 +828,7 @@ export async function fetchEventWithContext( ]; // Nudge the user when the event ID looks like an issue short ID - if (looksLikeIssueShortId(eventId)) { + if (issueForm(eventId) === "short-id") { suggestions.unshift( `This looks like an issue short ID. Try: sentry issue view ${eventId}`, ); diff --git a/packages/cli/src/commands/issue/utils.ts b/packages/cli/src/commands/issue/utils.ts index 42463b1ab..1ec3e5581 100644 --- a/packages/cli/src/commands/issue/utils.ts +++ b/packages/cli/src/commands/issue/utils.ts @@ -634,6 +634,63 @@ async function resolveSelector( return { org: orgSlug, issue }; } +/** + * How many issues a search fetches: enough to tell one match from several, + * and to list a few of them + */ +const SEARCH_CANDIDATES = 5; + +/** + * Resolve search text (an argument with spaces, which no identifier has) to + * the one issue it matches in the auto-detected org. The org's issue search + * takes free text, which matches issue titles and messages; it also looks up + * an event ID or short ID that is the whole query. + * + * @throws {ResolutionError} When no issue or several match, listing them and + * pointing at `sentry issue list --query` + */ +async function resolveIssueSearch( + query: string, + cwd: string, + commandContext: IssueCommandContext, +): Promise<StrictResolvedIssue> { + const { command, commandBase, commandHint, trailingArgs } = commandContext; + const resolved = await resolveOrg({ cwd }); + if (!resolved) { + throw new ContextError("Organization", commandHint); + } + const { org } = resolved; + const { data: issues, nextCursor } = await listIssuesPaginated(org, "", { + query, + perPage: SEARCH_CANDIDATES, + collapse: ISSUE_DETAIL_COLLAPSE, + }); + const [issue] = issues; + if (issue && issues.length === 1) { + return { org, issue }; + } + + const listCommand = `sentry issue list ${org}/ --query ${shellQuote(query)}`; + if (!issue) { + throw new ResolutionError( + `Search ${shellQuote(query)}`, + `matched no issues in '${org}'`, + listCommand, + ["Pass an issue ID or short ID instead, e.g. CLI-G or 123456789"], + ); + } + const count = nextCursor ? `${issues.length}+` : String(issues.length); + throw new ResolutionError( + `Search ${shellQuote(query)}`, + `matched ${count} issues in '${org}'`, + listCommand, + issues.map( + (candidate) => + `${appendCommandArguments(`${commandBase} ${command} ${org}/${candidate.shortId}`, trailingArgs)} (${candidate.title})`, + ), + ); +} + /** * Resolve a share URL to a full issue via two-step lookup: * 1. Call public share API to get numeric group ID @@ -857,6 +914,7 @@ function issueArgumentOf(issueArg: string | IssueArgument): IssueArgument { * - project-search: "cli-G" → search for project across orgs * - suffix-only: "G" (requires DSN context) * - numeric: "123456789" (direct fetch, no org) + * - search: "login timeout" (the one issue the text matches) * * @param options - Resolution options * @returns Object with org slug and full issue @@ -965,6 +1023,11 @@ export async function resolveIssue( result = await resolveShareIssue(parsed, cwd, commandHint); break; + case "search": + // Text, not an identifier: the issue it alone matches + result = await resolveIssueSearch(parsed.query, cwd, commandContext); + break; + default: { // Exhaustive check - this should never be reached const _exhaustive: never = parsed; diff --git a/packages/cli/src/commands/log/view.ts b/packages/cli/src/commands/log/view.ts index 7115d767f..f0a41e784 100644 --- a/packages/cli/src/commands/log/view.ts +++ b/packages/cli/src/commands/log/view.ts @@ -27,7 +27,7 @@ import { import { formatLogDetails } from "../../lib/formatters/index.js"; import { filterFields } from "../../lib/formatters/json.js"; import { CommandOutput } from "../../lib/formatters/output.js"; -import { ageInDaysFromUuidV7, validateHexId } from "../../lib/hex-id.js"; +import { ageInDaysFromUuidV7 } from "../../lib/hex-id.js"; import { handleRecoveryResult, recoverHexId, @@ -79,7 +79,7 @@ const USAGE_HINT = "sentry log view <org>/<project> <log-id> [<log-id>...]"; * IDs after any of those, each maybe repeating the target. An argument may * hold several lines, one log ID each. * - * Returns **raw** log IDs without running {@link validateHexId} — validation + * Returns **raw** log IDs without validating them — validation * is deferred to the main command so {@link recoverHexId} can use the * resolved org/project context for fuzzy prefix lookups. * @@ -118,7 +118,7 @@ export function parsePositionalArgs(args: readonly string[]): { /** * Validate and attempt to recover one log ID against the resolved target. * - * For each raw ID: run {@link validateHexId}; on {@link ValidationError}, + * For each raw ID: run `ID_KINDS.log.validate`; on {@link ValidationError}, * attempt {@link recoverHexId} with the resolved org/project. Successful * recovery returns a valid hex ID (and emits a `log.warn`). */ @@ -127,7 +127,7 @@ async function validateAndRecoverLogId( target: ResolvedLogTarget, ): Promise<string> { try { - return validateHexId(rawId, "log ID"); + return ID_KINDS.log.validate(rawId); } catch (err) { if (!(err instanceof ValidationError)) { throw err; diff --git a/packages/cli/src/commands/span/view.ts b/packages/cli/src/commands/span/view.ts index 30aff9040..4b2f29c84 100644 --- a/packages/cli/src/commands/span/view.ts +++ b/packages/cli/src/commands/span/view.ts @@ -191,7 +191,7 @@ async function validateAndRecoverSpanId( ctx: { org: string; project: string; traceId: string }, ): Promise<string> { try { - return validateSpanId(rawSpanId); + return ID_KINDS.span.validate(rawSpanId); } catch (err) { if (!(err instanceof ValidationError)) { throw err; diff --git a/packages/cli/src/lib/dashboard-arguments.ts b/packages/cli/src/lib/dashboard-arguments.ts index 6c6df4e17..0da5409a2 100644 --- a/packages/cli/src/lib/dashboard-arguments.ts +++ b/packages/cli/src/lib/dashboard-arguments.ts @@ -1,18 +1,25 @@ /** - * Dashboard positionals, parsed by `grammar/dashboard-arguments.pegjs`. + * Dashboard positionals. * - * Dashboards are org-scoped and named by a numeric ID or a title, which may - * hold spaces and slashes. That is a different syntax from the hex IDs of - * `id-arguments.ts`, so dashboards have their own grammar, but follow the - * same conventions: the result is a `targetArg` for `parseOrgProjectInput`, - * and parsing has no side effects. A pasted URL's server comes back as + * A command that takes one dashboard parses it like any ID argument, with + * `parseIdArguments()` and `ID_KINDS.dashboard` (`id-arguments.ts`): a + * numeric ID, or a title, which is the kind's name. Titles may hold spaces + * and slashes, and after a target, the rest of the arguments are one title + * (`my-org My Dashboard`). This module adds what is particular to + * dashboards: the messages for a missing dashboard and for a pasted URL. + * + * `dashboard list` takes a target and a title filter instead, with its own + * reading of one argument (`<org>/<project>/<filter>`, and a bare word is a + * filter), so {@link parseDashboardListArgs} parses it here. + * + * Parsing has no side effects: a pasted URL's server comes back as * `baseUrl` and anything to tell the user as `warnings`, for the caller to * apply. */ -import { parse } from "../generated/dashboard-arguments-grammar.js"; import { ValidationError } from "./errors.js"; -import { parseSentryUrl } from "./sentry-url-parser.js"; +import { ID_KINDS, parseIdArguments } from "./id-arguments.js"; +import { type ParsedSentryUrl, parseSentryUrl } from "./sentry-url-parser.js"; /** Dashboard arguments, parsed. */ export type DashboardArgs = { @@ -24,23 +31,33 @@ export type DashboardArgs = { readonly baseUrl?: string; /** Warnings to log, such as for an ID taken from a URL */ readonly warnings: readonly string[]; + /** + * When the first argument was a bare word before a title (`My + * Dashboard`), the word and the whole input as one title: the word names + * an org only if there is one + */ + readonly orgOrTitle?: { readonly org: string; readonly title: string }; }; /** * Parse the arguments of a command that takes one dashboard: - * `[<org/project>] <id-or-title>`, `<org>/[<project>/]<id>`, or a Sentry - * URL (see the grammar). After a target, the remaining arguments are one - * title: `my-org/ My Dashboard` names "My Dashboard". + * `[<org/project>] <id-or-title...>`, `<org>/[<project>/]<id>`, or a Sentry + * URL (see `grammar/id-arguments.pegjs`). After a target, the remaining + * arguments are one title: `my-org/ My Dashboard` names "My Dashboard"; + * after a numeric ID, they are extra. * - * @param usageHint - Usage shown when arguments after a URL are ignored + * @param usageHint - Usage shown when arguments are ignored * @throws {ValidationError} When no dashboard is given */ export function parseDashboardArgs( args: readonly string[], usageHint: string, ): DashboardArgs { - const syntax = parse(grammarInput(args), { list: false, sentryUrl }); - const { ref, url } = syntax; + // Not optional, but a missing dashboard has its own messages below + const { target, ids, name, nameWithTarget, baseUrl, warnings } = + parseIdArguments(args, ID_KINDS.dashboard, { usageHint, optional: true }); + const url = pastedUrl(args); + const ref = ids[0] ?? name; if (ref === undefined) { if (url?.org) { throw new ValidationError( @@ -55,20 +72,16 @@ export function parseDashboardArgs( "dashboard", ); } - const warnings: string[] = []; - if (url) { - warnings.push(urlWarning(url)); - } - if (syntax.extra.length > 0) { - warnings.push( - `Extra arguments ignored: ${syntax.extra.join(" ")}. Expected: ${usageHint}`, - ); - } return { dashboardRef: ref, - targetArg: syntax.target, - baseUrl: url?.baseUrl, - warnings, + // A URL with only an org is the target + targetArg: url && !url.dashboardId ? `${url.org}/` : target, + baseUrl: baseUrl ?? url?.baseUrl, + warnings: url ? [urlWarning(url), ...warnings] : warnings, + ...(nameWithTarget !== undefined && + target !== undefined && { + orgOrTitle: { org: target.slice(0, -1), title: nameWithTarget }, + }), }; } @@ -84,22 +97,38 @@ export type DashboardListArgs = { readonly warnings: readonly string[]; }; +/** `<org>/<project>/<filter>`: the filter is everything after the second slash */ +const TARGET_AND_FILTER_RE = /^([^/]*\/[^/]*)\/(.+)$/s; + /** * Parse `dashboard list` arguments: an optional target and title filter. - * One argument without a slash is a filter (dashboards are org-scoped, so a - * project search would only pick an org), and a filter may span several - * arguments (see the grammar). + * + * - (none): the org is auto-detected, no filter + * - `<filter>`: one argument without a slash (dashboards are org-scoped, so + * a project search would only pick an org) + * - `<org>/` or `<org>/<project>`: a target, no filter + * - `<org>/<project>/<filter>`: a target, then the filter + * - `<target> <filter...>`: the filter's words may be separate arguments, + * and a bare `<org>` means `<org>/` + * - `<org-url> [<filter...>]`: a Sentry URL with only an org * * @throws {ValidationError} For a dashboard URL, suggesting `dashboard view` */ export function parseDashboardListArgs( args: readonly (string | undefined)[], ): DashboardListArgs { - const syntax = parse( - grammarInput(args.filter((arg): arg is string => !!arg)), - { list: true, sentryUrl }, - ); - const { url } = syntax; + const given = args.filter((arg): arg is string => !!arg); + if (given.some((arg) => arg.includes("\0"))) { + throw new ValidationError( + "Arguments cannot contain NUL characters.", + "positional", + ); + } + const [first, ...rest] = given; + if (first === undefined) { + return { warnings: [] }; + } + const url = sentryUrlParts(first); if (url?.dashboardId) { const orgPrefix = url.org ? `${url.org}/ ` : ""; const orgSuffix = url.org ? ` ${url.org}/` : ""; @@ -112,32 +141,43 @@ export function parseDashboardListArgs( "dashboard", ); } - return { - targetArg: syntax.target, - titleFilter: syntax.filter, - baseUrl: url?.baseUrl, - warnings: url ? [urlWarning(url)] : [], - }; + if (url?.org) { + return { + targetArg: `${url.org}/`, + ...titleFilter(rest), + baseUrl: url.baseUrl, + warnings: [urlWarning(url)], + }; + } + if (rest.length > 0) { + return { + targetArg: first.includes("/") ? first : `${first}/`, + ...titleFilter(rest), + warnings: [], + }; + } + const split = TARGET_AND_FILTER_RE.exec(first); + if (split?.[1] !== undefined && split[2] !== undefined) { + return { targetArg: split[1], titleFilter: split[2], warnings: [] }; + } + return first.includes("/") + ? { targetArg: first, warnings: [] } + : { titleFilter: first, warnings: [] }; } -/** - * The grammar's input: a NUL before each argument. - * - * @throws {ValidationError} When an argument holds a NUL - */ -function grammarInput(args: readonly string[]): string { - if (args.some((arg) => arg.includes("\0"))) { - throw new ValidationError( - "Arguments cannot contain NUL characters.", - "positional", - ); - } - return args.map((arg) => `\0${arg}`).join(""); +function titleFilter(words: readonly string[]): { titleFilter?: string } { + return words.length > 0 ? { titleFilter: words.join(" ") } : {}; } -/** The parts of the Sentry URL `value` is, or `null` for anything else. */ -function sentryUrl(value: string) { - const url = parseSentryUrl(value); +/** The parts of a Sentry URL a dashboard argument cares about. */ +type DashboardUrl = { + readonly baseUrl: string; + readonly org?: string; + readonly dashboardId?: string; +}; + +function sentryUrlParts(value: string): DashboardUrl | null { + const url: ParsedSentryUrl | null = parseSentryUrl(value); return ( url && { baseUrl: url.baseUrl, @@ -147,10 +187,17 @@ function sentryUrl(value: string) { ); } -function urlWarning(url: { - readonly org?: string; - readonly dashboardId?: string; -}): string { +/** + * The Sentry URL the arguments start with, when it names a dashboard or an + * org: what the grammar took the dashboard or the target from. + */ +function pastedUrl(args: readonly string[]): DashboardUrl | null { + const first = args.find((arg) => arg.trim() !== "")?.trim(); + const url = first === undefined ? null : sentryUrlParts(first); + return url?.dashboardId || url?.org ? url : null; +} + +function urlWarning(url: DashboardUrl): string { if (url.dashboardId) { return ( `Extracted dashboard ID ${url.dashboardId} from URL` + diff --git a/packages/cli/src/lib/id-arguments.ts b/packages/cli/src/lib/id-arguments.ts index 3ab8b6d7c..b74278952 100644 --- a/packages/cli/src/lib/id-arguments.ts +++ b/packages/cli/src/lib/id-arguments.ts @@ -2,8 +2,8 @@ * Positionals that name IDs and the org/project to look them up in, parsed * by `grammar/id-arguments.pegjs`. * - * Every command that takes trace, span, replay, log, or event IDs shares one - * syntax, whatever the kind of ID: + * Every command that takes trace, span, replay, log, event, dashboard, or + * issue IDs shares one syntax, whatever the kind of ID: * * - `<id>`: the org/project is auto-detected * - `<org>/<id>` and `<org>/<project>/<id>`, like a URL (`<project>/<id>` @@ -16,6 +16,11 @@ * repeating the target or not. An argument may hold several lines, and IDs * after a space: `"<org>/<project> <id>"` is two arguments. * + * Some kinds also name items without an ID ({@link IdKind.names}): a + * dashboard by its title, an issue by search text. Then what is not an ID + * is a name, after the target or alone: `<org>/ My Dashboard`. Kinds + * without names say where to search instead ({@link IdKind.search}). + * * The command picks an {@link ID_KINDS} entry, which says what its IDs look * like, how to validate them, and which part of a URL holds one. The grammar * never guesses a kind from the shape of an ID: trace, replay, and log IDs @@ -27,6 +32,7 @@ */ import { parse } from "../generated/id-arguments-grammar.js"; +import { looksLikeIssueShortId } from "./arg-parsing.js"; import { ContextError, ValidationError } from "./errors.js"; import { isSpanId, @@ -35,7 +41,9 @@ import { validateSpanId, } from "./hex-id.js"; import { looksLikeHexIdFragment } from "./hex-id-recovery.js"; +import { isIssueIdentifier, parseIssueIdentifier } from "./issue-identifier.js"; import { type ParsedSentryUrl, parseSentryUrl } from "./sentry-url-parser.js"; +import { isAllDigits, shellQuote } from "./utils.js"; /** A kind of ID a command takes. */ export type IdKind = { @@ -58,15 +66,75 @@ export type IdKind = { readonly fromUrl: (url: ParsedSentryUrl) => string | undefined; /** A URL with this kind of ID, suggested when a pasted URL has none */ readonly urlExample?: string; + /** + * Items of this kind have names, such as a dashboard's title or issue + * search text: what is not, and may not be, an ID ({@link IdKind.mayBeId}) + * is a name, for the command to look up. A name may hold spaces and + * slashes, and after a target, takes the rest of the arguments, joined + * with spaces (an unquoted title). + */ + readonly names?: boolean; + /** + * An ID is a whole token with its own syntax for the org, project, and + * URL (issue short IDs): the grammar does not split a path off it, and + * {@link IdKind.validate} takes it apart. Only for kinds with names. + */ + readonly wholeIds?: boolean; + /** A bare word before the ID names an org, not a project */ + readonly orgScoped?: boolean; + /** + * For a kind without names: the list command that searches for items. + * {@link IdKind.validate} fails on text with a hint to search for it, + * instead of how the ID is malformed. + */ + readonly search?: { + /** The list command, e.g. `"sentry trace list"`, which takes `--query` */ + readonly command: string; + /** Whether `value`, not an ID, reads as text rather than a broken ID */ + readonly isText: (value: string) => boolean; + }; }; +/** Characters a hex ID may have, once lowercased and without dashes */ +const HEX_ID_CHARS_RE = /^[0-9a-f]*$/; + +/** + * Whether `value` reads as search text rather than a hex ID: it has letters + * past `f`, and is no flag (`--h`), placeholder (`<trace-id>`), or path, + * which the hex ID errors explain better. + */ +function isHexSearchText(value: string): boolean { + return ( + !(value.startsWith("-") || /[<>{}/]/.test(value)) && + !HEX_ID_CHARS_RE.test(value.toLowerCase().replace(/-/g, "")) + ); +} + +/** + * Whether `value` is issue search text: words with spaces, which no issue + * identifier has. Anything else (no space, a control character, or a URL) + * is a malformed identifier, rejected with its own message. + */ +function isIssueSearchText(value: string): boolean { + return ( + value.includes(" ") && + !CONTROL_CHAR_RE.test(value) && + parseSentryUrl(value) === null && + !URL_RE.test(value) + ); +} + +const CONTROL_CHAR_RE = /\p{Cc}/u; +const URL_RE = /^https?:\/\//i; + function hexKind( label: string, + listCommand: string | null, fromUrl: IdKind["fromUrl"], urlExample?: string, ): IdKind { const lower = label.charAt(0).toLowerCase() + label.slice(1); - return { + const kind: IdKind = { label, isId: (value) => tryNormalizeHexId(value) !== undefined, mayBeId: looksLikeHexIdFragment, @@ -74,26 +142,101 @@ function hexKind( fromUrl, urlExample, }; + return listCommand ? searchable(kind, listCommand) : kind; +} + +/** + * A kind without names whose IDs fail, when given text instead, with where + * to search for it rather than how the ID is malformed: `validate()` throws + * that, so a command that recovers malformed IDs first still gets it last. + */ +function searchable(kind: IdKind, listCommand: string): IdKind { + const search = { command: listCommand, isText: isHexSearchText }; + return { + ...kind, + search, + validate: (value) => { + if (!(kind.isId(value) || kind.mayBeId(value)) && search.isText(value)) { + const label = kind.label.charAt(0).toLowerCase() + kind.label.slice(1); + throw new ValidationError( + `Not a ${label}: "${value}". To search, try \`${search.command} --query ${shellQuote(value)}\`.\n\n` + + `If it names a project, pass the ${label} after it: <org>/<project> <${label}>`, + "positional", + ); + } + return kind.validate(value); + }, + }; } +/** + * Issue IDs: a numeric ID, short ID (`CLI-G`, `G`), selector (`@latest`), + * or issue URL, which carry their own org and project; words with spaces + * are a search. `parseIssueIdentifier()` takes an ID apart. + */ +const ISSUE_KIND: IdKind & { + /** + * The form of an issue ID that reads as one where another kind's ID may + * go too, as in `event view`: a short ID in its usual uppercase + * (`CLI-G`), which no project slug is, or a numeric ID; `null` for + * anything else + */ + readonly distinctForm: (value: string) => "short-id" | "numeric" | null; +} = { + label: "Issue ID", + isId: (value) => isIssueIdentifier(value), + mayBeId: (value) => !isIssueSearchText(value), + validate: (value) => { + parseIssueIdentifier(value); + return value; + }, + // Issue URLs are whole IDs: `parseIssueIdentifier()` reads them + fromUrl: () => undefined, + names: true, + wholeIds: true, + distinctForm: (value) => { + if (looksLikeIssueShortId(value)) { + return "short-id"; + } + return isAllDigits(value) ? "numeric" : null; + }, +}; + /** The kinds of ID commands take. Commands use these instead of their own. */ export const ID_KINDS = { - trace: hexKind("Trace ID", (url) => url.traceId), - event: hexKind("Event ID", (url) => url.eventId), + trace: hexKind("Trace ID", "sentry trace list", (url) => url.traceId), + // `event list` searches an issue's events, so there is no search to hint + event: hexKind("Event ID", null, (url) => url.eventId), // Replays are looked up in an org, so a replay URL needs one replay: hexKind( "Replay ID", + "sentry replay list", (url) => (url.org ? url.replayId : undefined), "https://sentry.io/organizations/{org}/explore/replays/{replayId}/", ), - log: hexKind("Log ID", () => undefined), - span: { - label: "Span ID", - isId: isSpanId, - mayBeId: looksLikeHexIdFragment, - validate: validateSpanId, - fromUrl: () => undefined, + log: hexKind("Log ID", "sentry log list", () => undefined), + span: searchable( + { + label: "Span ID", + isId: isSpanId, + mayBeId: looksLikeHexIdFragment, + validate: validateSpanId, + fromUrl: () => undefined, + }, + "sentry span list", + ), + // A numeric ID, or a title: `Latency/p95`, `Top 10`, or unquoted words + // after the org + dashboard: { + label: "Dashboard ID", + isId: isAllDigits, + mayBeId: () => false, + validate: (value) => value, + fromUrl: (url) => url.dashboardId, + names: true, + orgScoped: true, }, + issue: ISSUE_KIND, } as const satisfies Record<string, IdKind>; /** How a command takes its IDs. */ @@ -133,6 +276,17 @@ export type IdArguments = { readonly baseUrl?: string; /** Warnings to log, such as for ignored or reversed arguments */ readonly warnings: readonly string[]; + /** + * The name, for a kind with names, when the arguments name no ID; `ids` + * is then empty + */ + readonly name?: string; + /** + * When `target` was a bare word before `name` (`My Dashboard`), the whole + * input read as one name instead, for a command that falls back to it + * once it knows the word names nothing + */ + readonly nameWithTarget?: string; }; /** @@ -153,12 +307,15 @@ export function parseIdArguments( optional = false, needsProject = false, } = options; - const tokens = tokenize(args, kind.isId); + // A name may hold spaces, so IDs are not split off at them + const tokens = tokenize(args, kind.names ? null : kind.isId); const syntax = parse(tokens.map((token) => `\0${token}`).join(""), { optional, isId: kind.isId, mayBeId: kind.mayBeId, idUrl: (value) => idUrl(value, kind), + names: kind.names ?? false, + wholeIds: kind.wholeIds ?? false, }); const warnings: string[] = []; @@ -166,7 +323,20 @@ export function parseIdArguments( pathTargetArg(prefix, text, { usageHint, needsProject }); let target = syntax.path ? pathTarget(syntax.target ?? "", syntax.ids[0]?.text ?? "") - : syntax.target; + : separateTarget(syntax.target, kind); + + if (syntax.name !== undefined) { + return { + target, + ids: [], + baseUrl: syntax.baseUrl, + warnings, + name: syntax.name, + ...(syntax.bareTarget && { + nameWithTarget: `${syntax.target} ${syntax.name}`, + }), + }; + } const typed: { text: string; id: string }[] = []; for (const token of syntax.ids) { if (token.target) { @@ -246,6 +416,20 @@ export function parseIdArgument( return { ...rest, id }; } +/** + * A target that is its own argument: a bare word names an org for an + * org-scoped kind (`my-org` is `my-org/`), and a project otherwise. + */ +function separateTarget( + target: string | undefined, + kind: IdKind, +): string | undefined { + if (target === undefined || !kind.orgScoped || target.includes("/")) { + return target; + } + return `${target}/`; +} + /** Line breaks between IDs pasted into one argument */ const LINE_BREAK_RE = /\r?\n/; const WHITESPACE_RE = /\s+/; @@ -255,10 +439,13 @@ const WHITESPACE_RE = /\s+/; * IDs at the end of a line split off at spaces. A line's first word stays * whole, so a project's display name keeps its spaces. Empty lines are * dropped: an empty target means none. + * + * @param isId - Whether a word is an ID to split off; `null` for a kind + * with names, which may hold spaces, so lines stay whole and as typed */ function tokenize( args: readonly string[], - isId: (value: string) => boolean, + isId: ((value: string) => boolean) | null, ): string[] { const tokens: string[] = []; for (const arg of args) { @@ -269,6 +456,13 @@ function tokenize( ); } for (const line of arg.split(LINE_BREAK_RE)) { + if (!isId) { + const token = line.trim(); + if (token) { + tokens.push(token); + } + continue; + } const words = line.trim().split(WHITESPACE_RE); const ids: string[] = []; while (words.length > 1 && isId(words.at(-1) ?? "")) { diff --git a/packages/cli/src/lib/issue-arguments.ts b/packages/cli/src/lib/issue-arguments.ts index a9bc79bd7..e68a09c57 100644 --- a/packages/cli/src/lib/issue-arguments.ts +++ b/packages/cli/src/lib/issue-arguments.ts @@ -1,44 +1,24 @@ /** - * Issue arguments: the issue ID, short ID, selector, or URL that `issue - * view`, `issue explain`, and every other issue command takes. + * Issue arguments: the issue ID, short ID, selector, URL, or search text + * that `issue view`, `issue explain`, and every other issue command takes. * - * Which part of an argument is the org, the project, and the issue is - * decided by `grammar/issue-argument.pegjs`, which also lists the forms. - * Issue IDs are not hex IDs, so they have their own syntax rather than the - * shared one in `id-arguments.ts`. This module validates the parts the - * grammar found and normalizes them into a {@link ParsedIssueArg}. + * An argument is parsed like any ID argument, by `parseIdArguments()` with + * `ID_KINDS.issue` (`id-arguments.ts`): an issue identifier is one whole + * token, which `issue-identifier.ts` takes apart, and anything that cannot + * be one (text with spaces) is a name, which the command searches issues + * for. */ -import { - type IssueSyntax, - type IssueSyntaxError, - parse, -} from "../generated/issue-argument-grammar.js"; -import { - firstNonBlankLine, - type IssueSelector, - normalizeSlug, - parseSelector, - rejectIssueCommandTokenWithNumericSuffix, -} from "./arg-parsing.js"; +import { firstNonBlankLine } from "./arg-parsing.js"; import { ValidationError } from "./errors.js"; -import { validateResourceId } from "./input-validation.js"; +import { ID_KINDS, parseIdArguments } from "./id-arguments.js"; import { - applySentryUrlContext, - type ParsedSentryUrl, - parseSentryUrl, -} from "./sentry-url-parser.js"; -import { isAllDigits } from "./utils.js"; + type ParsedIssueArg, + parseIssueIdentifier, +} from "./issue-identifier.js"; +import { applySentryUrlContext } from "./sentry-url-parser.js"; -export type ParsedIssueArg = - | { type: "numeric"; id: string } - | { type: "explicit"; org: string; project: string; suffix: string } - | { type: "explicit-org-suffix"; org: string; suffix: string } - | { type: "explicit-org-numeric"; org: string; numericId: string } - | { type: "project-search"; projectSlug: string; suffix: string } - | { type: "suffix-only"; suffix: string } - | { type: "selector"; selector: IssueSelector; org?: string } - | { type: "share"; shareId: string; org?: string; baseUrl: string }; +export type { ParsedIssueArg } from "./issue-identifier.js"; /** An issue argument, parsed. */ export type IssueArgument = { @@ -59,17 +39,15 @@ export type IssueArgument = { * and fail with a cryptic "contains a newline" (CLI-1G1). An issue * identifier is one token, so joining the lines would make garbage. * - * @throws {ValidationError} When the argument is empty or malformed, or a - * Sentry URL has no issue in it + * Text that cannot be an identifier, because it has spaces, is a search: + * `{ type: "search", query }`. + * + * @throws {ValidationError} When the argument is empty or a malformed + * identifier, or a Sentry URL has no issue in it * * @example - * parseIssueArgument("123456789").parsed // { type: "numeric", id: "123456789" } - * parseIssueArgument("sentry/cli-G").parsed // { type: "explicit", org: "sentry", project: "cli", suffix: "G" } - * parseIssueArgument("cli-G").parsed // { type: "project-search", projectSlug: "cli", suffix: "G" } - * parseIssueArgument("sentry/G").parsed // { type: "explicit-org-suffix", org: "sentry", suffix: "G" } - * parseIssueArgument("G").parsed // { type: "suffix-only", suffix: "G" } - * parseIssueArgument("sentry/cli#CLI-G").parsed // { type: "explicit", org: "sentry", project: "cli", suffix: "G" } - * parseIssueArgument("#CLI-G").parsed // { type: "project-search", projectSlug: "cli", suffix: "G" } + * parseIssueArgument("cli-G").parsed // { type: "project-search", projectSlug: "cli", suffix: "G" } + * parseIssueArgument("login timeout").parsed // { type: "search", query: "login timeout" } */ export function parseIssueArgument(arg: string): IssueArgument { const input = firstNonBlankLine(arg); @@ -79,33 +57,17 @@ export function parseIssueArgument(arg: string): IssueArgument { "issue identifier", ); } - - const url = parseSentryUrl(input); - if (url) { - const parsed = issueArgFromUrl(url); - if (!parsed) { - // A Sentry URL without an issue (a trace or settings URL) - throw new ValidationError( - "This Sentry URL does not contain an issue ID. Use an issue URL like:\n" + - " https://sentry.io/organizations/{org}/issues/{id}/", - ); - } - return { text: arg, parsed, baseUrl: url.baseUrl }; - } - - const syntax = parse(input, { - isSelector: (value) => parseSelector(value) !== undefined, + // One token: an identifier (validated below, for its own messages) or a + // name + const { ids, name } = parseIdArguments([input], ID_KINDS.issue, { + usageHint: "sentry issue <command> <issue>", + validate: false, }); - if (syntax.form === "selector") { - return { text: arg, parsed: selectorArg(syntax) }; - } - if (syntax.form === "hash") { - return { text: arg, parsed: hashArg(input, syntax) }; + if (name !== undefined) { + return { text: arg, parsed: { type: "search", query: name } }; } - // Slashes separate the parts; `?`, `#`, `%`, whitespace, and control - // characters are never valid in an issue identifier - validateResourceId(input.replace(/\//g, ""), "issue identifier"); - return { text: arg, parsed: issueArgFromSyntax(input, syntax) }; + const { parsed, baseUrl } = parseIssueIdentifier(ids[0] ?? input); + return { text: arg, parsed, ...(baseUrl && { baseUrl }) }; } /** @@ -121,223 +83,3 @@ export function parseIssueArg(arg: string): ParsedIssueArg { } return parsed; } - -/** - * The issue in a Sentry URL: a share link, a numeric group ID, or a short ID - * (`CLI-G`) from the path; `null` when the URL names no issue. - */ -function issueArgFromUrl(url: ParsedSentryUrl): ParsedIssueArg | null { - if (url.shareId) { - return { - type: "share", - shareId: url.shareId, - org: url.org, - baseUrl: url.baseUrl, - }; - } - // Every non-share issue URL has an org - const { issueId, org } = url; - if (!(issueId && org)) { - return null; - } - if (isAllDigits(issueId)) { - return { type: "explicit-org-numeric", org, numericId: issueId }; - } - const dash = issueId.lastIndexOf("-"); - const project = issueId.slice(0, dash); - const suffix = issueId.slice(dash + 1).toUpperCase(); - if (dash > 0 && suffix) { - return { type: "explicit", org, project: project.toLowerCase(), suffix }; - } - return { type: "explicit-org-suffix", org, suffix: issueId.toUpperCase() }; -} - -function selectorArg( - syntax: Extract<IssueSyntax, { form: "selector" }>, -): ParsedIssueArg { - // The grammar only matches recognized selectors - const selector = parseSelector(syntax.selector) as IssueSelector; - if (syntax.org === undefined) { - return { type: "selector", selector }; - } - const org = normalizeSlug(syntax.org).slug; - validateResourceId(org, "organization slug"); - return { type: "selector", selector, org }; -} - -/** - * A GitHub-style `#` reference, which AI agents often pass (CLI-1G1): - * `org/project#SHORTID` (like `org/project/SHORTID`), `project#SHORTID`, or - * `#SHORTID`. - * - * It skips the check of the whole argument, which rejects `#`, so it checks - * both parts itself. `validateResourceId` permits `:`, so a `:` next to a - * `#` is rejected here rather than swallowed into the suffix. - * - * @throws {ValidationError} When the fragment is empty or has a second `#`, - * a `:`, or a forbidden character, or the prefix is invalid - */ -function hashArg( - input: string, - syntax: Extract<IssueSyntax, { form: "hash" }>, -): ParsedIssueArg { - const { prefix, fragment } = syntax; - if (fragment.includes("#") || fragment === "") { - throw new ValidationError( - `Invalid issue identifier: "${input}".\n` + - " Use a single '#' separating the project from the short ID, e.g. `org/project#PROJ-123`.\n" + - " Or use a slash: `org/project/PROJ-123`.", - "issue identifier", - ); - } - if (fragment.includes(":")) { - throw new ValidationError( - `Invalid issue identifier: "${input}". Do not mix '#' and ':' separators.\n` + - " Use `org/project#PROJ-123` or `project:PROJ-123`.", - "issue identifier", - ); - } - validateResourceId(fragment, "issue identifier"); - - if (prefix === "") { - return issueArgFromSyntax(fragment, reparse(fragment)); - } - if (prefix.includes("/")) { - validateResourceId(prefix.replace(/\//g, ""), "issue identifier"); - const path = `${prefix}/${fragment}`; - return issueArgFromSyntax(path, reparse(path)); - } - validateResourceId(prefix, "project slug"); - return projectIdentifier(prefix, fragment); -} - -/** Parse a part of a `#` reference, which has neither `#` nor a selector. */ -function reparse(input: string): IssueSyntax { - return parse(input, { isSelector: () => false }); -} - -/** The messages of malformed issue arguments. */ -const SYNTAX_ERRORS: Record<IssueSyntaxError, string> = { - "missing-id": "Missing issue ID after slash.", - "missing-project-or-id": "Missing project or issue ID segment.", - "slash-before-suffix": "Cannot use trailing slash before suffix.", - "missing-project": "Missing project before suffix.", - "missing-suffix": "Missing suffix after dash.", -}; - -/** - * Normalize the parts of an argument: Sentry slugs are lowercase, so a short - * ID typed in uppercase (`EASI-API-3Y4`) still finds its project (CLI-C8), - * and suffixes are uppercase. - * - * @throws {ValidationError} When the argument is malformed, or names an - * issue command and a number (`issue-1`) - */ -function issueArgFromSyntax( - input: string, - syntax: IssueSyntax, -): ParsedIssueArg { - switch (syntax.form) { - case "numeric": - return { type: "numeric", id: syntax.id }; - case "project-id": - return projectIdentifier(syntax.project, syntax.id); - case "short-id": { - const suffix = syntax.suffix.toUpperCase(); - rejectIssueCommandTokenWithNumericSuffix(input, syntax.project, suffix); - return { - type: "project-search", - projectSlug: syntax.project.toLowerCase(), - suffix, - }; - } - case "suffix": - return { type: "suffix-only", suffix: syntax.suffix.toUpperCase() }; - case "org-numeric": - return { - type: "explicit-org-numeric", - org: syntax.org, - numericId: syntax.id, - }; - case "org-project-id": - return orgProjectIdentifier(syntax.org, syntax.project, syntax.id); - case "org-short-id": { - const suffix = syntax.suffix.toUpperCase(); - rejectIssueCommandTokenWithNumericSuffix(input, syntax.project, suffix); - return { - type: "explicit", - org: syntax.org, - project: syntax.project.toLowerCase(), - suffix, - }; - } - case "org-suffix": - return { - type: "explicit-org-suffix", - org: syntax.org, - suffix: syntax.suffix.toUpperCase(), - }; - case "invalid": - throw new ValidationError( - `Invalid issue format: "${input}". ${SYNTAX_ERRORS[syntax.reason]}`, - "issue", - ); - default: - // A selector or `#` reference only comes from the whole argument - throw new ValidationError( - `Invalid issue identifier: "${input}".`, - "issue identifier", - ); - } -} - -/** - * `<project>:<id>` or `<project>#<id>`. A numeric ID needs no project; a - * full short ID (`PROJECT-SUFFIX`) gives its suffix; anything else is the - * suffix. - */ -function projectIdentifier(project: string, id: string): ParsedIssueArg { - if (isAllDigits(id)) { - return { type: "numeric", id }; - } - const projectSlug = project.toLowerCase(); - const suffix = id.slice(id.lastIndexOf("-") + 1).toUpperCase(); - return { - type: "project-search", - projectSlug, - suffix: suffix || id.toUpperCase(), - }; -} - -/** - * `<org>/<project>/<id>`: a full short ID whose prefix is the project - * (`sentry/cli/CLI-A1`) gives its suffix; anything else is the suffix - * (`org/project/SUBPROJ-G` → `SUBPROJ-G`). - */ -function orgProjectIdentifier( - org: string, - project: string, - id: string, -): ParsedIssueArg { - const projectSlug = project.toLowerCase(); - const dash = id.lastIndexOf("-"); - const prefix = id.slice(0, dash); - const suffix = id.slice(dash + 1).toUpperCase(); - if (dash !== -1 && prefix && suffix) { - return { - type: "explicit", - org, - project: projectSlug, - suffix: - prefix.toLowerCase() === projectSlug - ? suffix - : `${prefix}-${suffix}`.toUpperCase(), - }; - } - return { - type: "explicit", - org, - project: projectSlug, - suffix: id.toUpperCase(), - }; -} diff --git a/packages/cli/src/lib/issue-identifier.ts b/packages/cli/src/lib/issue-identifier.ts new file mode 100644 index 000000000..edb313218 --- /dev/null +++ b/packages/cli/src/lib/issue-identifier.ts @@ -0,0 +1,327 @@ +/** + * The syntax of an issue identifier: the issue ID, short ID, selector, or + * URL in one argument of `issue view`, `issue explain`, and every other + * issue command. + * + * Issue identifiers carry their own org and project (`<org>/<project>-<suffix>`, + * `<project>:<id>`, an issue URL), so `ID_KINDS.issue` (`id-arguments.ts`) + * takes them as whole tokens, and this module takes them apart: which part + * is the org, the project, and the issue is decided by + * `grammar/issue-argument.pegjs`, which also lists the forms. This module + * validates the parts the grammar found and normalizes them into a + * {@link ParsedIssueArg}. + */ + +import { + type IssueSyntax, + type IssueSyntaxError, + parse, +} from "../generated/issue-argument-grammar.js"; +import { + type IssueSelector, + normalizeSlug, + parseSelector, + rejectIssueCommandTokenWithNumericSuffix, +} from "./arg-parsing.js"; +import { ValidationError } from "./errors.js"; +import { validateResourceId } from "./input-validation.js"; +import { type ParsedSentryUrl, parseSentryUrl } from "./sentry-url-parser.js"; +import { isAllDigits } from "./utils.js"; + +export type ParsedIssueArg = + | { type: "numeric"; id: string } + | { type: "explicit"; org: string; project: string; suffix: string } + | { type: "explicit-org-suffix"; org: string; suffix: string } + | { type: "explicit-org-numeric"; org: string; numericId: string } + | { type: "project-search"; projectSlug: string; suffix: string } + | { type: "suffix-only"; suffix: string } + | { type: "selector"; selector: IssueSelector; org?: string } + | { type: "share"; shareId: string; org?: string; baseUrl: string } + /** Not an identifier: text to search issues for */ + | { type: "search"; query: string }; + +/** An issue identifier, taken apart. */ +export type IssueIdentifier = { + readonly parsed: Exclude<ParsedIssueArg, { type: "search" }>; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; +}; + +/** + * Take an issue identifier apart, without side effects: the server of a + * pasted Sentry URL is returned as `baseUrl` instead of applied. + * + * @param input - One line, trimmed and not empty + * @throws {ValidationError} When the identifier is malformed, or a Sentry + * URL has no issue in it + * + * @example + * parseIssueIdentifier("123456789").parsed // { type: "numeric", id: "123456789" } + * parseIssueIdentifier("sentry/cli-G").parsed // { type: "explicit", org: "sentry", project: "cli", suffix: "G" } + * parseIssueIdentifier("cli-G").parsed // { type: "project-search", projectSlug: "cli", suffix: "G" } + * parseIssueIdentifier("sentry/G").parsed // { type: "explicit-org-suffix", org: "sentry", suffix: "G" } + * parseIssueIdentifier("G").parsed // { type: "suffix-only", suffix: "G" } + * parseIssueIdentifier("sentry/cli#CLI-G").parsed // { type: "explicit", org: "sentry", project: "cli", suffix: "G" } + * parseIssueIdentifier("#CLI-G").parsed // { type: "project-search", projectSlug: "cli", suffix: "G" } + */ +export function parseIssueIdentifier(input: string): IssueIdentifier { + const url = parseSentryUrl(input); + if (url) { + const parsed = issueArgFromUrl(url); + if (!parsed) { + // A Sentry URL without an issue (a trace or settings URL) + throw new ValidationError( + "This Sentry URL does not contain an issue ID. Use an issue URL like:\n" + + " https://sentry.io/organizations/{org}/issues/{id}/", + ); + } + return { parsed, baseUrl: url.baseUrl }; + } + + const syntax = parse(input, { + isSelector: (value) => parseSelector(value) !== undefined, + }); + if (syntax.form === "selector") { + return { parsed: selectorArg(syntax) }; + } + if (syntax.form === "hash") { + return { parsed: hashArg(input, syntax) }; + } + // Slashes separate the parts; `?`, `#`, `%`, whitespace, and control + // characters are never valid in an issue identifier + validateResourceId(input.replace(/\//g, ""), "issue identifier"); + return { parsed: issueArgFromSyntax(input, syntax) }; +} + +/** Whether {@link parseIssueIdentifier} takes `input` apart. */ +export function isIssueIdentifier(input: string): boolean { + try { + parseIssueIdentifier(input); + return true; + } catch (error) { + if (error instanceof ValidationError) { + return false; + } + throw error; + } +} + +/** What an identifier names: anything but a search. */ +type IdentifierArg = IssueIdentifier["parsed"]; + +/** + * The issue in a Sentry URL: a share link, a numeric group ID, or a short ID + * (`CLI-G`) from the path; `null` when the URL names no issue. + */ +function issueArgFromUrl(url: ParsedSentryUrl): IdentifierArg | null { + if (url.shareId) { + return { + type: "share", + shareId: url.shareId, + org: url.org, + baseUrl: url.baseUrl, + }; + } + // Every non-share issue URL has an org + const { issueId, org } = url; + if (!(issueId && org)) { + return null; + } + if (isAllDigits(issueId)) { + return { type: "explicit-org-numeric", org, numericId: issueId }; + } + const dash = issueId.lastIndexOf("-"); + const project = issueId.slice(0, dash); + const suffix = issueId.slice(dash + 1).toUpperCase(); + if (dash > 0 && suffix) { + return { type: "explicit", org, project: project.toLowerCase(), suffix }; + } + return { type: "explicit-org-suffix", org, suffix: issueId.toUpperCase() }; +} + +function selectorArg( + syntax: Extract<IssueSyntax, { form: "selector" }>, +): IdentifierArg { + // The grammar only matches recognized selectors + const selector = parseSelector(syntax.selector) as IssueSelector; + if (syntax.org === undefined) { + return { type: "selector", selector }; + } + const org = normalizeSlug(syntax.org).slug; + validateResourceId(org, "organization slug"); + return { type: "selector", selector, org }; +} + +/** + * A GitHub-style `#` reference, which AI agents often pass (CLI-1G1): + * `org/project#SHORTID` (like `org/project/SHORTID`), `project#SHORTID`, or + * `#SHORTID`. + * + * It skips the check of the whole argument, which rejects `#`, so it checks + * both parts itself. `validateResourceId` permits `:`, so a `:` next to a + * `#` is rejected here rather than swallowed into the suffix. + * + * @throws {ValidationError} When the fragment is empty or has a second `#`, + * a `:`, or a forbidden character, or the prefix is invalid + */ +function hashArg( + input: string, + syntax: Extract<IssueSyntax, { form: "hash" }>, +): IdentifierArg { + const { prefix, fragment } = syntax; + if (fragment.includes("#") || fragment === "") { + throw new ValidationError( + `Invalid issue identifier: "${input}".\n` + + " Use a single '#' separating the project from the short ID, e.g. `org/project#PROJ-123`.\n" + + " Or use a slash: `org/project/PROJ-123`.", + "issue identifier", + ); + } + if (fragment.includes(":")) { + throw new ValidationError( + `Invalid issue identifier: "${input}". Do not mix '#' and ':' separators.\n` + + " Use `org/project#PROJ-123` or `project:PROJ-123`.", + "issue identifier", + ); + } + validateResourceId(fragment, "issue identifier"); + + if (prefix === "") { + return issueArgFromSyntax(fragment, reparse(fragment)); + } + if (prefix.includes("/")) { + validateResourceId(prefix.replace(/\//g, ""), "issue identifier"); + const path = `${prefix}/${fragment}`; + return issueArgFromSyntax(path, reparse(path)); + } + validateResourceId(prefix, "project slug"); + return projectIdentifier(prefix, fragment); +} + +/** Parse a part of a `#` reference, which has neither `#` nor a selector. */ +function reparse(input: string): IssueSyntax { + return parse(input, { isSelector: () => false }); +} + +/** The messages of malformed issue arguments. */ +const SYNTAX_ERRORS: Record<IssueSyntaxError, string> = { + "missing-id": "Missing issue ID after slash.", + "missing-project-or-id": "Missing project or issue ID segment.", + "slash-before-suffix": "Cannot use trailing slash before suffix.", + "missing-project": "Missing project before suffix.", + "missing-suffix": "Missing suffix after dash.", +}; + +/** + * Normalize the parts of an argument: Sentry slugs are lowercase, so a short + * ID typed in uppercase (`EASI-API-3Y4`) still finds its project (CLI-C8), + * and suffixes are uppercase. + * + * @throws {ValidationError} When the argument is malformed, or names an + * issue command and a number (`issue-1`) + */ +function issueArgFromSyntax(input: string, syntax: IssueSyntax): IdentifierArg { + switch (syntax.form) { + case "numeric": + return { type: "numeric", id: syntax.id }; + case "project-id": + return projectIdentifier(syntax.project, syntax.id); + case "short-id": { + const suffix = syntax.suffix.toUpperCase(); + rejectIssueCommandTokenWithNumericSuffix(input, syntax.project, suffix); + return { + type: "project-search", + projectSlug: syntax.project.toLowerCase(), + suffix, + }; + } + case "suffix": + return { type: "suffix-only", suffix: syntax.suffix.toUpperCase() }; + case "org-numeric": + return { + type: "explicit-org-numeric", + org: syntax.org, + numericId: syntax.id, + }; + case "org-project-id": + return orgProjectIdentifier(syntax.org, syntax.project, syntax.id); + case "org-short-id": { + const suffix = syntax.suffix.toUpperCase(); + rejectIssueCommandTokenWithNumericSuffix(input, syntax.project, suffix); + return { + type: "explicit", + org: syntax.org, + project: syntax.project.toLowerCase(), + suffix, + }; + } + case "org-suffix": + return { + type: "explicit-org-suffix", + org: syntax.org, + suffix: syntax.suffix.toUpperCase(), + }; + case "invalid": + throw new ValidationError( + `Invalid issue format: "${input}". ${SYNTAX_ERRORS[syntax.reason]}`, + "issue", + ); + default: + // A selector or `#` reference only comes from the whole argument + throw new ValidationError( + `Invalid issue identifier: "${input}".`, + "issue identifier", + ); + } +} + +/** + * `<project>:<id>` or `<project>#<id>`. A numeric ID needs no project; a + * full short ID (`PROJECT-SUFFIX`) gives its suffix; anything else is the + * suffix. + */ +function projectIdentifier(project: string, id: string): IdentifierArg { + if (isAllDigits(id)) { + return { type: "numeric", id }; + } + const projectSlug = project.toLowerCase(); + const suffix = id.slice(id.lastIndexOf("-") + 1).toUpperCase(); + return { + type: "project-search", + projectSlug, + suffix: suffix || id.toUpperCase(), + }; +} + +/** + * `<org>/<project>/<id>`: a full short ID whose prefix is the project + * (`sentry/cli/CLI-A1`) gives its suffix; anything else is the suffix + * (`org/project/SUBPROJ-G` → `SUBPROJ-G`). + */ +function orgProjectIdentifier( + org: string, + project: string, + id: string, +): IdentifierArg { + const projectSlug = project.toLowerCase(); + const dash = id.lastIndexOf("-"); + const prefix = id.slice(0, dash); + const suffix = id.slice(dash + 1).toUpperCase(); + if (dash !== -1 && prefix && suffix) { + return { + type: "explicit", + org, + project: projectSlug, + suffix: + prefix.toLowerCase() === projectSlug + ? suffix + : `${prefix}-${suffix}`.toUpperCase(), + }; + } + return { + type: "explicit", + org, + project: projectSlug, + suffix: id.toUpperCase(), + }; +} diff --git a/packages/cli/test/commands/dashboard/arguments.test.ts b/packages/cli/test/commands/dashboard/arguments.test.ts new file mode 100644 index 000000000..2a77c0d22 --- /dev/null +++ b/packages/cli/test/commands/dashboard/arguments.test.ts @@ -0,0 +1,179 @@ +/** + * Dashboard argument resolution tests (src/commands/dashboard/arguments.ts): + * a bare first word before a title names an org only if there is one. + * Parsing alone is tested in test/lib/dashboard-arguments.test.ts. + */ + +import { beforeEach, describe, expect, test, vi } from "vitest"; + +const { fakeLog, mocks } = vi.hoisted(() => { + const log: { + warn: ReturnType<typeof vi.fn>; + withTag: () => typeof log; + } = { warn: vi.fn(), withTag: () => log }; + return { + fakeLog: log, + mocks: { + getOrganization: vi.fn(), + listDashboardsPaginated: vi.fn(), + getCachedOrganizations: vi.fn(), + getOrgRegion: vi.fn(), + resolveOrgOnlyTarget: vi.fn(), + }, + }; +}); + +vi.mock("../../../src/lib/logger.js", async (importOriginal) => ({ + ...(await importOriginal<typeof import("../../../src/lib/logger.js")>()), + logger: fakeLog, +})); + +vi.mock("../../../src/lib/api-client.js", async (importOriginal) => ({ + ...(await importOriginal<typeof import("../../../src/lib/api-client.js")>()), + getOrganization: mocks.getOrganization, + listDashboardsPaginated: mocks.listDashboardsPaginated, +})); + +vi.mock("../../../src/lib/db/regions.js", async (importOriginal) => ({ + ...(await importOriginal<typeof import("../../../src/lib/db/regions.js")>()), + getCachedOrganizations: mocks.getCachedOrganizations, + getOrgRegion: mocks.getOrgRegion, +})); + +vi.mock("../../../src/lib/resolve-target.js", async (importOriginal) => ({ + ...(await importOriginal< + typeof import("../../../src/lib/resolve-target.js") + >()), + resolveOrgOnlyTarget: mocks.resolveOrgOnlyTarget, +})); + +import { + parseDashboardInput, + resolveDashboardInput, +} from "../../../src/commands/dashboard/arguments.js"; +import { ApiError, ValidationError } from "../../../src/lib/errors.js"; + +const USAGE_HINT = "sentry dashboard view <org>/ <id-or-title>"; +const DETECTED_ORG = "detected-org"; + +/** Dashboards per org, for `listDashboardsPaginated` */ +let dashboards: Record<string, { id: string; title: string }[]>; + +beforeEach(() => { + vi.clearAllMocks(); + dashboards = {}; + mocks.getCachedOrganizations.mockReturnValue([]); + mocks.getOrgRegion.mockReturnValue(undefined); + mocks.getOrganization.mockRejectedValue(new ApiError("Not Found", 404)); + mocks.listDashboardsPaginated.mockImplementation(async (org: string) => ({ + data: dashboards[org] ?? [], + })); + mocks.resolveOrgOnlyTarget.mockImplementation( + async (parsed: { type: string; org?: string }) => + parsed.type === "auto-detect" ? DETECTED_ORG : parsed.org, + ); +}); + +function resolve(args: string[]) { + return resolveDashboardInput( + parseDashboardInput(args, USAGE_HINT), + "/tmp", + USAGE_HINT, + ); +} + +/** Warnings about falling back to the whole input as the title */ +function fallbackWarnings(): unknown[] { + return fakeLog.warn.mock.calls + .map(([message]) => message) + .filter((message) => String(message).startsWith("No org ")); +} + +describe("a bare word before a title", () => { + test("names an org when there is one, from the cache: no fallback", async () => { + mocks.getCachedOrganizations.mockReturnValue([{ slug: "my-org" }]); + dashboards["my-org"] = [{ id: "7", title: "Errors" }]; + + expect(await resolve(["my-org", "Errors"])).toEqual({ + orgSlug: "my-org", + dashboardId: "7", + }); + expect(mocks.getOrganization).not.toHaveBeenCalled(); + expect(fallbackWarnings()).toEqual([]); + }); + + test("names an org the API finds on a cache miss: its lookup is final", async () => { + mocks.getOrganization.mockResolvedValue({ slug: "my-org" }); + dashboards[DETECTED_ORG] = [{ id: "9", title: "my-org Errors" }]; + + await expect(resolve(["my-org", "Errors"])).rejects.toThrow( + "No dashboard with title 'Errors' found in 'my-org'", + ); + expect(mocks.getOrganization).toHaveBeenCalledWith("my-org"); + expect(fallbackWarnings()).toEqual([]); + }); + + test("without that org, is the title's first word, with a warning", async () => { + dashboards[DETECTED_ORG] = [{ id: "42", title: "My Dashboard" }]; + + expect(await resolve(["My", "Dashboard"])).toEqual({ + orgSlug: DETECTED_ORG, + dashboardId: "42", + }); + expect(fallbackWarnings()).toEqual([ + `No org 'My'; treating "My Dashboard" as the dashboard title. Use my-org/ to name an org.`, + ]); + }); + + test("a 403 is no org either", async () => { + mocks.getOrganization.mockRejectedValue(new ApiError("Forbidden", 403)); + dashboards[DETECTED_ORG] = [{ id: "42", title: "My Dashboard" }]; + + expect((await resolve(["My", "Dashboard"])).dashboardId).toBe("42"); + }); + + test("when neither reading finds a dashboard, one error names both", async () => { + const error = await resolve(["My", "Dashboard"]).catch( + (caught: unknown) => caught, + ); + expect(error).toBeInstanceOf(ValidationError); + const { message } = error as ValidationError; + expect(message).toContain("no org 'My' to find 'Dashboard' in"); + expect(message).toContain( + `no dashboard 'My Dashboard' in '${DETECTED_ORG}'`, + ); + expect(message).toContain("<org>/ Dashboard"); + expect(message).toContain("quote it: 'My Dashboard'"); + }); + + test("a server error checking the org propagates, without a fallback", async () => { + const serverError = new ApiError("Internal Server Error", 500); + mocks.getOrganization.mockRejectedValue(serverError); + dashboards[DETECTED_ORG] = [{ id: "42", title: "My Dashboard" }]; + + await expect(resolve(["My", "Dashboard"])).rejects.toBe(serverError); + expect(mocks.listDashboardsPaginated).not.toHaveBeenCalled(); + expect(fallbackWarnings()).toEqual([]); + }); +}); + +describe("never falls back", () => { + test("for an explicit org: `my-org/ My Dashboard`", async () => { + dashboards[DETECTED_ORG] = [{ id: "42", title: "my-org/ My Dashboard" }]; + + await expect(resolve(["my-org/", "My", "Dashboard"])).rejects.toThrow( + "No dashboard with title 'My Dashboard' found in 'my-org'", + ); + expect(mocks.getOrganization).not.toHaveBeenCalled(); + expect(fallbackWarnings()).toEqual([]); + }); + + test("for a numeric ID", async () => { + expect(await resolve(["my-org", "123"])).toEqual({ + orgSlug: "my-org", + dashboardId: "123", + }); + expect(mocks.getOrganization).not.toHaveBeenCalled(); + expect(mocks.listDashboardsPaginated).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/cli/test/commands/event/view.test.ts b/packages/cli/test/commands/event/view.test.ts index 7d3af40a7..5a186f29c 100644 --- a/packages/cli/test/commands/event/view.test.ts +++ b/packages/cli/test/commands/event/view.test.ts @@ -256,6 +256,18 @@ describe("parseEventViewInput", () => { expect(result.issueId).toBeUndefined(); }); + test("<org>/<short-id> and <org>/<numeric-id> keep their order around <short-id>/<event-id>", () => { + expect(parseEventViewInput(["my-org/CAM-82X"])).toMatchObject({ + issueShortId: "CAM-82X", + targetArg: "my-org/", + }); + // A short number is no event ID: the issue ID, after a target + expect(parseEventViewInput(["CAM-82X/123"])).toMatchObject({ + issueId: "123", + targetArg: "CAM-82X/", + }); + }); + test("rejects an incomplete event ID after <short-id>/", () => { // Not project `CAM-82X` with a partial event ID to recover expect(() => parseEventViewInput(["CAM-82X/95fd7f5a"])).toThrow( diff --git a/packages/cli/test/commands/issue/utils.test.ts b/packages/cli/test/commands/issue/utils.test.ts index f1cf1fc02..2d359915f 100644 --- a/packages/cli/test/commands/issue/utils.test.ts +++ b/packages/cli/test/commands/issue/utils.test.ts @@ -2437,3 +2437,68 @@ describe("resolveIssue with share URLs", () => { ]); }); }); + +describe("resolveIssue with search text", () => { + useEnvSandbox(["SENTRY_ORG", "SENTRY_PROJECT", "SENTRY_DSN"]); + + /** Issues the search returns, and the queries it got */ + function mockSearch( + issues: readonly { id: string; shortId: string; title: string }[], + ): URLSearchParams[] { + const queries: URLSearchParams[] = []; + globalThis.fetch = mockFetch(async (input, init) => { + const url = new URL(new Request(input, init).url); + if (url.pathname === "/api/0/organizations/my-org/issues/") { + queries.push(url.searchParams); + return Response.json( + issues.map((issue) => ({ ...issue, status: "unresolved" })), + ); + } + return Response.json({ detail: "Not found" }, { status: 404 }); + }); + return queries; + } + + function resolve(issueArg: string) { + process.env.SENTRY_ORG = "my-org"; + return resolveIssue({ issueArg, cwd: getConfigDir(), command: "view" }); + } + + test("resolves the one issue the text matches, in the detected org", async () => { + const queries = mockSearch([ + { id: "1", shortId: "CLI-G", title: "Login timeout" }, + ]); + const result = await resolve("login timeout"); + expect(result.org).toBe("my-org"); + expect(result.issue.shortId).toBe("CLI-G"); + expect(queries[0]?.get("query")).toBe("login timeout"); + }); + + test("no match is a ResolutionError pointing at issue list", async () => { + mockSearch([]); + const error = await resolve("login timeout").catch((e: unknown) => e); + expect(error).toBeInstanceOf(ResolutionError); + expect((error as ResolutionError).message).toContain( + "matched no issues in 'my-org'", + ); + expect((error as ResolutionError).hint).toBe( + "sentry issue list my-org/ --query 'login timeout'", + ); + }); + + test("several matches are a ResolutionError listing them", async () => { + mockSearch([ + { id: "1", shortId: "CLI-G", title: "Login timeout" }, + { id: "2", shortId: "CLI-H", title: "Login timeout on retry" }, + ]); + const error = await resolve("login timeout").catch((e: unknown) => e); + expect(error).toBeInstanceOf(ResolutionError); + expect((error as ResolutionError).message).toContain( + "matched 2 issues in 'my-org'", + ); + expect((error as ResolutionError).suggestions).toEqual([ + "sentry issue view my-org/CLI-G (Login timeout)", + "sentry issue view my-org/CLI-H (Login timeout on retry)", + ]); + }); +}); diff --git a/packages/cli/test/lib/arg-parsing.property.test.ts b/packages/cli/test/lib/arg-parsing.property.test.ts index 2fddf0116..947e39105 100644 --- a/packages/cli/test/lib/arg-parsing.property.test.ts +++ b/packages/cli/test/lib/arg-parsing.property.test.ts @@ -218,7 +218,8 @@ describe("parseIssueArg properties", () => { describe("parseIssueArg: GitHub-style # separator properties (CLI-1G1)", () => { /** Forbidden characters (besides the structural #) that must always be rejected. */ - const forbiddenCharArb = constantFrom("?", "%", " ", "\t"); + // A space makes words, which are a search rather than an identifier + const forbiddenCharArb = constantFrom("?", "%", "\t"); test("org/project#FULL-SHORTID is equivalent to org/project/FULL-SHORTID", async () => { fcAssert( diff --git a/packages/cli/test/lib/arg-parsing.test.ts b/packages/cli/test/lib/arg-parsing.test.ts index 4cf88c2ab..d465fb589 100644 --- a/packages/cli/test/lib/arg-parsing.test.ts +++ b/packages/cli/test/lib/arg-parsing.test.ts @@ -1650,11 +1650,22 @@ describe("parseIssueArg: injection hardening", () => { test("rejects forbidden characters inside a # fragment", () => { // A bare "#" is now a valid GitHub-style separator (CLI-1G1), but the // fragment after it is still validated against injection characters. - expect(() => parseIssueArg("cli#an chor")).toThrow(ValidationError); expect(() => parseIssueArg("cli#an%20chor")).toThrow(ValidationError); expect(() => parseIssueArg("cli#G?extra")).toThrow(ValidationError); }); + test("words with spaces are a search, not an identifier", () => { + // A search is sent as a query parameter, never as a path segment + expect(parseIssueArg("cli#an chor")).toEqual({ + type: "search", + query: "cli#an chor", + }); + expect(parseIssueArg("12345 6789")).toEqual({ + type: "search", + query: "12345 6789", + }); + }); + test("rejects pre-encoded space in issue arg", () => { expect(() => parseIssueArg("CLI-G%20extra")).toThrow(ValidationError); }); @@ -1665,10 +1676,6 @@ describe("parseIssueArg: injection hardening", () => { expect(() => parseIssueArg("CLI\tG")).toThrow(ValidationError); }); - test("rejects space in numeric ID", () => { - expect(() => parseIssueArg("12345 6789")).toThrow(ValidationError); - }); - test("rejects query string in org/issue format", () => { expect(() => parseIssueArg("sentry/CLI-G?extra")).toThrow(ValidationError); }); @@ -1794,7 +1801,6 @@ describe("parseIssueArg: GitHub-style # separator (CLI-1G1)", () => { test("forbidden characters in fragment throw", () => { expect(() => parseIssueArg("cli#?bad")).toThrow(ValidationError); - expect(() => parseIssueArg("cli#an chor")).toThrow(ValidationError); expect(() => parseIssueArg("cli#an%20chor")).toThrow(ValidationError); }); diff --git a/packages/cli/test/lib/dashboard-arguments.test.ts b/packages/cli/test/lib/dashboard-arguments.test.ts index a6154a6a9..71f1d103b 100644 --- a/packages/cli/test/lib/dashboard-arguments.test.ts +++ b/packages/cli/test/lib/dashboard-arguments.test.ts @@ -1,6 +1,6 @@ /** - * Dashboard argument parsing tests (src/lib/dashboard-arguments.ts and - * grammar/dashboard-arguments.pegjs). + * Dashboard argument parsing tests (src/lib/dashboard-arguments.ts, on + * grammar/id-arguments.pegjs). */ import { describe, expect, test } from "vitest"; @@ -294,11 +294,6 @@ describe("parseDashboardArgs warnings and edge cases", () => { }); test("after a target, an unquoted title's words form one title", () => { - expect(parseRef(["my-org", "My", "Dashboard"])).toEqual({ - dashboardRef: "My Dashboard", - targetArg: "my-org/", - warnings: [], - }); expect(parseRef(["my-org/", "Backend", "Performance"])).toEqual({ dashboardRef: "Backend Performance", targetArg: "my-org/", @@ -311,6 +306,51 @@ describe("parseDashboardArgs warnings and edge cases", () => { }); }); + test("a title of three or more words", () => { + expect(parseRef(["my-org/", "API", "Latency", "by", "Region"])).toEqual({ + dashboardRef: "API Latency by Region", + targetArg: "my-org/", + warnings: [], + }); + }); + + test("a slash title followed by more words", () => { + expect(parseRef(["my-org/", "Latency/p95", "by", "Region"])).toEqual({ + dashboardRef: "Latency/p95 by Region", + targetArg: "my-org/", + warnings: [], + }); + }); + + test("a numeric ID after a target keeps the rest as extra", () => { + expect(parseRef(["my-org", "123", "extra"])).toEqual({ + dashboardRef: "123", + targetArg: "my-org/", + warnings: [`Extra arguments ignored: extra. Expected: ${USAGE_HINT}`], + }); + }); + + test("a bare first word may be the org or the title's first word", () => { + expect(parseRef(["my-org", "My", "Dashboard"])).toEqual({ + dashboardRef: "My Dashboard", + targetArg: "my-org/", + warnings: [], + orgOrTitle: { org: "my-org", title: "my-org My Dashboard" }, + }); + expect(parseRef(["My", "Dashboard"])).toEqual({ + dashboardRef: "Dashboard", + targetArg: "My/", + warnings: [], + orgOrTitle: { org: "My", title: "My Dashboard" }, + }); + }); + + test("an explicit org or a numeric ID is never a title's first word", () => { + expect(parseRef(["my-org/", "My", "Dashboard"]).orgOrTitle).toBeUndefined(); + expect(parseRef(["my-org", "123"]).orgOrTitle).toBeUndefined(); + expect(parseRef(["my-org/web", "Errors"]).orgOrTitle).toBeUndefined(); + }); + test("after an org URL, an unquoted title's words form one title", () => { expect( parseRef(["https://acme.sentry.io/", "Backend", "Performance"]), @@ -333,15 +373,26 @@ describe("parseDashboardArgs warnings and edge cases", () => { ); }); - test("an empty argument is still a dashboard reference", () => { - expect(parseRef([""])).toEqual({ dashboardRef: "", warnings: [] }); + test("empty arguments are skipped", () => { + expect(() => parseRef([""])).toThrow("Dashboard ID or title is required"); expect(parseRef(["my-org", ""])).toEqual({ - dashboardRef: "", - targetArg: "my-org/", + dashboardRef: "my-org", warnings: [], }); }); + test("a numeric ID first keeps the rest as extra, or reads them reversed", () => { + expect(parseRef(["42", "extra"])).toEqual({ + dashboardRef: "42", + warnings: [`Extra arguments ignored: extra. Expected: ${USAGE_HINT}`], + }); + expect(parseRef(["42", "my-org/web"])).toEqual({ + dashboardRef: "42", + targetArg: "my-org/web", + warnings: ["Arguments appear reversed. Interpreting as: my-org/web 42"], + }); + }); + test("an org URL without a dashboard suggests the org form", () => { expect(() => parseRef(["https://acme.sentry.io/"])).toThrow( "Try: sentry dashboard <command> acme/ <id-or-title>", diff --git a/packages/cli/test/lib/id-arguments.test.ts b/packages/cli/test/lib/id-arguments.test.ts index 745cb14ed..6dcb9cf45 100644 --- a/packages/cli/test/lib/id-arguments.test.ts +++ b/packages/cli/test/lib/id-arguments.test.ts @@ -22,8 +22,11 @@ const SPAN = "aaaa1111bbbb2222"; const SPAN2 = "cccc3333dddd4444"; const HINT = "sentry thing view [<org>/<project>/]<id>"; -/** An ID of each kind, and a second one */ -const EXAMPLES: Record<keyof typeof ID_KINDS, readonly [string, string]> = { +/** Kinds without names: a token that is no ID is a target */ +type UnnamedKind = "trace" | "event" | "replay" | "log" | "span"; + +/** An ID of each kind without names, and a second one */ +const EXAMPLES: Record<UnnamedKind, readonly [string, string]> = { trace: [HEX, HEX2], event: [HEX, HEX2], replay: [HEX, HEX2], @@ -31,9 +34,8 @@ const EXAMPLES: Record<keyof typeof ID_KINDS, readonly [string, string]> = { span: [SPAN, SPAN2], }; -const KINDS = Object.entries(ID_KINDS).map( - ([name, kind]) => - [name, kind, ...EXAMPLES[name as keyof typeof ID_KINDS]] as const, +const KINDS = (Object.keys(EXAMPLES) as UnnamedKind[]).map( + (name) => [name, ID_KINDS[name] as IdKind, ...EXAMPLES[name]] as const, ); function parse( @@ -144,18 +146,46 @@ describe.each(KINDS)("%s IDs", (_name, kind, id, other) => { ); test.each([ - [["not-hex"], undefined], - [["acme/web/not-hex"], "acme/web"], - [["acme/web", "not-hex"], "acme/web"], - [["not-hex", "acme/web"], "acme/web"], + [["abc123"], undefined], + [["acme/web/abc123"], "acme/web"], + [["acme/web", "abc123"], "acme/web"], + [["abc123", "acme/web"], "acme/web"], ])("%j → invalid ID, or raw ID and target %j", (args, target) => { expect(() => parse(kind, args)).toThrow(ValidationError); expect(parse(kind, args, { validate: false })).toMatchObject({ target, - ids: ["not-hex"], + ids: ["abc123"], }); }); + test.each([ + [["login"]], + [["login error"]], + [["acme/web", "checkout"]], + [["acme/web/checkout"]], + ])("%j → not an ID, with a hint to search, if there is one", (args) => { + if (!kind.search) { + // `event list` lists an issue's events: there is no search to hint + expect(() => parse(kind, args)).toThrow(/^Invalid /); + return; + } + const label = kind.label.charAt(0).toLowerCase() + kind.label.slice(1); + const text = args.at(-1)?.split("/").at(-1) ?? ""; + expect(() => parse(kind, args)).toThrow( + `Not a ${label}: "${text}". To search, try \`${kind.search?.command} --query '${text}'\`.`, + ); + // Unvalidated, it stays raw for the command to recover, then validate + expect(parse(kind, args, { validate: false }).ids).toEqual([text]); + expect(() => kind.validate(text)).toThrow(/^Not a .* To search, try/); + }); + + test.each([["--h"], ["<trace-id>"], ["{traceId}"], ["abc-123"]])( + "%j is no search text: the ID's own error explains it", + (arg) => { + expect(() => parse(kind, [arg])).toThrow(/^Invalid /); + }, + ); + test.each([ [`acme/${id.slice(0, 12)}`, id.slice(0, 12)], [ @@ -303,3 +333,107 @@ describe("tokens", () => { ).toEqual({ target: HEX, ids: [SPAN], warnings: [] }); }); }); + +describe("kinds with names", () => { + const dashboard = ID_KINDS.dashboard; + const issue = ID_KINDS.issue; + + test.each([ + [["42"], { ids: ["42"] }], + [["acme/42"], { target: "acme/", ids: ["42"] }], + [["acme/web/42"], { target: "acme/web", ids: ["42"] }], + [["My Dashboard"], { ids: [], name: "My Dashboard" }], + // A name is never split at spaces, even before a number + [["Top 10"], { ids: [], name: "Top 10" }], + // Slashes stay in a name unless slugs come before an ID + [["Latency/p95"], { ids: [], name: "Latency/p95" }], + [["Errors.v2/123"], { ids: [], name: "Errors.v2/123" }], + [["acme/"], { ids: [], name: "acme/" }], + ])("%j → %j", (args, expected) => { + expect(parse(dashboard, args)).toEqual({ warnings: [], ...expected }); + }); + + test("a name takes every argument after the target", () => { + expect( + parse(dashboard, ["acme/", "API", "Latency", "by", "Region"]), + ).toEqual({ + target: "acme/", + ids: [], + name: "API Latency by Region", + warnings: [], + }); + expect( + parse(dashboard, ["acme/web", "Latency/p95", "by", "Region"]), + ).toEqual({ + target: "acme/web", + ids: [], + name: "Latency/p95 by Region", + warnings: [], + }); + }); + + test("an ID after the target keeps the rest as extra", () => { + expect(parse(dashboard, ["acme/", "123", "extra"])).toEqual({ + target: "acme/", + ids: ["123"], + warnings: [`Extra arguments ignored: extra. Expected: ${HINT}`], + }); + }); + + test("a bare word before a name may be the name's first word", () => { + // An org-scoped kind reads the bare word as an org + expect(parse(dashboard, ["My", "Dashboard"])).toEqual({ + target: "My/", + ids: [], + name: "Dashboard", + nameWithTarget: "My Dashboard", + warnings: [], + }); + expect(parse(dashboard, ["acme/", "Dashboard"]).nameWithTarget).toBe( + undefined, + ); + }); + + test("a URL with no ID is a target before a name", () => { + expect( + parse(dashboard, ["https://acme.sentry.io/", "Backend", "Performance"]), + ).toMatchObject({ + target: "https://acme.sentry.io/", + ids: [], + name: "Backend Performance", + }); + }); + + test("a URL with an ID names it", () => { + expect(parse(dashboard, ["https://acme.sentry.io/dashboard/7/"])).toEqual({ + target: "acme/", + ids: ["7"], + baseUrl: "https://acme.sentry.io", + warnings: [], + }); + }); + + test("an ID with its own syntax is a whole token", () => { + for (const id of ["acme/cli-G", "acme/cli/G", "cli#CLI-G", "@latest"]) { + expect(parse(issue, [id])).toEqual({ ids: [id], warnings: [] }); + } + }); + + test("a name is what cannot be an ID", () => { + expect(parse(issue, ["login timeout"])).toEqual({ + ids: [], + name: "login timeout", + warnings: [], + }); + // No space: a malformed identifier, which the kind rejects + expect(parse(issue, ["acme/"], { validate: false })).toEqual({ + ids: ["acme/"], + warnings: [], + }); + expect(() => parse(issue, ["acme/"])).toThrow(ValidationError); + }); + + test("a kind with names has no search hint", () => { + expect("search" in dashboard || "search" in issue).toBe(false); + }); +});