diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 22e887f26..ce2d8dab1 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -316,9 +316,9 @@ what leaves a target for `func` to parse: the completion-only parsers `resolve*FromArg` helpers. Commands not migrated yet are on `UNBOUND_TARGET_PARSING_ALLOWED` in `lint-rules/cli-oxlint-plugin.js`, each with its follow-up; the list is a ratchet, so a file that stops using them -fails until its entry is removed. `AD_HOC_ID_PARSING_ALLOWED` does the same -for ID positionals, including the completion-only `parseIssueTarget` and -`parseDashboardTarget`. +fails until its entry is removed. `sentry-cli/no-ad-hoc-id-arguments` has no +allow-list: no command may parse IDs by hand, and the completion-only +`parseIssueTarget` and `parseDashboardTarget` are rejected too. **Release positionals:** `[/]` uses `releaseParameter(USAGE_HINT, brief)` from `src/commands/release/parse.ts` diff --git a/packages/cli/lint-rules/cli-oxlint-plugin.js b/packages/cli/lint-rules/cli-oxlint-plugin.js index 759ddea56..dbaf3cb88 100644 --- a/packages/cli/lint-rules/cli-oxlint-plugin.js +++ b/packages/cli/lint-rules/cli-oxlint-plugin.js @@ -70,12 +70,15 @@ const BOUND_TARGET_TYPES = new Set([ "BoundArguments", "ConversationArguments", "DashboardArguments", + "DashboardCreateArguments", "DashboardListArguments", "EventViewArguments", + "InitArguments", "IntegrationArguments", "IssuesArguments", "LogListArguments", "LogViewArguments", + "ProjectCreateArguments", "ReplayArguments", "SpanListArguments", "SpanViewArguments", @@ -179,13 +182,6 @@ const AD_HOC_ID_PARSING = new Set([ "parseDashboardTarget", ]); -/** - * Commands still allowed to, each with its follow-up. A ratchet: an entry - * whose file no longer uses them is reported, so the list only shrinks. - * Empty: every command parses its IDs with `parseIdArguments()`. - */ -const AD_HOC_ID_PARSING_ALLOWED = new Map(); - /** * Helpers that leave a target positional for `func` to parse: * `parseOrgTarget` and `parseOrgProjectTarget` only tag it for completion, @@ -203,7 +199,10 @@ const UNBOUND_TARGET_PARSING = new Set([ "resolveProjectBoundFromArg", ]); -/** Commands still allowed to, each with its follow-up; a ratchet too. */ +/** + * Commands still allowed to, each with its follow-up. A ratchet: an entry + * whose file no longer uses them is reported, so the list only shrinks. + */ const UNBOUND_TARGET_PARSING_ALLOWED = new Map([ // Follow-ups (#1564): // `[/[/]]`, resolving to every matching project @@ -217,10 +216,6 @@ const UNBOUND_TARGET_PARSING_ALLOWED = new Map([ ["/src/commands/alert/metrics/edit.ts", "#1564: alert rule target"], ["/src/commands/alert/metrics/list.ts", "#1564: alert rule target"], ["/src/commands/alert/metrics/view.ts", "#1564: alert rule target"], - // A target followed by other positionals - ["/src/commands/dashboard/create.ts", "#1564: target and title"], - ["/src/commands/init.ts", "#1564: target and directory"], - ["/src/commands/project/create.ts", "#1564: name and platform"], ]); /** The follow-up of `file` in an allow-list, if it is on it. */ @@ -231,11 +226,28 @@ function allowedFollowUp(allowed, file) { return undefined; } +/** Whether `node` uses one of `names`; imports and `typeof` do not. */ +function usesName(node, names) { + return ( + names.has(node.name) && + node.parent?.type !== "ImportSpecifier" && + node.parent?.type !== "TSTypeQuery" + ); +} + +/** Visitors reporting each use of one of `names` in a command file. */ +function bannedNames(file, names, report, message) { + if (!file.includes("/src/commands/")) return {}; + return { + Identifier(node) { + if (usesName(node, names)) report(node, message); + }, + }; +} + /** - * Visitors reporting each reference to one of `names` in a command file, - * except in files on `allowed`; those are reported instead when they no - * longer reference any, so the allow-list only shrinks. Imports and - * `typeof` references are not uses. + * Like {@link bannedNames}, except in files on `allowed`; those are reported + * instead when they no longer use any, so the allow-list only shrinks. */ function ratchetedNames(file, names, allowed, report, message) { if (!file.includes("/src/commands/")) return {}; @@ -243,12 +255,7 @@ function ratchetedNames(file, names, allowed, report, message) { let used = false; return { Identifier(node) { - if ( - !names.has(node.name) || - node.parent?.type === "ImportSpecifier" || - node.parent?.type === "TSTypeQuery" - ) - return; + if (!usesName(node, names)) return; used = true; if (followUp === undefined) report(node, message); }, @@ -454,12 +461,7 @@ const rules = { if (!file.includes("/src/") || file.endsWith("/lib/id-arguments.ts")) return {}; return { - ...ratchetedNames( - file, - AD_HOC_ID_PARSING, - AD_HOC_ID_PARSING_ALLOWED, - report, - ), + ...bannedNames(file, AD_HOC_ID_PARSING, report), CallExpression(node) { if ( ID_ARGUMENT_PARSERS.has(node.callee.name) && diff --git a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/init.md b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/init.md index 291551707..25dd3abce 100644 --- a/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/init.md +++ b/packages/cli/plugins/sentry-cli/skills/sentry-cli/references/init.md @@ -19,8 +19,7 @@ Initialize Sentry in your project (experimental) | Argument | Description | |----------|-------------| -| `[]` | <org>/<project>, <org>/, <project>, or a directory path | -| `[]` | Project directory (default: current directory) | +| `[] []` | Optional target (<org>/<project>, <org>/, or <project>) and directory (default: current directory) | **Flags:** - `-y, --yes - Accept non-interactive defaults (requires --features outside a TTY)` diff --git a/packages/cli/src/commands/dashboard/create.ts b/packages/cli/src/commands/dashboard/create.ts index 1f669dd4f..4b657ccfd 100644 --- a/packages/cli/src/commands/dashboard/create.ts +++ b/packages/cli/src/commands/dashboard/create.ts @@ -8,13 +8,17 @@ import type { SentryContext } from "../../context.js"; import { createDashboard, getProject } from "../../lib/api-client.js"; import { type ParsedOrgProject, - parseOrgProjectArg, + parseOrgProjectInput, } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; +import { ORG_PROJECT_TARGET } from "../../lib/completion-target.js"; import { ContextError, ValidationError } from "../../lib/errors.js"; import { formatDashboardCreated } from "../../lib/formatters/human.js"; import { CommandOutput } from "../../lib/formatters/output.js"; +import { + type BoundArguments, + boundArguments, +} from "../../lib/positional-parse.js"; import { fetchProjectId, resolveAllTargets, @@ -22,6 +26,7 @@ import { resolveProjectBoundSlug, toNumericId, } from "../../lib/resolve-target.js"; +import { applySentryUrlContext } from "../../lib/sentry-url-parser.js"; import { buildDashboardUrl } from "../../lib/sentry-web-urls.js"; import { setOrgProjectContext } from "../../lib/telemetry.js"; import type { DashboardDetail } from "../../types/dashboard.js"; @@ -34,25 +39,34 @@ type CreateFlags = { type CreateResult = DashboardDetail & { url: string }; +/** `dashboard create`'s positionals, parsed: the title and the target. */ +type CreateInput = { + readonly title: string; + readonly target: ParsedOrgProject; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; +}; + /** - * Parse array positional args for `dashboard create`. + * Parse `dashboard create`'s positionals without side effects. * * Handles: * - `` — title only (auto-detect org/project) - * - `<target> <title>` — explicit target + title + * - `<target> <title...>` — explicit target + title; the title's words may + * be separate arguments + * + * @throws {ValidationError} When there is no title or the target is invalid */ -function parsePositionalArgs(args: string[]): { - title: string; - targetArg: string | undefined; -} { - if (args.length === 0) { +function parseCreateInput(args: readonly string[]): CreateInput { + const [first, ...rest] = args; + if (first === undefined) { throw new ValidationError("Dashboard title is required.", "title"); } - if (args.length === 1) { - return { title: args[0] as string, targetArg: undefined }; + if (rest.length === 0) { + return { title: first, target: { type: "auto-detect" } }; } - // Two args: first is target, second is title - return { title: args[1] as string, targetArg: args[0] as string }; + const { parsed, baseUrl } = parseOrgProjectInput(first); + return { title: rest.join(" "), target: parsed, baseUrl }; } /** Result of resolving org + project IDs from the parsed target */ @@ -61,6 +75,12 @@ type ResolvedDashboardTarget = { projectIds: number[]; }; +/** `dashboard create`'s `[<org/project>] <title>` arguments. */ +export type DashboardCreateArguments = BoundArguments< + CreateInput, + ResolvedDashboardTarget +>; + /** Enrich targets that lack a projectId by calling the project API */ async function enrichTargetProjectIds( targets: { org: string; project: string; projectId?: number }[], @@ -163,22 +183,27 @@ export const createCommand = buildCommand({ human: formatDashboardCreated, }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "org/project/title", - brief: "[<org/project>] <title>", - parse: parseOrgProjectTarget, + positional: boundArguments({ + placeholder: "org/project/title", + brief: "[<org/project>] <title>", + kind: ORG_PROJECT_TARGET, + parse: parseCreateInput, + resolve: async (input: CreateInput, context: SentryContext) => { + if (input.baseUrl) { + applySentryUrlContext(input.baseUrl); + } + return await resolveDashboardTarget(input.target, context.cwd); }, - }, + }), flags: {}, }, - async *func(this: SentryContext, _flags: CreateFlags, ...args: string[]) { - const { cwd } = this; - - const { title, targetArg } = parsePositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - const { orgSlug, projectIds } = await resolveDashboardTarget(parsed, cwd); + async *func( + this: SentryContext, + _flags: CreateFlags, + args: DashboardCreateArguments, + ) { + const { title } = args.parsed; + const { orgSlug, projectIds } = await args.resolved; const dashboard = await withDashboardErrors( { orgSlug, operation: "create" }, diff --git a/packages/cli/src/commands/init.ts b/packages/cli/src/commands/init.ts index 0b78f105c..109511bdd 100644 --- a/packages/cli/src/commands/init.ts +++ b/packages/cli/src/commands/init.ts @@ -20,7 +20,11 @@ import path from "node:path"; import { setTag } from "@sentry/node"; import type { SentryContext } from "../context.js"; -import { looksLikePath, parseOrgProjectArg } from "../lib/arg-parsing.js"; +import { + looksLikePath, + type ParsedOrgProject, + parseOrgProjectInput, +} from "../lib/arg-parsing.js"; import { buildCommand } from "../lib/command.js"; import { refreshToken } from "../lib/db/auth.js"; import { ContextError, ValidationError } from "../lib/errors.js"; @@ -34,7 +38,12 @@ import { YES_ALIASES, YES_FLAG, } from "../lib/mutate-command.js"; +import { + type BoundArguments, + boundArguments, +} from "../lib/positional-parse.js"; import { classifyProjectSearchTarget } from "../lib/resolve-target.js"; +import { applySentryUrlContext } from "../lib/sentry-url-parser.js"; const log = logger.withTag("init"); @@ -98,14 +107,19 @@ type InitFlags = { * Classify and separate two optional positional args into a target and a directory. * * Uses {@link looksLikePath} to distinguish filesystem paths from org/project targets. - * Detects swapped arguments and emits a warning when auto-correcting. + * Detects swapped arguments and returns a warning when auto-correcting. * - * @returns Resolved target string (or undefined) and directory string (or undefined) + * @returns Target string (or undefined), directory string (or undefined), + * and the warning for swapped arguments, if any */ function classifyArgs( first?: string, second?: string, -): { target: string | undefined; directory: string | undefined } { +): { + target: string | undefined; + directory: string | undefined; + warning?: string; +} { // No args — auto-detect everything if (!first) { return { target: undefined, directory: undefined }; @@ -152,8 +166,56 @@ function classifyArgs( } // (PATH, TARGET) — swapped, auto-correct with warning - log.warn(`Arguments appear reversed. Interpreting as: ${second} ${first}`); - return { target: second, directory: first }; + return { + target: second, + directory: first, + warning: `Arguments appear reversed. Interpreting as: ${second} ${first}`, + }; +} + +/** `init`'s positionals, parsed: the target and the directory. */ +type InitInput = { + readonly target: ParsedOrgProject; + /** The directory as typed, relative to the cwd; `undefined` for the cwd */ + readonly directory?: string; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + /** Set when the arguments were swapped; `func` logs it */ + readonly warning?: string; +}; + +/** `init`'s target, resolved to explicit org and project values. */ +type ResolvedInitTarget = { + readonly org: string | undefined; + readonly project: string | undefined; +}; + +/** `init`'s `[<target>] [<directory>]` arguments. */ +export type InitArguments = BoundArguments<InitInput, ResolvedInitTarget>; + +/** + * Parse `init`'s positionals without side effects: tell the target from + * the directory ({@link classifyArgs}), then parse and validate the target. + * + * @throws {ValidationError} For more than two arguments, two directories, + * two targets, or an invalid org or project slug + */ +function parseInitInput(args: readonly string[]): InitInput { + if (args.length > 2) { + throw new ValidationError( + `Too many arguments: ${args.join(" ")}. Expected: sentry init [<target>] [<directory>]`, + ); + } + const { target, directory, warning } = classifyArgs(args[0], args[1]); + const { parsed, baseUrl } = parseOrgProjectInput(target); + // Validate user-provided slugs before they reach API calls + if (parsed.type === "explicit") { + validateResourceId(parsed.org, "organization slug"); + validateResourceId(parsed.project, "project name"); + } else if (parsed.type === "org-all") { + validateResourceId(parsed.org, "organization slug"); + } + return { target: parsed, directory, baseUrl, warning }; } function parseFeatures( @@ -218,27 +280,24 @@ function validateNonInteractiveInit( } /** - * Resolve the parsed org/project target into explicit org and project values. + * Resolve the parsed org/project target into explicit org and project values, + * after applying a pasted URL's server. * * For `project-search` (bare slug), searches for an existing project first. * If not found, treats the slug as a **new project name** to create — * org will be resolved later by init preflight before the workflow starts. * If the slug matches an org name, treats it as org-only (like `slug/`). */ -async function resolveTarget(targetArg: string | undefined): Promise<{ - org: string | undefined; - project: string | undefined; -}> { - const parsed = parseOrgProjectArg(targetArg); +async function resolveTarget(input: InitInput): Promise<ResolvedInitTarget> { + if (input.baseUrl) { + applySentryUrlContext(input.baseUrl); + } + const parsed = input.target; switch (parsed.type) { case "explicit": - // Validate user-provided slugs before they reach API calls - validateResourceId(parsed.org, "organization slug"); - validateResourceId(parsed.project, "project name"); return { org: parsed.org, project: parsed.project }; case "org-all": - validateResourceId(parsed.org, "organization slug"); return { org: parsed.org, project: undefined }; case "project-search": { const resolution = await classifyProjectSearchTarget(parsed, { @@ -288,11 +347,12 @@ async function resolveTarget(targetArg: string | undefined): Promise<{ export const initCommand = buildCommand< InitFlags, - [string?, string?], + [InitArguments], SentryContext >({ docs: { brief: "Initialize Sentry in your project (experimental)", + customUsage: ["[<target>] [<directory>]"], fullDescription: "EXPERIMENTAL: This command may modify your source files.\n\n" + "Runs the Sentry setup wizard to detect your project's framework, " + @@ -338,23 +398,13 @@ export const initCommand = buildCommand< ], }, parameters: { - positional: { - kind: "tuple", - parameters: [ - { - placeholder: "target", - brief: "<org>/<project>, <org>/, <project>, or a directory path", - parse: String, - optional: true, - }, - { - placeholder: "directory", - brief: "Project directory (default: current directory)", - parse: String, - optional: true, - }, - ], - }, + positional: boundArguments({ + placeholder: "target/directory", + brief: + "Optional target (<org>/<project>, <org>/, or <project>) and directory (default: current directory)", + parse: parseInitInput, + resolve: resolveTarget, + }), flags: { yes: { ...YES_FLAG, @@ -397,17 +447,13 @@ export const initCommand = buildCommand< }, }, // oxlint-disable-next-line require-yield -- init renders through WizardUI instead of command output - async *func( - this: SentryContext, - flags: InitFlags, - first?: string, - second?: string, - ) { - // 1. Classify positionals into target vs directory - const { target: targetArg, directory: dirArg } = classifyArgs( - first, - second, - ); + async *func(this: SentryContext, flags: InitFlags, args: InitArguments) { + // 1. Positionals were classified into target vs directory before func + // ran; report a swap now, as before + const { directory: dirArg, warning } = args.parsed; + if (warning) { + log.warn(warning); + } // 2. Resolve directory const targetDir = dirArg ? path.resolve(this.cwd, dirArg) : this.cwd; @@ -426,11 +472,10 @@ export const initCommand = buildCommand< } // 4. Resolve target → org + project - // Validation of user-provided slugs happens inside resolveTarget. + // User-provided slugs were validated when the target was parsed. // For bare slugs, if no existing project is found, the slug becomes // the name for a new project (org resolved later by the wizard). - const { org: explicitOrg, project: explicitProject } = - await resolveTarget(targetArg); + const { org: explicitOrg, project: explicitProject } = await args.resolved; // Mastra requires an explicit bearer, unlike regular Sentry API requests // that refresh OAuth lazily. Resolve it before opening the wizard so an diff --git a/packages/cli/src/commands/project/create.ts b/packages/cli/src/commands/project/create.ts index 36bde4a9b..d317ded13 100644 --- a/packages/cli/src/commands/project/create.ts +++ b/packages/cli/src/commands/project/create.ts @@ -26,7 +26,7 @@ import { listTeams, MEMBER_PROJECT_CREATION_DISABLED_DETAIL, } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; +import { parseOrgProjectInput } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; import { ApiError, @@ -45,17 +45,22 @@ import { CommandOutput } from "../../lib/formatters/output.js"; import { logger } from "../../lib/logger.js"; import { DRY_RUN_ALIASES, DRY_RUN_FLAG } from "../../lib/mutate-command.js"; import { renderPlatformGrid } from "../../lib/platform-grid.js"; +import { + type BoundArguments, + boundArguments, +} from "../../lib/positional-parse.js"; import { COMMON_PLATFORMS, isValidPlatform, suggestPlatform, } from "../../lib/platforms.js"; -import { resolveOrg } from "../../lib/resolve-target.js"; +import { type ResolvedOrg, resolveOrg } from "../../lib/resolve-target.js"; import { buildOrgNotFoundError, type ResolvedConcreteTeam, resolveOrCreateTeam, } from "../../lib/resolve-team.js"; +import { applySentryUrlContext } from "../../lib/sentry-url-parser.js"; import { slugify } from "../../lib/utils.js"; const log = logger.withTag("project.create"); @@ -77,17 +82,18 @@ type CreateFlags = { * Sentry's SDK guide URLs use dots (e.g., `sentry.io/for/javascript.nextjs`) * but platform identifiers use hyphens (`javascript-nextjs`). Users often * copy the dot-notation directly. This auto-corrects dots to hyphens and - * warns via consola logger, following the same pattern as `normalizeFields` in `api.ts`. + * adds a warning to `warnings`, which the resolver logs, following the same + * pattern as `normalizeFields` in `api.ts`. * * Safe to auto-correct because the input is already invalid (dots are never * valid in platform identifiers) and the correction is unambiguous. */ -function normalizePlatform(platform: string): string { +function normalizePlatform(platform: string, warnings: string[]): string { if (!platform.includes(".")) { return platform; } const corrected = platform.replace(/\./g, "-"); - log.warn( + warnings.push( `Platform '${platform}' uses '.' instead of '-' — interpreting as '${corrected}'`, ); return corrected; @@ -375,13 +381,32 @@ type ParsedProjectSpec = { platform: string; }; +/** `project create`'s positionals, parsed. */ +type ProjectSpecsInput = { + /** Org prefix the names share, if any */ + readonly explicitOrg?: string; + readonly parsed: readonly ParsedProjectSpec[]; + /** Server of a pasted Sentry URL, for `applySentryUrlContext` */ + readonly baseUrl?: string; + readonly warnings: readonly string[]; +}; + +/** `project create`'s `[<org>/]<name>:<platform>...` arguments. */ +export type ProjectCreateArguments = BoundArguments< + ProjectSpecsInput, + ResolvedOrg +>; + /** * Parse and validate a project name independently from its platform source. * Project names cannot contain whitespace in either supported syntax. + * + * @param baseUrls - Collects the server of a pasted Sentry URL */ function parseProjectName( rawName: string, platform: string, + baseUrls: string[], ): ParsedProjectSpec { if (rawName.trim() === "") { throw new ValidationError("Project name cannot be empty.", "name"); @@ -393,7 +418,10 @@ function parseProjectName( ); } - const parsedName = parseOrgProjectArg(rawName); + const { parsed: parsedName, baseUrl } = parseOrgProjectInput(rawName); + if (baseUrl) { + baseUrls.push(baseUrl); + } switch (parsedName.type) { case "explicit": return { @@ -419,12 +447,16 @@ function parseProjectName( } /** Validate and normalize a platform associated with a project name. */ -function parseProjectPlatform(rawName: string, rawPlatform: string): string { +function parseProjectPlatform( + rawName: string, + rawPlatform: string, + warnings: string[], +): string { const trimmedPlatform = rawPlatform.trim(); if (trimmedPlatform === "") { throw new ValidationError(buildPlatformError(rawName), "platform"); } - const platform = normalizePlatform(trimmedPlatform); + const platform = normalizePlatform(trimmedPlatform, warnings); if (!isValidPlatform(platform)) { throw new ValidationError( buildPlatformError(rawName, platform), @@ -440,7 +472,11 @@ function parseProjectPlatform(rawName: string, rawPlatform: string): string { * space-separated fallback — the platform must always be attached with `:`, * with or without an explicit org prefix on the name. */ -function parsePairedProjectSpec(rawSpec: string): ParsedProjectSpec { +function parsePairedProjectSpec( + rawSpec: string, + warnings: string[], + baseUrls: string[], +): ParsedProjectSpec { const separatorIndex = rawSpec.lastIndexOf(":"); if (separatorIndex === -1) { throw new ValidationError( @@ -453,19 +489,18 @@ function parsePairedProjectSpec(rawSpec: string): ParsedProjectSpec { const platform = parseProjectPlatform( rawName, rawSpec.slice(separatorIndex + 1), + warnings, ); - return parseProjectName(rawName, platform); + return parseProjectName(rawName, platform, baseUrls); } /** * Parse one or more `[<org>/]<name>:<platform>` pairs, then require explicit * org prefixes to agree. A lone positional with no colon at all gets a * friendlier "platform is required" error instead of the generic syntax error. + * Has no side effects: warnings and a pasted URL's server are returned. */ -function parseProjectSpecs(rawSpecs: readonly string[]): { - explicitOrg?: string; - parsed: ParsedProjectSpec[]; -} { +function parseProjectSpecs(rawSpecs: readonly string[]): ProjectSpecsInput { if (rawSpecs.length === 0) { throw new ContextError("Project specification", USAGE_HINT, []); } @@ -476,7 +511,11 @@ function parseProjectSpecs(rawSpecs: readonly string[]): { "platform", ); } - const parsed = rawSpecs.map(parsePairedProjectSpec); + const warnings: string[] = []; + const baseUrls: string[] = []; + const parsed = rawSpecs.map((spec) => + parsePairedProjectSpec(spec, warnings, baseUrls), + ); const orgs = new Set( parsed.map((p) => p.org).filter((o): o is string => Boolean(o)), @@ -489,7 +528,33 @@ function parseProjectSpecs(rawSpecs: readonly string[]): { ); } const [explicitOrg] = orgs; - return { explicitOrg, parsed }; + // Applied in order before, so the last pasted URL's server won + return { explicitOrg, parsed, baseUrl: baseUrls.at(-1), warnings }; +} + +/** + * Resolve the org every project is created in, after applying a pasted + * URL's server and logging the parse's warnings. + * + * @throws {ContextError} When no org is given or detected + */ +async function resolveProjectSpecsOrg( + input: ProjectSpecsInput, + cwd: string, +): Promise<ResolvedOrg> { + if (input.baseUrl) { + applySentryUrlContext(input.baseUrl); + } + for (const warning of input.warnings) { + log.warn(warning); + } + const resolved = await resolveOrg({ org: input.explicitOrg, cwd }); + if (!resolved) { + throw new ContextError("Organization", USAGE_HINT, [ + `Include org in name: ${USAGE_HINT}`, + ]); + } + return resolved; } /** @@ -652,14 +717,13 @@ export const createCommand = buildCommand({ ], }, parameters: { - positional: { - kind: "array", - parameter: { - placeholder: "name:platform", - brief: "One or more project name and platform pairs", - parse: String, - }, - }, + positional: boundArguments({ + placeholder: "name:platform", + brief: "One or more project name and platform pairs", + parse: parseProjectSpecs, + resolve: (input: ProjectSpecsInput, context: SentryContext) => + resolveProjectSpecsOrg(input, context.cwd), + }), flags: { team: { kind: "parsed", @@ -671,18 +735,15 @@ export const createCommand = buildCommand({ }, aliases: { ...DRY_RUN_ALIASES, t: "team" }, }, - async *func(this: SentryContext, flags: CreateFlags, ...args: string[]) { - const { cwd } = this; - - const { explicitOrg, parsed } = parseProjectSpecs(args); + async *func( + this: SentryContext, + flags: CreateFlags, + args: ProjectCreateArguments, + ) { + const { parsed } = args.parsed; // Resolve organization once — all projects are created in the same org. - const resolved = await resolveOrg({ org: explicitOrg, cwd }); - if (!resolved) { - throw new ContextError("Organization", USAGE_HINT, [ - `Include org in name: ${USAGE_HINT}`, - ]); - } + const resolved = await args.resolved; const orgSlug = resolved.org; // If the org has no teams, the first project auto-creates one and the rest diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts index f9977e4ac..857efa172 100644 --- a/packages/cli/src/lib/positional-parse.ts +++ b/packages/cli/src/lib/positional-parse.ts @@ -137,6 +137,7 @@ export type BoundArguments<T, R> = BoundPositional<T, R, readonly string[]>; * {@link ArgumentParseError}, which fails like any other argument (exit 252), * unless an argument is `help`: then it turns into the command's help. * + * @param options.kind - What the arguments complete; omit for none * @param options.parse - Parses all the arguments, possibly none; may * throw. Must have no side effects * @param options.resolve - Resolves the parsed value; gets the command @@ -145,7 +146,7 @@ export type BoundArguments<T, R> = BoundPositional<T, R, readonly string[]>; export function boundArguments<T, R, C>(options: { readonly placeholder: string; readonly brief: string; - readonly kind: CompletionTarget; + readonly kind?: CompletionTarget; readonly parse: (raw: readonly string[]) => T; readonly resolve: ( value: T, @@ -160,12 +161,15 @@ export function boundArguments<T, R, C>(options: { readonly parse: (input: string) => string; }; } { + const identity = (input: string) => input; const positional = { kind: "array", parameter: { placeholder: options.placeholder, brief: options.brief, - parse: withCompletionTarget(options.kind, (input: string) => input), + parse: options.kind + ? withCompletionTarget(options.kind, identity) + : identity, }, } as const; argumentParsers.set(positional, function (this: unknown, values) { diff --git a/packages/cli/test/commands/dashboard/create.test.ts b/packages/cli/test/commands/dashboard/create.test.ts index 463e201ac..690ab6461 100644 --- a/packages/cli/test/commands/dashboard/create.test.ts +++ b/packages/cli/test/commands/dashboard/create.test.ts @@ -38,6 +38,7 @@ 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"; import type { DashboardDetail } from "../../../src/types/dashboard.js"; +import { parseFailure } from "../../helpers.js"; // --------------------------------------------------------------------------- // Helpers @@ -134,11 +135,10 @@ describe("dashboard create", () => { const { context } = createMockContext(); const func = await createCommand.loader(); - const err = await func - .call(context, { json: false }) - .catch((e: Error) => e); + const err = await func.call(context, { json: false }).catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain("Dashboard title is required"); + expect(createDashboardSpy).not.toHaveBeenCalled(); }); test("two args parses target + title correctly", async () => { @@ -153,6 +153,18 @@ describe("dashboard create", () => { }); }); + test("words after the target are one title", async () => { + const { context } = createMockContext(); + const func = await createCommand.loader(); + await func.call(context, { json: false }, "my-org/", "My", "Dashboard"); + + expect(createDashboardSpy).toHaveBeenCalledWith("my-org", { + title: "My Dashboard", + widgets: [], + projects: undefined, + }); + }); + test("throws ContextError when org cannot be resolved", async () => { resolveOrgSpy.mockResolvedValue(null); diff --git a/packages/cli/test/commands/init.test.ts b/packages/cli/test/commands/init.test.ts index 41ae6056b..9a569b979 100644 --- a/packages/cli/test/commands/init.test.ts +++ b/packages/cli/test/commands/init.test.ts @@ -23,6 +23,7 @@ import * as prefetchNs from "../../src/lib/init/org-prefetch.js"; import { resetPrefetch } from "../../src/lib/init/org-prefetch.js"; // oxlint-disable-next-line sentry-cli/no-namespace-import -- spyOn requires object reference import * as wizardRunner from "../../src/lib/init/wizard-runner.js"; +import { parseFailure } from "../helpers.js"; /** Minimal org shape for mock returns */ const MOCK_ORG = { id: "1", slug: "resolved-org", name: "Resolved Org" }; @@ -47,8 +48,7 @@ const func = (await initCommand.loader()) as unknown as ( stdin: { isTTY?: boolean }; }, flags: Record<string, unknown>, - first?: string, - second?: string, + ...args: string[] ) => Promise<void>; function makeContext( @@ -545,18 +545,23 @@ describe("init command func", () => { describe("error cases", () => { test("two paths throws ValidationError with specific message", async () => { const ctx = makeContext(); - const promise = func.call(ctx, DEFAULT_FLAGS, "./dir1", "./dir2"); - await expect(promise).rejects.toThrow(ValidationError); - await expect(promise).rejects.toThrow( + const err = await func + .call(ctx, DEFAULT_FLAGS, "./dir1", "./dir2") + .catch(parseFailure); + expect(err).toBeInstanceOf(ValidationError); + expect(err.message).toContain( '"./dir1" and "./dir2" are both directory paths', ); + expect(runWizardSpy).not.toHaveBeenCalled(); }); test("two targets throws ValidationError with specific message", async () => { const ctx = makeContext(); - const promise = func.call(ctx, DEFAULT_FLAGS, "acme/", "other/"); - await expect(promise).rejects.toThrow(ValidationError); - await expect(promise).rejects.toThrow( + const err = await func + .call(ctx, DEFAULT_FLAGS, "acme/", "other/") + .catch(parseFailure); + expect(err).toBeInstanceOf(ValidationError); + expect(err.message).toContain( '"acme/" and "other/" are both treated as targets', ); }); @@ -565,9 +570,22 @@ describe("init command func", () => { // Spaces in org slugs now hit validateResourceId and throw // ValidationError — normalizeSlug no longer converts them. const ctx = makeContext(); - await expect(func.call(ctx, DEFAULT_FLAGS, "acme corp/")).rejects.toThrow( - ValidationError, - ); + const err = await func + .call(ctx, DEFAULT_FLAGS, "acme corp/") + .catch(parseFailure); + expect(err).toBeInstanceOf(ValidationError); + // Rejected before any project lookup + expect(findProjectsSpy).not.toHaveBeenCalled(); + }); + + test("more than two arguments fail to parse", async () => { + const ctx = makeContext(); + const err = await func + .call(ctx, DEFAULT_FLAGS, "acme/", "./app", "extra") + .catch(parseFailure); + expect(err).toBeInstanceOf(ValidationError); + expect(err.message).toContain("Too many arguments"); + expect(runWizardSpy).not.toHaveBeenCalled(); }); }); diff --git a/packages/cli/test/commands/project/create.test.ts b/packages/cli/test/commands/project/create.test.ts index 136b9cc31..eaa497328 100644 --- a/packages/cli/test/commands/project/create.test.ts +++ b/packages/cli/test/commands/project/create.test.ts @@ -42,7 +42,7 @@ import type { ProjectCreatedResult } from "../../../src/lib/formatters/human.js" import * as resolveTarget from "../../../src/lib/resolve-target.js"; import { slugify } from "../../../src/lib/utils.js"; import type { SentryProject, SentryTeam } from "../../../src/types/index.js"; -import { useTestConfigDir } from "../../helpers.js"; +import { parseFailure, useTestConfigDir } from "../../helpers.js"; const sampleTeam: SentryTeam = { id: "1", @@ -221,7 +221,7 @@ describe("project create", () => { const err = await func .call(context, { json: false }, ...args) - .catch((error: Error) => error); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain("must use <name>:<platform> syntax"); @@ -522,7 +522,7 @@ describe("project create", () => { const err = await func .call(context, { json: false }, "my-app:javascript-node") - .catch((e: Error) => e); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect((err as ValidationError).exitCode).toBe(EXIT.VALIDATION); expect(err.message).toContain("Invalid platform 'javascript-node'"); @@ -718,9 +718,10 @@ describe("project create", () => { const func = await createCommand.loader(); // Missing name after slash - await expect( - func.call(context, { json: false }, "acme-corp/:node"), - ).rejects.toThrow(ContextError); + const err = await func + .call(context, { json: false }, "acme-corp/:node") + .catch(parseFailure); + expect(err).toBeInstanceOf(ContextError); }); test("shows platform in human output", async () => { @@ -774,9 +775,7 @@ describe("project create", () => { const { context } = createMockContext(); const func = await createCommand.loader(); - const err = await func - .call(context, { json: false }) - .catch((e: Error) => e); + const err = await func.call(context, { json: false }).catch(parseFailure); expect(err).toBeInstanceOf(ContextError); expect(err.message).toContain("Project specification is required"); expect(err.message).toContain( @@ -792,7 +791,7 @@ describe("project create", () => { const err = await func .call(context, { json: false }, `${name}:node`) - .catch((error: Error) => error); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain("Project name cannot be empty"); }, @@ -806,7 +805,7 @@ describe("project create", () => { const err = await func .call(context, { json: false }, projectArg) - .catch((e: Error) => e); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect((err as ValidationError).exitCode).toBe(EXIT.VALIDATION); expect(err.message).toContain("Platform is required"); @@ -825,7 +824,7 @@ describe("project create", () => { const err = await func .call(context, { json: false }, "web", "api", "node") - .catch((error: Error) => error); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain("must use <name>:<platform> syntax"); expect(createProjectWithDsnSpy).not.toHaveBeenCalled(); @@ -930,7 +929,7 @@ describe("project create", () => { // python.django.rest → python-django-rest (not a valid platform) const err = await func .call(context, { json: false }, "my-app:python.django.rest") - .catch((e: Error) => e); + .catch(parseFailure); expect(err).toBeInstanceOf(CliError); expect(err.message).toContain("Invalid platform 'python-django-rest'"); }); @@ -1038,7 +1037,7 @@ describe("project create", () => { const func = await createCommand.loader(); const err = await func .call(context, { json: false }, "proj1:node", "proj2:not-a-platform") - .catch((e: Error) => e); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain("Invalid platform 'not-a-platform'"); expect(err.message).toContain("sentry project create proj2:<platform>"); @@ -1051,7 +1050,7 @@ describe("project create", () => { const func = await createCommand.loader(); const err = await func .call(context, { json: false }, ...args) - .catch((error: Error) => error); + .catch(parseFailure); expect(err).toBeInstanceOf(ValidationError); expect(err.message).toContain( @@ -1155,7 +1154,7 @@ describe("project create", () => { { json: false, "dry-run": true }, "my-app:invalid-platform", ) - .catch((e: Error) => e); + .catch(parseFailure); expect(err).toBeInstanceOf(CliError); expect(err.message).toContain("Invalid platform"); }); diff --git a/packages/cli/test/helpers.ts b/packages/cli/test/helpers.ts index 078a953b6..621a0c1ac 100644 --- a/packages/cli/test/helpers.ts +++ b/packages/cli/test/helpers.ts @@ -7,7 +7,8 @@ import { mkdirSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; -import { afterEach, beforeEach } from "vitest"; +import { ArgumentParseError } from "@stricli/core"; +import { afterEach, beforeEach, expect } from "vitest"; import { resetAccountSelection, resetAuthRowCache, @@ -370,3 +371,13 @@ export function envWithoutAgent(): NodeJS.ProcessEnv { Object.entries(process.env).filter(([key]) => !agentKeys.has(key)), ); } + +/** + * The error a command's positionals failed to parse with: arguments parse + * before `func` runs, so it reaches the caller wrapped in Stricli's + * ArgumentParseError. For `.catch()` on a command call. + */ +export function parseFailure(error: unknown): Error { + expect(error).toBeInstanceOf(ArgumentParseError); + return (error as ArgumentParseError).exception as Error; +} diff --git a/packages/cli/test/lib/argument-conventions.test.ts b/packages/cli/test/lib/argument-conventions.test.ts index ddef950f7..48723a2b8 100644 --- a/packages/cli/test/lib/argument-conventions.test.ts +++ b/packages/cli/test/lib/argument-conventions.test.ts @@ -11,6 +11,7 @@ import { ArgumentParseError } from "@stricli/core"; import { beforeEach, describe, expect, test, vi } from "vitest"; import { viewCommand as agentConversationViewCommand } from "../../src/commands/agent-conversation/view.js"; +import { createCommand as dashboardCreateCommand } from "../../src/commands/dashboard/create.js"; import { listCommand as dashboardListCommand } from "../../src/commands/dashboard/list.js"; import { restoreCommand as dashboardRestoreCommand } from "../../src/commands/dashboard/restore.js"; import { revisionsCommand as dashboardRevisionsCommand } from "../../src/commands/dashboard/revisions.js"; @@ -24,6 +25,7 @@ import { resolveCommand as feedbackResolveCommand } from "../../src/commands/fee import { spamCommand as feedbackSpamCommand } from "../../src/commands/feedback/spam.js"; import { unresolveCommand as feedbackUnresolveCommand } from "../../src/commands/feedback/unresolve.js"; import { viewCommand as feedbackViewCommand } from "../../src/commands/feedback/view.js"; +import { initCommand } from "../../src/commands/init.js"; import { deleteCommand as integrationDeleteCommand } from "../../src/commands/integration/delete.js"; import { archiveCommand } from "../../src/commands/issue/archive.js"; import { eventsCommand as issueEventsCommand } from "../../src/commands/issue/events.js"; @@ -39,6 +41,7 @@ import { viewCommand as issueViewCommand } from "../../src/commands/issue/view.j import { listCommand as logListCommand } from "../../src/commands/log/list.js"; import { viewCommand as logViewCommand } from "../../src/commands/log/view.js"; import { viewCommand as orgViewCommand } from "../../src/commands/org/view.js"; +import { createCommand as projectCreateCommand } from "../../src/commands/project/create.js"; import { archiveCommand as releaseArchiveCommand } from "../../src/commands/release/archive.js"; import { createCommand as releaseCreateCommand } from "../../src/commands/release/create.js"; import { deleteCommand as releaseDeleteCommand } from "../../src/commands/release/delete.js"; @@ -975,3 +978,102 @@ describe.each(ORG_COMMANDS)("%s", (name, command, before) => { expect(applyUrl).toHaveBeenCalledWith(SERVER); }); }); + +const ORG_URL = `${SERVER}/organizations/acme/`; + +/** + * Commands that take an optional `[<org>/][<project>]` target, then other + * positionals of their own. + */ +const TARGET_THEN_POSITIONALS: readonly { + readonly name: string; + readonly command: unknown; + /** Arguments that name org `acme` */ + readonly args: readonly string[]; + /** The org the command parsed */ + readonly org: (parsed: unknown) => string | undefined; + /** Arguments that fail to parse */ + readonly invalid: readonly string[]; + /** Whether the target may be a pasted Sentry URL */ + readonly url: boolean; +}[] = [ + { + name: "init", + command: initCommand, + args: ["acme/", "./app"], + org: (parsed) => (parsed as { target: ParsedTarget }).target.org, + // Two targets, and no directory + invalid: ["acme/", "other/"], + url: true, + }, + { + name: "dashboard create", + command: dashboardCreateCommand, + args: ["acme/", "My", "Dashboard"], + org: (parsed) => (parsed as { target: ParsedTarget }).target.org, + // No title + invalid: [], + url: true, + }, + { + name: "project create", + command: projectCreateCommand, + args: ["acme/web:node", "acme/api:python"], + org: (parsed) => (parsed as { explicitOrg?: string }).explicitOrg, + // No platform + invalid: ["acme/web"], + url: false, + }, +]; + +describe.each(TARGET_THEN_POSITIONALS)("$name", (entry) => { + test("parses its arguments before the command runs", () => { + const [bound] = bind(entry.command, entry.args); + expect(bound).toBeInstanceOf(BoundPositional); + expect(entry.org((bound as BoundPositional<unknown, unknown>).parsed)).toBe( + "acme", + ); + }); + + test("invalid arguments fail to parse, before the command runs", () => { + expect(parseError(entry.command, entry.invalid)).toBeInstanceOf( + ArgumentParseError, + ); + }); + + test.runIf(entry.url)( + "a pasted URL names the org; its server is applied on resolve, not parse", + async () => { + const [, ...rest] = entry.args; + const [bound] = bind(entry.command, [ORG_URL, ...rest]); + // Reading `resolved` starts the resolver, so only read `parsed` first + const value = bound as BoundPositional<unknown, unknown>; + expect(entry.org(value.parsed)).toBe("acme"); + expect(value.parsed).toMatchObject({ baseUrl: SERVER }); + expect(applyUrl).not.toHaveBeenCalled(); + + await expect(value.resolved).rejects.toBeInstanceOf(UrlApplied); + expect(applyUrl).toHaveBeenCalledWith(SERVER); + }, + ); +}); + +describe("init target and directory", () => { + test.each([ + [["./app"], { directory: "./app", target: { type: "auto-detect" } }], + [ + ["acme/web"], + { target: { type: "explicit", org: "acme", project: "web" } }, + ], + [["acme/", "./app"], { directory: "./app" }], + [ + ["./app", "acme/"], + { directory: "./app", warning: expect.stringMatching(REVERSED) }, + ], + ])("tells the target from the directory in %j", (args, expected) => { + const [bound] = bind(initCommand, args); + expect((bound as BoundPositional<unknown, unknown>).parsed).toMatchObject( + expected, + ); + }); +}); diff --git a/scripts/cli-lint-rules.test.mjs b/scripts/cli-lint-rules.test.mjs index 62a0ce1f5..4d4265f94 100644 --- a/scripts/cli-lint-rules.test.mjs +++ b/scripts/cli-lint-rules.test.mjs @@ -198,7 +198,7 @@ tester.run( }, { // On the allow-list, with its follow-up - filename: "/repo/packages/cli/src/commands/init.ts", + filename: "/repo/packages/cli/src/commands/alert/metrics/view.ts", code: "const org = { placeholder: 'org', parse: parseOrgTarget };", }, ], @@ -235,7 +235,7 @@ tester.run( }, { // A ratchet: an allow-listed file that takes a bound target now - filename: "/repo/packages/cli/src/commands/init.ts", + filename: "/repo/packages/cli/src/commands/alert/metrics/view.ts", code: "const org = { parse: orgOptionalTarget };", errors: [{ message: /Remove this file from the allow-list/ }], },