From 8af331196ed69a580e359b57dc432b73e9eb7017 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 11:51:19 +0000 Subject: [PATCH 1/6] feat(cli): Parse org/project targets in their positional parser Add parseOrgProjectTargetArg, a positional parser that hands func a TargetArg wrapping the raw input and its parseOrgProjectArg result. The parse runs lazily on first access inside func, so bad input still throws the parser's CliError with its exit code instead of Stricli's argument parse error. Plural-alias interception, help recovery and telemetry read the raw input. The typed SDK invoke path now runs positional parsers too, since it calls func without Stricli's scanner. Completion kinds now come from a tag on the parser (withCompletionTarget), so parsing and marker-only parsers complete the same way. Migrate trace list, replay list, feedback list, project view and project delete. Refs #1564 Co-Authored-By: Burak Yigit Kaya --- packages/cli/AGENTS.md | 15 ++- packages/cli/src/commands/feedback/list.ts | 20 ++- packages/cli/src/commands/project/delete.ts | 17 ++- packages/cli/src/commands/project/view.ts | 21 +-- packages/cli/src/commands/replay/list.ts | 22 +++- packages/cli/src/commands/trace/list.ts | 20 ++- packages/cli/src/lib/command.ts | 8 +- packages/cli/src/lib/completion-target.ts | 57 +++++--- packages/cli/src/lib/list-command.ts | 20 ++- packages/cli/src/lib/sdk-invoke.ts | 60 ++++++++- packages/cli/src/lib/target-arg.ts | 79 +++++++++++ .../cli/test/commands/feedback/list.test.ts | 13 +- .../cli/test/commands/project/delete.test.ts | 73 ++++++++--- .../test/commands/project/view.func.test.ts | 65 +++++++-- .../cli/test/commands/replay/list.test.ts | 15 ++- packages/cli/test/commands/trace/list.test.ts | 33 ++--- packages/cli/test/lib/help-positional.test.ts | 8 ++ packages/cli/test/lib/sdk-invoke.test.ts | 76 ++++++++++- packages/cli/test/lib/target-arg.test.ts | 124 ++++++++++++++++++ 19 files changed, 613 insertions(+), 133 deletions(-) create mode 100644 packages/cli/src/lib/target-arg.ts create mode 100644 packages/cli/test/lib/target-arg.test.ts diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 2e8d0c163..f773601b4 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -239,11 +239,24 @@ const parsed = parseOrgProjectArg(targetArg); Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` +**Parsed target positionals:** A single `[/][]` positional can +use `parseOrgProjectTargetArg` from `src/lib/target-arg.ts`. `func` then +receives a `TargetArg` and reads `parsedOrgProject(target)` instead of calling +`parseOrgProjectArg` itself. Parsing runs on first access inside `func`, so +invalid input still fails with the parser's `CliError` and exit code. Use +`target.raw` (or `rawPositional`) for what the user typed. Tests that call +`func` directly pass `parseOrgProjectTargetArg("org/project")`. The SDK invoke +path runs positional parsers too (`applyPositionalParsers`). Positionals whose +meaning depends on how many arguments were given, such as +`[//]`, keep parsing in `func`. + **Shell completion:** Target positionals that take an org or project 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. `LIST_TARGET_POSITIONAL` -and `LIST_ORG_TARGET_POSITIONAL` already use them. +and `LIST_ORG_TARGET_POSITIONAL` already use them. A parser tagged with +`withCompletionTarget`, such as `parseOrgProjectTargetArg`, completes the same +way. `pnpm run generate:completions` collects these into the gitignored `src/generated/completion-targets.json` that the `__complete` fast path reads. `project create` keeps its dedicated `name:platform` completer in diff --git a/packages/cli/src/commands/feedback/list.ts b/packages/cli/src/commands/feedback/list.ts index d146db42e..b6035dc06 100644 --- a/packages/cli/src/commands/feedback/list.ts +++ b/packages/cli/src/commands/feedback/list.ts @@ -11,7 +11,6 @@ import { listFeedback, } from "../../lib/api-client.js"; import { validateLimit } from "../../lib/arg-parsing.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { advancePaginationState, buildPaginationContextKey, @@ -35,10 +34,15 @@ import { jsonTransformListResult } from "../../lib/org-list.js"; import { withProgress } from "../../lib/polling.js"; import { type ResolvedOrgOptionalTarget, - resolveOrgOptionalFromArg, + resolveOrgOptionalTarget, toNumericId, } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, +} from "../../lib/target-arg.js"; import { appendPeriodHint, PERIOD_BRIEF, @@ -159,7 +163,7 @@ export const listCommand = buildListCommand("feedback", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectTarget, + parse: parseOrgProjectTargetArg, optional: true, }, ], @@ -196,9 +200,13 @@ export const listCommand = buildListCommand("feedback", { q: "query", }, }, - async *func(this: SentryContext, flags: ListFlags, target?: string) { - const resolved = await resolveOrgOptionalFromArg( - target, + async *func( + this: SentryContext, + flags: ListFlags, + target?: OrgProjectTargetArg, + ) { + const resolved = await resolveOrgOptionalTarget( + parsedOrgProject(target), this.cwd, COMMAND_NAME, ); diff --git a/packages/cli/src/commands/project/delete.ts b/packages/cli/src/commands/project/delete.ts index 3598d0554..514754c2e 100644 --- a/packages/cli/src/commands/project/delete.ts +++ b/packages/cli/src/commands/project/delete.ts @@ -24,8 +24,6 @@ import { getOrganization, getProject, } from "../../lib/api-client.js"; -import { parseOrgProjectArg } from "../../lib/arg-parsing.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { getCachedOrgRole } from "../../lib/db/regions.js"; import { ApiError } from "../../lib/errors.js"; import { @@ -42,6 +40,11 @@ import { } from "../../lib/mutate-command.js"; import { resolveProjectBoundTarget } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, +} from "../../lib/target-arg.js"; const log = logger.withTag("project.delete"); @@ -169,16 +172,20 @@ export const deleteCommand = buildDeleteCommand({ { placeholder: "org/project", brief: "/ or (search across orgs)", - parse: parseOrgProjectTarget, + parse: parseOrgProjectTargetArg, }, ], }, }, - async *func(this: SentryContext, flags: DeleteFlags, target: string) { + async *func( + this: SentryContext, + flags: DeleteFlags, + target: OrgProjectTargetArg, + ) { const { cwd } = this; // Block auto-detect for safety — destructive commands require explicit targets - const parsed = parseOrgProjectArg(target); + const parsed = parsedOrgProject(target); requireExplicitTarget( parsed, "Project target", diff --git a/packages/cli/src/commands/project/view.ts b/packages/cli/src/commands/project/view.ts index aa4e2bb5f..7315e0fc5 100644 --- a/packages/cli/src/commands/project/view.ts +++ b/packages/cli/src/commands/project/view.ts @@ -11,13 +11,9 @@ import { resolveOrgDisplayName, tryGetPrimaryDsn, } from "../../lib/api-client.js"; -import { - ProjectSpecificationType, - parseOrgProjectArg, -} from "../../lib/arg-parsing.js"; +import { ProjectSpecificationType } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { AuthError, ContextError, withAuthGuard } from "../../lib/errors.js"; import { divider, formatProjectDetails } from "../../lib/formatters/index.js"; import { filterFields } from "../../lib/formatters/json.js"; @@ -34,6 +30,11 @@ import { resolveProjectBoundSlug, } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, +} from "../../lib/target-arg.js"; import type { SentryProject } from "../../types/index.js"; type ViewFlags = { @@ -280,7 +281,7 @@ export const viewCommand = buildCommand({ { placeholder: "org/project", brief: "/, (search), or omit for auto-detect", - parse: parseOrgProjectTarget, + parse: parseOrgProjectTargetArg, optional: true, }, ], @@ -295,11 +296,15 @@ export const viewCommand = buildCommand({ }, aliases: { ...FRESH_ALIASES, w: "web" }, }, - async *func(this: SentryContext, flags: ViewFlags, targetArg?: string) { + async *func( + this: SentryContext, + flags: ViewFlags, + target?: OrgProjectTargetArg, + ) { applyFreshFlag(flags); const { cwd } = this; - const parsed = parseOrgProjectArg(targetArg); + const parsed = parsedOrgProject(target); let resolvedTargets: ResolvedTarget[]; let footer: string | undefined; diff --git a/packages/cli/src/commands/replay/list.ts b/packages/cli/src/commands/replay/list.ts index 47eeed5aa..6ee12dede 100644 --- a/packages/cli/src/commands/replay/list.ts +++ b/packages/cli/src/commands/replay/list.ts @@ -11,7 +11,6 @@ import { type ReplaySortValue, } from "../../lib/api-client.js"; import { validateLimit } from "../../lib/arg-parsing.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { advancePaginationState, buildPaginationContextKey, @@ -45,8 +44,13 @@ import { getReplayUserLabel, parseReplayEnvironmentFilter, } from "../../lib/replay-search.js"; -import { resolveOrgOptionalFromArg } from "../../lib/resolve-target.js"; +import { resolveOrgOptionalTarget } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, +} from "../../lib/target-arg.js"; import { appendPeriodHint, serializeTimeRange, @@ -281,7 +285,7 @@ export const listCommand = buildListCommand("replay", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectTarget, + parse: parseOrgProjectTargetArg, optional: true, }, ], @@ -323,13 +327,21 @@ export const listCommand = buildListCommand("replay", { s: "sort", }, }, - async *func(this: SentryContext, flags: ListFlags, target?: string) { + async *func( + this: SentryContext, + flags: ListFlags, + target?: OrgProjectTargetArg, + ) { const { cwd } = this; const timeRange = flags.period; const environment = parseReplayEnvironmentFilter(flags.environment); const { query } = flags; - const resolved = await resolveOrgOptionalFromArg(target, cwd, COMMAND_NAME); + const resolved = await resolveOrgOptionalTarget( + parsedOrgProject(target), + cwd, + COMMAND_NAME, + ); const contextKey = buildPaginationContextKey( "replay", diff --git a/packages/cli/src/commands/trace/list.ts b/packages/cli/src/commands/trace/list.ts index cc55d4b0a..b7a644682 100644 --- a/packages/cli/src/commands/trace/list.ts +++ b/packages/cli/src/commands/trace/list.ts @@ -7,7 +7,6 @@ import type { SentryContext } from "../../context.js"; import { listTransactions } from "../../lib/api-client.js"; import { validateLimit } from "../../lib/arg-parsing.js"; -import { parseOrgProjectTarget } from "../../lib/completion-target.js"; import { advancePaginationState, buildPaginationContextKey, @@ -32,9 +31,14 @@ import { import { withProgress } from "../../lib/polling.js"; import { resolveLogProjectId, - resolveProjectBoundFromArg, + resolveProjectBoundTarget, } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, +} from "../../lib/target-arg.js"; import { appendPeriodHint, serializeTimeRange, @@ -225,7 +229,7 @@ export const listCommand = buildListCommand("trace", { { placeholder: "org/project", brief: "/ or (search)", - parse: parseOrgProjectTarget, + parse: parseOrgProjectTargetArg, optional: true, }, ], @@ -258,14 +262,18 @@ export const listCommand = buildListCommand("trace", { s: "sort", }, }, - async *func(this: SentryContext, flags: ListFlags, target?: string) { + async *func( + this: SentryContext, + flags: ListFlags, + target?: OrgProjectTargetArg, + ) { const { cwd } = this; const timeRange = flags.period; const { query } = flags; // Resolve org/project from positional arg, config, or DSN auto-detection - const { org, project } = await resolveProjectBoundFromArg( - target, + const { org, project } = await resolveProjectBoundTarget( + parsedOrgProject(target), cwd, COMMAND_NAME, ); diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 33d6de8f7..36584bb84 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -65,6 +65,7 @@ import { parseLogLevel, setLogLevel, } from "./logger.js"; +import { rawPositional } from "./target-arg.js"; import { setArgsContext, setFlagContext, withTracing } from "./telemetry.js"; import { applyTipsPreference } from "./tips.js"; @@ -674,7 +675,10 @@ export function buildCommand< } if ( args.length === 0 || - !args.some((a) => a === "help" || a === "--h" || a === "-help") + !args.some((a) => { + const raw = rawPositional(a); + return raw === "help" || raw === "--h" || raw === "-help"; + }) ) { return false; } @@ -722,7 +726,7 @@ export function buildCommand< const cleanFlags = cleanRawFlags(flags as Record); setFlagContext(cleanFlags); if (args.length > 0) { - setArgsContext(args); + setArgsContext(args.map(rawPositional)); } // Environment-based JSON mode (used by library entry point) diff --git a/packages/cli/src/lib/completion-target.ts b/packages/cli/src/lib/completion-target.ts index b29ed62cb..b13812eec 100644 --- a/packages/cli/src/lib/completion-target.ts +++ b/packages/cli/src/lib/completion-target.ts @@ -1,12 +1,15 @@ /** * Completion-aware positional parsers. * - * Target positionals use these instead of `String` so shell completion can - * read the target kind straight from the command's `parameters`. Both parsers - * return their input unchanged; the kind is identified by which function a - * positional uses. `script/generate-completion-targets.ts` collects them at - * build time because the `__complete` fast path cannot boot Stricli to walk - * the route tree. + * Target positionals use a parser tagged with {@link withCompletionTarget} + * instead of `String` so shell completion can read the target kind straight + * from the command's `parameters`. `script/generate-completion-targets.ts` + * collects them at build time because the `__complete` fast path cannot boot + * Stricli to walk the route tree. + * + * {@link parseOrgTarget} and {@link parseOrgProjectTarget} return their input + * unchanged and only mark the kind. Parsers that also parse the value, such + * as `parseOrgProjectTargetArg` in `target-arg.ts`, use the same tag. */ /** Completes organization slugs only (e.g. `org`, `org/version`, `org/id`). */ @@ -21,30 +24,46 @@ export const COMPLETION_TARGETS = [ORG_TARGET, ORG_PROJECT_TARGET] as const; /** Which cached slugs shell completion suggests for a target positional. */ export type CompletionTarget = (typeof COMPLETION_TARGETS)[number]; -/** Parser for positionals whose leading segment is an organization slug. */ -export function parseOrgTarget(input: string): string { - return input; +const COMPLETION_TARGET_KEY = Symbol("completionTarget"); + +/** + * Tag a positional parser with the completion kind it accepts. + * + * @returns `parse` itself, so the tag travels with the function + */ +export function withCompletionTarget

unknown>( + kind: CompletionTarget, + parse: P, +): P { + Object.defineProperty(parse, COMPLETION_TARGET_KEY, { value: kind }); + return parse; } +/** Parser for positionals whose leading segment is an organization slug. */ +export const parseOrgTarget = withCompletionTarget( + ORG_TARGET, + (input: string): string => input, +); + /** Parser for positionals that start with `org/project`, a project, or an alias. */ -export function parseOrgProjectTarget(input: string): string { - return input; -} +export const parseOrgProjectTarget = withCompletionTarget( + ORG_PROJECT_TARGET, + (input: string): string => input, +); /** * Read the completion kind from a positional's `parse` function. * - * @returns The kind for {@link parseOrgTarget} / {@link parseOrgProjectTarget}, + * @returns The kind a parser was tagged with by {@link withCompletionTarget}, * or `undefined` for any other parser */ export function getCompletionTarget( parse: unknown, ): CompletionTarget | undefined { - if (parse === parseOrgTarget) { - return ORG_TARGET; - } - if (parse === parseOrgProjectTarget) { - return ORG_PROJECT_TARGET; + if (typeof parse !== "function") { + return; } - return; + return (parse as { [COMPLETION_TARGET_KEY]?: CompletionTarget })[ + COMPLETION_TARGET_KEY + ]; } diff --git a/packages/cli/src/lib/list-command.ts b/packages/cli/src/lib/list-command.ts index 8092e01a4..87adec7ec 100644 --- a/packages/cli/src/lib/list-command.ts +++ b/packages/cli/src/lib/list-command.ts @@ -45,6 +45,7 @@ import { type OrgListConfig, } from "./org-list.js"; import { disableResponseCache } from "./response-cache.js"; +import { rawPositional } from "./target-arg.js"; import { PERIOD_BRIEF, parsePeriod } from "./time-range.js"; import { shellQuote } from "./utils.js"; @@ -617,19 +618,16 @@ export function buildListCommand< // Auto-apply fresh flag before command runs applyFreshFlag(flags as unknown as { readonly fresh: boolean }); - // The first positional arg is always the target (org/project pattern). - // Intercept it to handle plural alias confusion. - if ( - args.length > 0 && - (typeof args[0] === "string" || args[0] === undefined) - ) { + // The first positional arg is always the target (org/project pattern), + // either a raw string or a TargetArg. Intercept it to handle plural + // alias confusion. + const rawTarget = rawPositional(args[0]); + if (typeof rawTarget === "string") { // All list commands use SentryContext which has stderr at top level const ctx = this as unknown as { stderr: { write(s: string): void } }; - args[0] = interceptSubcommand( - args[0] as string | undefined, - ctx.stderr, - routeName, - ); + if (interceptSubcommand(rawTarget, ctx.stderr, routeName) === undefined) { + args[0] = undefined; + } } return originalFunc.call(this, flags, ...(args as unknown as ARGS)); } as typeof originalFunc; diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index 25cbb66a9..47ec012f4 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -76,16 +76,29 @@ export type FlagDef = { variadic?: boolean; }; -/** Resolved command: handler function + flag definitions for default application. */ +/** One positional parameter — subset needed to run its parser. */ +type PositionalParamDef = { parse: (this: unknown, input: string) => unknown }; + +/** Stricli positional definition shape — subset needed by {@link applyPositionalParsers}. */ +type PositionalDef = + | { kind: "tuple"; parameters: readonly PositionalParamDef[] } + | { kind: "array"; parameter: PositionalParamDef }; + +/** Resolved command: handler function + parameter definitions. */ type ResolvedCommand = { handler: CommandHandler; flagDefs: Record; + positionalDef?: PositionalDef; }; /** Cached command entries, keyed by joined path (e.g., "org.list"). */ const commandCache = new Map< string, - { loader: () => Promise; flagDefs: Record } + { + loader: () => Promise; + flagDefs: Record; + positionalDef?: PositionalDef; + } >(); /** @@ -117,10 +130,41 @@ async function resolveCommand(path: string[]): Promise { (m: any) => (typeof m === "function" ? m : m.default), ), flagDefs, + positionalDef: command.parameters?.positional, }; commandCache.set(key, cached); } - return { handler: await cached.loader(), flagDefs: cached.flagDefs }; + return { + handler: await cached.loader(), + flagDefs: cached.flagDefs, + positionalDef: cached.positionalDef, + }; +} + +/** + * Run a command's positional parsers on SDK-provided strings. + * + * Like flag defaults ({@link applyFlagDefaults}), positional parsing normally + * happens in Stricli's argument scanner, which the SDK invoke path bypasses. + * Without this, a command whose positional parses its own value (e.g. + * `parseOrgProjectTargetArg`) would receive a bare string. + * + * @param args - Positional strings from the SDK caller + * @param positionalDef - The command's Stricli positional definition + * @param context - `this` for each parser, as Stricli passes it + */ +export function applyPositionalParsers( + args: readonly string[], + positionalDef: PositionalDef | undefined, + context: unknown, +): unknown[] { + return args.map((arg, index) => { + const param = + positionalDef?.kind === "array" + ? positionalDef.parameter + : positionalDef?.parameters[index]; + return param ? param.parse.call(context, arg) : arg; + }); } /** @@ -559,7 +603,8 @@ export function buildInvoker(options?: SentryOptions) { ): Promise | AsyncIterable { if (meta?.streaming) { return executeWithStream(options, async (ctx, span) => { - const { handler, flagDefs } = await resolveCommand(commandPath); + const { handler, flagDefs, positionalDef } = + await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); @@ -568,13 +613,14 @@ export function buildInvoker(options?: SentryOptions) { await handler.call( ctx.context, { ...resolvedFlags, json: true }, - ...positionalArgs, + ...applyPositionalParsers(positionalArgs, positionalDef, ctx.context), ); }); } return executeWithCapture(options, async (ctx, span) => { - const { handler, flagDefs } = await resolveCommand(commandPath); + const { handler, flagDefs, positionalDef } = + await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); @@ -583,7 +629,7 @@ export function buildInvoker(options?: SentryOptions) { await handler.call( ctx.context, { ...resolvedFlags, json: true }, - ...positionalArgs, + ...applyPositionalParsers(positionalArgs, positionalDef, ctx.context), ); }); }; diff --git a/packages/cli/src/lib/target-arg.ts b/packages/cli/src/lib/target-arg.ts new file mode 100644 index 000000000..b319c1c46 --- /dev/null +++ b/packages/cli/src/lib/target-arg.ts @@ -0,0 +1,79 @@ +/** + * Target positionals that parse their own value. + * + * A positional that uses one of these parsers reaches the command function + * as a {@link TargetArg} instead of a raw string, so the target syntax is + * defined once, next to the positional, rather than re-parsed in every + * command's `func`. + * + * Parsing is lazy: Stricli only wraps the input, and the parse runs on the + * first {@link TargetArg.parsed} access inside `func`. Invalid input therefore + * still throws the parser's `CliError` subclass from the command, with + * its exit code, telemetry, and `--json` error output, instead of Stricli's + * generic argument-parse error. Code that needs what the user typed (plural + * alias interception, `help` recovery, telemetry) reads {@link TargetArg.raw} + * through {@link rawPositional}. + */ + +import { type ParsedOrgProject, parseOrgProjectArg } from "./arg-parsing.js"; +import { + ORG_PROJECT_TARGET, + withCompletionTarget, +} from "./completion-target.js"; + +/** A scanned target positional: the raw input and its lazily parsed value. */ +export class TargetArg { + readonly raw: string; + readonly #parse: (raw: string) => T; + #parsed: { value: T } | undefined; + + constructor(raw: string, parse: (raw: string) => T) { + this.raw = raw; + this.#parse = parse; + } + + /** + * The parsed target. Parses on first access. + * + * @throws The parser's error (usually a `ValidationError`) for invalid input + */ + get parsed(): T { + this.#parsed ??= { value: this.#parse(this.raw) }; + return this.#parsed.value; + } +} + +/** An `[/][]` positional, parsed by `parseOrgProjectArg`. */ +export type OrgProjectTargetArg = TargetArg; + +/** + * Positional parser for `/`, `/`, ``, and Sentry + * URLs. Completes like `parseOrgProjectTarget`. + */ +export const parseOrgProjectTargetArg = withCompletionTarget( + ORG_PROJECT_TARGET, + (input: string): OrgProjectTargetArg => + new TargetArg(input, (raw) => parseOrgProjectArg(raw)), +); + +/** + * The parsed value of an `[/][]` positional. + * + * @returns Auto-detect when the positional was omitted, as + * `parseOrgProjectArg(undefined)` does + * @throws The parser's error for invalid input + */ +export function parsedOrgProject( + target: OrgProjectTargetArg | undefined, +): ParsedOrgProject { + return target === undefined ? { type: "auto-detect" } : target.parsed; +} + +/** + * What the user typed for a positional value. + * + * @returns {@link TargetArg.raw} for a target positional, otherwise `value` + */ +export function rawPositional(value: unknown): unknown { + return value instanceof TargetArg ? value.raw : value; +} diff --git a/packages/cli/test/commands/feedback/list.test.ts b/packages/cli/test/commands/feedback/list.test.ts index 6773a19e9..87df0b8f0 100644 --- a/packages/cli/test/commands/feedback/list.test.ts +++ b/packages/cli/test/commands/feedback/list.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { listCommand } from "../../../src/commands/feedback/list.js"; import { ApiError, ValidationError } from "../../../src/lib/errors.js"; +import { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import type { SentryFeedback } from "../../../src/types/index.js"; @@ -101,7 +102,7 @@ describe("feedback list", () => { slug: "test-project", name: "Test Project", }); - resolveTargetSpy = vi.spyOn(resolveTarget, "resolveOrgOptionalFromArg"); + resolveTargetSpy = vi.spyOn(resolveTarget, "resolveOrgOptionalTarget"); resolveCursorSpy = vi.spyOn(paginationDb, "resolveCursor").mockReturnValue({ cursor: undefined, direction: "next", @@ -140,7 +141,7 @@ describe("feedback list", () => { json: true, fresh: false, }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); expect(listFeedbackSpy).toHaveBeenCalledWith("test-org", "test-project", { @@ -184,7 +185,7 @@ describe("feedback list", () => { json: false, fresh: false, }, - "test-org/", + parseOrgProjectTargetArg("test-org/"), ); expect(listFeedbackSpy).toHaveBeenCalledWith("test-org", "", { @@ -230,7 +231,7 @@ describe("feedback list", () => { const output = stdoutWrite.mock.calls.map((call) => call[0]).join(""); expect(JSON.parse(output).data).toEqual([{ shortId: "TEST-PROJECT-2SDJ" }]); expect(resolveTargetSpy).toHaveBeenCalledWith( - undefined, + { type: "auto-detect" }, "/tmp", "feedback list", ); @@ -255,11 +256,11 @@ describe("feedback list", () => { json: true, fresh: false, }, - "test-project", + parseOrgProjectTargetArg("test-project"), ); expect(resolveTargetSpy).toHaveBeenCalledWith( - "test-project", + { type: "project-search", projectSlug: "test-project" }, "/tmp", "feedback list", ); diff --git a/packages/cli/test/commands/project/delete.test.ts b/packages/cli/test/commands/project/delete.test.ts index dcd839625..4b39926f2 100644 --- a/packages/cli/test/commands/project/delete.test.ts +++ b/packages/cli/test/commands/project/delete.test.ts @@ -112,6 +112,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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import type { SentryProject } from "../../../src/types/index.js"; const sampleProject: SentryProject = { @@ -178,7 +179,11 @@ describe("project delete", () => { test("deletes project with explicit org/project and --yes", async () => { const { context, stdoutWrite } = createMockContext(); const func = await deleteCommand.loader(); - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); expect(getProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -192,7 +197,11 @@ describe("project delete", () => { test("delegates to resolveProjectBoundTarget for resolution", async () => { const { context } = createMockContext(); const func = await deleteCommand.loader(); - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -208,7 +217,7 @@ describe("project delete", () => { test("resolves bare slug via resolveProjectBoundTarget", async () => { const { context } = createMockContext(); const func = await deleteCommand.loader(); - await func.call(context, defaultFlags, "my-app"); + await func.call(context, defaultFlags, parseOrgProjectTargetArg("my-app")); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -227,7 +236,11 @@ describe("project delete", () => { // isatty(0) returns false in test environments (non-TTY) await expect( - func.call(context, { ...defaultFlags, yes: false }, "acme-corp/my-app"), + func.call( + context, + { ...defaultFlags, yes: false }, + parseOrgProjectTargetArg("acme-corp/my-app"), + ), ).rejects.toThrow("non-interactive mode"); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -242,7 +255,11 @@ describe("project delete", () => { const func = await deleteCommand.loader(); await expect( - func.call(context, defaultFlags, "acme-corp/my-app"), + func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ), ).rejects.toThrow(ApiError); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -263,7 +280,11 @@ describe("project delete", () => { const func = await deleteCommand.loader(); try { - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -290,7 +311,11 @@ describe("project delete", () => { const func = await deleteCommand.loader(); try { - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -312,7 +337,11 @@ describe("project delete", () => { const func = await deleteCommand.loader(); try { - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -327,7 +356,11 @@ describe("project delete", () => { test("verifies project exists before attempting delete", async () => { const { context } = createMockContext(); const func = await deleteCommand.loader(); - await func.call(context, defaultFlags, "acme-corp/my-app"); + await func.call( + context, + defaultFlags, + parseOrgProjectTargetArg("acme-corp/my-app"), + ); // getProject must be called before deleteProject const getProjectOrder = getProjectSpy.mock.invocationCallOrder[0]; @@ -343,7 +376,7 @@ describe("project delete", () => { await func.call( context, { ...defaultFlags, "dry-run": true }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(getProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -361,7 +394,7 @@ describe("project delete", () => { await func.call( context, { ...defaultFlags, "dry-run": true, json: true }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -381,7 +414,7 @@ describe("project delete", () => { await func.call( context, { ...defaultFlags, json: true }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -398,9 +431,9 @@ describe("project delete", () => { const func = await deleteCommand.loader(); // Empty string triggers auto-detect in parseOrgProjectArg - await expect(func.call(context, defaultFlags, "")).rejects.toThrow( - ContextError, - ); + await expect( + func.call(context, defaultFlags, parseOrgProjectTargetArg("")), + ).rejects.toThrow(ContextError); expect(deleteProjectSpy).not.toHaveBeenCalled(); }); @@ -465,7 +498,7 @@ describe("project delete — interactive confirmation", () => { await func.call( context, { yes: false, "dry-run": false }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -481,7 +514,7 @@ describe("project delete — interactive confirmation", () => { await func.call( context, { yes: false, "dry-run": false }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -497,7 +530,7 @@ describe("project delete — interactive confirmation", () => { await func.call( context, { yes: false, "dry-run": false }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -512,7 +545,7 @@ describe("project delete — interactive confirmation", () => { await func.call( context, { yes: false, "dry-run": false }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -526,7 +559,7 @@ describe("project delete — interactive confirmation", () => { await func.call( context, { yes: false, "dry-run": false }, - "acme-corp/my-app", + parseOrgProjectTargetArg("acme-corp/my-app"), ); expect(mockPrompt).toHaveBeenCalledWith( diff --git a/packages/cli/test/commands/project/view.func.test.ts b/packages/cli/test/commands/project/view.func.test.ts index 5d385b588..b718ea27c 100644 --- a/packages/cli/test/commands/project/view.func.test.ts +++ b/packages/cli/test/commands/project/view.func.test.ts @@ -51,6 +51,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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import type { ProjectKey, SentryProject } from "../../../src/types/sentry.js"; const sampleProject: SentryProject = { @@ -111,7 +112,11 @@ describe("viewCommand.func", () => { const { context, stdoutWrite } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: true, web: false }, "my-org/test-project"); + await func.call( + context, + { json: true, web: false }, + parseOrgProjectTargetArg("my-org/test-project"), + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -132,7 +137,7 @@ describe("viewCommand.func", () => { await func.call( context, { json: false, web: false }, - "my-org/test-project", + parseOrgProjectTargetArg("my-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -145,7 +150,11 @@ describe("viewCommand.func", () => { const { context } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: false, web: true }, "my-org/test-project"); + await func.call( + context, + { json: false, web: true }, + parseOrgProjectTargetArg("my-org/test-project"), + ); expect(openInBrowserSpy).toHaveBeenCalled(); // Should NOT fetch project details when using --web @@ -195,7 +204,11 @@ describe("viewCommand.func", () => { const { context, stdoutWrite } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: true, web: false }, "frontend"); + await func.call( + context, + { json: true, web: false }, + parseOrgProjectTargetArg("frontend"), + ); expect(resolveProjectBoundSlugSpy).toHaveBeenCalledWith( "frontend", @@ -214,7 +227,11 @@ describe("viewCommand.func", () => { const func = await viewCommand.loader(); try { - await func.call(context, { json: false, web: false }, "my-org/"); + await func.call( + context, + { json: false, web: false }, + parseOrgProjectTargetArg("my-org/"), + ); expect.unreachable("Should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ContextError); @@ -301,7 +318,11 @@ describe("viewCommand.func", () => { const func = await viewCommand.loader(); await expect( - func.call(context, { json: false, web: false }, "my-org/bad-project"), + func.call( + context, + { json: false, web: false }, + parseOrgProjectTargetArg("my-org/bad-project"), + ), ).rejects.toThrow("404 Not Found"); expect(getProjectSpy).toHaveBeenCalledWith("my-org", "bad-project"); @@ -322,7 +343,11 @@ describe("viewCommand.func", () => { const func = await viewCommand.loader(); await expect( - func.call(context, { json: false, web: false }, "frontend"), + func.call( + context, + { json: false, web: false }, + parseOrgProjectTargetArg("frontend"), + ), ).rejects.toThrow("403 Forbidden"); }); @@ -365,7 +390,11 @@ describe("viewCommand.func", () => { const func = await viewCommand.loader(); await expect( - func.call(context, { json: false, web: false }, "my-org/test-project"), + func.call( + context, + { json: false, web: false }, + parseOrgProjectTargetArg("my-org/test-project"), + ), ).rejects.toThrow(AuthError); }); @@ -383,7 +412,11 @@ describe("viewCommand.func", () => { const { context, stdoutWrite } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: true, web: false }, "my-org/test-project"); + await func.call( + context, + { json: true, web: false }, + parseOrgProjectTargetArg("my-org/test-project"), + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -405,7 +438,11 @@ describe("viewCommand.func", () => { const { context, stdoutWrite } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: true, web: false }, "my-org/test-project"); + await func.call( + context, + { json: true, web: false }, + parseOrgProjectTargetArg("my-org/test-project"), + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -420,7 +457,11 @@ describe("viewCommand.func", () => { const { context, stdoutWrite } = createMockContext(); const func = await viewCommand.loader(); - await func.call(context, { json: true, web: false }, "my-org/test-project"); + await func.call( + context, + { json: true, web: false }, + parseOrgProjectTargetArg("my-org/test-project"), + ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -447,7 +488,7 @@ describe("viewCommand.func", () => { web: false, fields: ["slug", "organization.slug"], }, - "my-org/test-project", + parseOrgProjectTargetArg("my-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); diff --git a/packages/cli/test/commands/replay/list.test.ts b/packages/cli/test/commands/replay/list.test.ts index 05f8b5103..41371f20f 100644 --- a/packages/cli/test/commands/replay/list.test.ts +++ b/packages/cli/test/commands/replay/list.test.ts @@ -48,6 +48,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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import { REPLAY_LIST_FIELDS, @@ -106,7 +107,7 @@ describe("listCommand.func", () => { beforeEach(() => { listReplaysSpy = vi.spyOn(apiClient, "listReplays"); - resolveTargetSpy = vi.spyOn(resolveTarget, "resolveOrgOptionalFromArg"); + resolveTargetSpy = vi.spyOn(resolveTarget, "resolveOrgOptionalTarget"); resolveCursorSpy = vi.spyOn(paginationDb, "resolveCursor").mockReturnValue({ cursor: undefined, direction: "next" as const, @@ -139,7 +140,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 25, json: true, period: parsePeriod("7d"), sort: "-started_at" }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ); expect(listReplaysSpy).toHaveBeenCalledWith("test-org", { @@ -181,7 +182,7 @@ describe("listCommand.func", () => { sort: "-started_at", query: "bad:::query", }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ), ).rejects.toBeInstanceOf(ValidationError); }); @@ -203,7 +204,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ), ).rejects.toBeInstanceOf(ApiError); }); @@ -226,7 +227,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ); expect(listReplaysSpy).toHaveBeenCalledWith("test-org", { @@ -255,7 +256,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ); const output = stdoutWrite.mock.calls.map((call) => call[0]).join(""); @@ -282,7 +283,7 @@ describe("listCommand.func", () => { period: parsePeriod(LIST_PERIOD_FLAG.default), sort: "-started_at", }, - "test-org/cli", + parseOrgProjectTargetArg("test-org/cli"), ); const output = stdoutWrite.mock.calls.map((call) => call[0]).join(""); diff --git a/packages/cli/test/commands/trace/list.test.ts b/packages/cli/test/commands/trace/list.test.ts index fb89ef87e..ae9442eac 100644 --- a/packages/cli/test/commands/trace/list.test.ts +++ b/packages/cli/test/commands/trace/list.test.ts @@ -57,6 +57,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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import type { TransactionListItem } from "../../../src/types/sentry.js"; @@ -333,7 +334,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -353,7 +354,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -372,7 +373,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -387,7 +388,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -409,7 +410,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 2, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -425,7 +426,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 100, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -441,7 +442,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -463,7 +464,7 @@ describe("listCommand.func", () => { query: "transaction:GET", period: parsePeriod("7d"), }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); expect(listTransactionsSpy).toHaveBeenCalledWith( @@ -491,7 +492,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -514,7 +515,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "duration", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -531,7 +532,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const callArgs = listTransactionsSpy.mock.calls[0]; @@ -549,7 +550,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -574,7 +575,7 @@ describe("listCommand.func", () => { query: 'transaction:"POST /checkout"', period: parsePeriod("7d"), }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -590,7 +591,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -608,7 +609,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("24h") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); expect(listTransactionsSpy).toHaveBeenCalledWith( @@ -631,7 +632,7 @@ describe("listCommand.func", () => { await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("30d") }, - "test-org/test-project", + parseOrgProjectTargetArg("test-org/test-project"), ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); diff --git a/packages/cli/test/lib/help-positional.test.ts b/packages/cli/test/lib/help-positional.test.ts index 598c92a67..380755ff4 100644 --- a/packages/cli/test/lib/help-positional.test.ts +++ b/packages/cli/test/lib/help-positional.test.ts @@ -142,6 +142,14 @@ describe("help recovery on ResolutionError", () => { expect(stderr).toContain("sentry issue list --help"); }); + + test("sentry project view help → recovers through a parsed target positional", async () => { + // project view's positional reaches func as a TargetArg, not a string + const { stdout, stderr } = await runCommand(["project", "view", "help"]); + + expect(stdout).toContain("sentry project view"); + expect(stderr).toContain("sentry project view --help"); + }); }); describe("help recovery on ValidationError", () => { diff --git a/packages/cli/test/lib/sdk-invoke.test.ts b/packages/cli/test/lib/sdk-invoke.test.ts index 7d2c8b764..d25297bf5 100644 --- a/packages/cli/test/lib/sdk-invoke.test.ts +++ b/packages/cli/test/lib/sdk-invoke.test.ts @@ -1,16 +1,23 @@ /** * Unit tests for the SDK invoke layer. * - * Focuses on `applyFlagDefaults` which replicates Stricli's default - * application for the SDK direct-invoke path (bypasses Stricli parsing). + * Focuses on `applyFlagDefaults` and `applyPositionalParsers`, which + * replicate Stricli's default application and positional parsing for the SDK + * direct-invoke path (bypasses Stricli parsing). */ import { describe, expect, test } from "vitest"; import { applyFlagDefaults, + applyPositionalParsers, + buildInvoker, type FlagDef, parseOutput, } from "../../src/lib/sdk-invoke.js"; +import { parseOrgProjectTargetArg } from "../../src/lib/target-arg.js"; +import { useTestConfigDir } from "../helpers.js"; + +useTestConfigDir("sdk-invoke-"); // --------------------------------------------------------------------------- // applyFlagDefaults — parsed flags with defaults @@ -222,3 +229,68 @@ describe("parseOutput: binary output", () => { expect(parseOutput(obj, ["ignored"])).toBe(obj); }); }); + +// --------------------------------------------------------------------------- +// applyPositionalParsers +// --------------------------------------------------------------------------- + +describe("applyPositionalParsers", () => { + test("runs tuple parsers by position", () => { + const [target, name] = applyPositionalParsers( + ["acme/web", "checkout"], + { + kind: "tuple", + parameters: [{ parse: parseOrgProjectTargetArg }, { parse: String }], + }, + {}, + ); + expect(target).toEqual(parseOrgProjectTargetArg("acme/web")); + expect(name).toBe("checkout"); + }); + + test("runs the array parser on every value", () => { + const parsed = applyPositionalParsers( + ["1", "2"], + { kind: "array", parameter: { parse: Number } }, + {}, + ); + expect(parsed).toEqual([1, 2]); + }); + + test("passes the context as `this`, like Stricli", () => { + const context = { name: "ctx" }; + const [seen] = applyPositionalParsers( + ["x"], + { + kind: "tuple", + parameters: [ + { + parse(this: unknown) { + return this; + }, + }, + ], + }, + context, + ); + expect(seen).toBe(context); + }); + + test("leaves values without a parameter definition unchanged", () => { + expect(applyPositionalParsers(["a"], undefined, {})).toEqual(["a"]); + expect( + applyPositionalParsers(["a", "b"], { kind: "tuple", parameters: [] }, {}), + ).toEqual(["a", "b"]); + }); +}); + +describe("buildInvoker: positional parsing", () => { + test("a command's target positional is parsed and validated", async () => { + // Without positional parsing, project view would get the bare string + // and silently fall back to auto-detection instead of rejecting it. + const invoke = buildInvoker({ token: "sntrys_test" }); + await expect( + invoke(["project", "view"], {}, ["@latest"]) as Promise, + ).rejects.toThrow("is an issue selector"); + }); +}); diff --git a/packages/cli/test/lib/target-arg.test.ts b/packages/cli/test/lib/target-arg.test.ts new file mode 100644 index 000000000..b4c5cbf85 --- /dev/null +++ b/packages/cli/test/lib/target-arg.test.ts @@ -0,0 +1,124 @@ +/** + * Target positionals that parse their own value (src/lib/target-arg.ts). + * + * Runs a test command through Stricli's `run()` so the positional parser, + * the command wrapper, and exit-code handling are exercised together. + */ + +import { buildApplication, type CommandContext, run } from "@stricli/core"; +import { describe, expect, test } from "vitest"; +import type { ParsedOrgProject } from "../../src/lib/arg-parsing.js"; +import { buildCommand } from "../../src/lib/command.js"; +import { + getCompletionTarget, + ORG_PROJECT_TARGET, +} from "../../src/lib/completion-target.js"; +import { EXIT, getExitCode, ValidationError } from "../../src/lib/errors.js"; +import { buildRouteMap } from "../../src/lib/route-map.js"; +import { + type OrgProjectTargetArg, + parsedOrgProject, + parseOrgProjectTargetArg, + rawPositional, +} from "../../src/lib/target-arg.js"; + +type TestContext = CommandContext & { + process: { exitCode?: number }; + stdout: { write: (s: string) => boolean }; +}; + +/** Run `test cmd ...args` with an org/project target positional. */ +async function runWithTarget(args: string[]): Promise<{ + parsed: ParsedOrgProject[]; + stderr: string; + exitCode: number | undefined; +}> { + const parsed: ParsedOrgProject[] = []; + const command = buildCommand< + Record, + [OrgProjectTargetArg?], + TestContext + >({ + docs: { brief: "Takes a target" }, + auth: false, + parameters: { + positional: { + kind: "tuple", + parameters: [ + { + placeholder: "org/project", + brief: "Target", + parse: parseOrgProjectTargetArg, + optional: true, + }, + ], + }, + }, + // oxlint-disable-next-line require-yield -- test command only records the parsed target + async *func(_flags, target) { + parsed.push(parsedOrgProject(target)); + }, + }); + const app = buildApplication( + buildRouteMap({ routes: { cmd: command }, docs: { brief: "Test" } }), + { name: "test", determineExitCode: getExitCode }, + ); + let stderr = ""; + const stdout = { write: () => true }; + const context = { + process: { + stdout, + stderr: { + write: (s: string) => { + stderr += s; + return true; + }, + }, + exitCode: undefined, + }, + stdout, + } as unknown as TestContext; + await run(app, ["cmd", ...args], context); + return { parsed, stderr, exitCode: context.process.exitCode }; +} + +describe("parseOrgProjectTargetArg", () => { + test("hands the command a parsed target", async () => { + const { parsed } = await runWithTarget(["acme/web"]); + expect(parsed).toEqual([{ type: "explicit", org: "acme", project: "web" }]); + }); + + test("an omitted target means auto-detect", async () => { + const { parsed } = await runWithTarget([]); + expect(parsed).toEqual([{ type: "auto-detect" }]); + }); + + test("invalid input fails inside the command with the CliError exit code", async () => { + const { parsed, stderr, exitCode } = await runWithTarget(["@latest"]); + expect(parsed).toEqual([]); + expect(exitCode).toBe(EXIT.VALIDATION); + expect(stderr).toContain("is an issue selector"); + // Not Stricli's argument-parse error, which would exit with -4. + expect(stderr).not.toContain("Failed to parse"); + }); + + test("parses lazily, on first access", () => { + const target = parseOrgProjectTargetArg("@latest"); + expect(target.raw).toBe("@latest"); + expect(() => target.parsed).toThrow(ValidationError); + }); + + test("keeps completing org/project slugs", () => { + expect(getCompletionTarget(parseOrgProjectTargetArg)).toBe( + ORG_PROJECT_TARGET, + ); + }); +}); + +describe("rawPositional", () => { + test("returns what the user typed for a target", () => { + expect(rawPositional(parseOrgProjectTargetArg("acme/"))).toBe("acme/"); + expect(rawPositional("plain")).toBe("plain"); + expect(rawPositional(undefined)).toBeUndefined(); + }); +}); From cb22cad2bfe1cc878e1975e5f8797319d2978817 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Thu, 22 Oct 2026 18:56:30 +0000 Subject: [PATCH 2/6] ref(cli): Hand commands parsed targets instead of a lazy wrapper Review on #1594 asked to drop the two-stage TargetArg parse inside func. parseOrgProjectPositional now returns the ParsedOrgProject directly and commands take it as their argument, defaulting an omitted optional target to AUTO_DETECT_TARGET. buildCommand wraps every positional parser (positional-parse.ts). A CliError from a parser comes back as a DeferredPositionalError value, and the command wrapper throws it before the auth guard. Bad input keeps its CliError exit code and output instead of Stricli's "Failed to parse" (exit -4). The wrapper also records the raw input of parsed values, which plural-alias interception, help recovery and telemetry read through rawPositional. Tests call commands through loadCommandFunc, a test helper that runs the command's own positional parsers via applyPositionalParsers, so they pass raw strings again. Refs #1564 --- packages/cli/AGENTS.md | 21 +-- packages/cli/src/commands/feedback/list.ts | 18 +-- packages/cli/src/commands/project/delete.ts | 14 +- packages/cli/src/commands/project/view.ts | 18 ++- packages/cli/src/commands/replay/list.ts | 22 ++- packages/cli/src/commands/trace/list.ts | 18 +-- packages/cli/src/lib/arg-parsing.ts | 20 +++ packages/cli/src/lib/command.ts | 13 +- packages/cli/src/lib/completion-target.ts | 2 +- packages/cli/src/lib/list-command.ts | 4 +- packages/cli/src/lib/positional-parse.ts | 128 ++++++++++++++++++ packages/cli/src/lib/sdk-invoke.ts | 18 +-- packages/cli/src/lib/target-arg.ts | 79 ----------- .../cli/test/commands/feedback/list.test.ts | 20 +-- .../cli/test/commands/project/delete.test.ts | 110 ++++++--------- .../test/commands/project/view.func.test.ts | 100 ++++---------- .../cli/test/commands/replay/list.test.ts | 26 ++-- packages/cli/test/commands/trace/list.test.ts | 66 ++++----- packages/cli/test/helpers.ts | 33 +++++ packages/cli/test/lib/help-positional.test.ts | 2 +- ...t-arg.test.ts => positional-parse.test.ts} | 67 ++++++--- packages/cli/test/lib/sdk-invoke.test.ts | 6 +- 22 files changed, 429 insertions(+), 376 deletions(-) create mode 100644 packages/cli/src/lib/positional-parse.ts delete mode 100644 packages/cli/src/lib/target-arg.ts rename packages/cli/test/lib/{target-arg.test.ts => positional-parse.test.ts} (65%) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index f773601b4..dcd4b7b89 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -240,14 +240,17 @@ const parsed = parseOrgProjectArg(targetArg); Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` **Parsed target positionals:** A single `[/][]` positional can -use `parseOrgProjectTargetArg` from `src/lib/target-arg.ts`. `func` then -receives a `TargetArg` and reads `parsedOrgProject(target)` instead of calling -`parseOrgProjectArg` itself. Parsing runs on first access inside `func`, so -invalid input still fails with the parser's `CliError` and exit code. Use -`target.raw` (or `rawPositional`) for what the user typed. Tests that call -`func` directly pass `parseOrgProjectTargetArg("org/project")`. The SDK invoke -path runs positional parsers too (`applyPositionalParsers`). Positionals whose -meaning depends on how many arguments were given, such as +use `parseOrgProjectPositional` from `src/lib/arg-parsing.ts`. `func` then +receives the `ParsedOrgProject` directly (default an optional positional to +`AUTO_DETECT_TARGET`) and never calls `parseOrgProjectArg` itself. +`buildCommand` wraps every positional parser (`src/lib/positional-parse.ts`) +so a `CliError` it throws is rethrown by the command wrapper before `func` +runs, keeping its exit code, telemetry, and `--json` output instead of +Stricli's generic argument error. Use `rawPositional(value)` for what the user +typed. Tests that call `func` directly load it with `loadCommandFunc(command)` +from `test/helpers.ts` and pass raw strings; it runs the command's own +positional parsers, as does the SDK invoke path (`applyPositionalParsers`). +Positionals whose meaning depends on how many arguments were given, such as `[//]`, keep parsing in `func`. **Shell completion:** Target positionals that take an org or project use a @@ -255,7 +258,7 @@ 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. `LIST_TARGET_POSITIONAL` and `LIST_ORG_TARGET_POSITIONAL` already use them. A parser tagged with -`withCompletionTarget`, such as `parseOrgProjectTargetArg`, completes the same +`withCompletionTarget`, such as `parseOrgProjectPositional`, completes the same way. `pnpm run generate:completions` collects these into the gitignored `src/generated/completion-targets.json` that the `__complete` fast path reads. diff --git a/packages/cli/src/commands/feedback/list.ts b/packages/cli/src/commands/feedback/list.ts index b6035dc06..83c23a3d6 100644 --- a/packages/cli/src/commands/feedback/list.ts +++ b/packages/cli/src/commands/feedback/list.ts @@ -10,7 +10,12 @@ import { getProject, listFeedback, } from "../../lib/api-client.js"; -import { validateLimit } from "../../lib/arg-parsing.js"; +import { + validateLimit, + AUTO_DETECT_TARGET, + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -38,11 +43,6 @@ import { toNumericId, } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; -import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, -} from "../../lib/target-arg.js"; import { appendPeriodHint, PERIOD_BRIEF, @@ -163,7 +163,7 @@ export const listCommand = buildListCommand("feedback", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, optional: true, }, ], @@ -203,10 +203,10 @@ export const listCommand = buildListCommand("feedback", { async *func( this: SentryContext, flags: ListFlags, - target?: OrgProjectTargetArg, + target: ParsedOrgProject = AUTO_DETECT_TARGET, ) { const resolved = await resolveOrgOptionalTarget( - parsedOrgProject(target), + target, this.cwd, COMMAND_NAME, ); diff --git a/packages/cli/src/commands/project/delete.ts b/packages/cli/src/commands/project/delete.ts index 514754c2e..975c5b237 100644 --- a/packages/cli/src/commands/project/delete.ts +++ b/packages/cli/src/commands/project/delete.ts @@ -19,6 +19,10 @@ */ import type { SentryContext } from "../../context.js"; +import { + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../lib/arg-parsing.js"; import { deleteProject, getOrganization, @@ -40,11 +44,6 @@ import { } from "../../lib/mutate-command.js"; import { resolveProjectBoundTarget } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; -import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, -} from "../../lib/target-arg.js"; const log = logger.withTag("project.delete"); @@ -172,7 +171,7 @@ export const deleteCommand = buildDeleteCommand({ { placeholder: "org/project", brief: "/ or (search across orgs)", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, }, ], }, @@ -180,12 +179,11 @@ export const deleteCommand = buildDeleteCommand({ async *func( this: SentryContext, flags: DeleteFlags, - target: OrgProjectTargetArg, + parsed: ParsedOrgProject, ) { const { cwd } = this; // Block auto-detect for safety — destructive commands require explicit targets - const parsed = parsedOrgProject(target); requireExplicitTarget( parsed, "Project target", diff --git a/packages/cli/src/commands/project/view.ts b/packages/cli/src/commands/project/view.ts index 7315e0fc5..a494a34f0 100644 --- a/packages/cli/src/commands/project/view.ts +++ b/packages/cli/src/commands/project/view.ts @@ -11,7 +11,12 @@ import { resolveOrgDisplayName, tryGetPrimaryDsn, } from "../../lib/api-client.js"; -import { ProjectSpecificationType } from "../../lib/arg-parsing.js"; +import { + ProjectSpecificationType, + AUTO_DETECT_TARGET, + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; import { AuthError, ContextError, withAuthGuard } from "../../lib/errors.js"; @@ -30,11 +35,6 @@ import { resolveProjectBoundSlug, } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; -import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, -} from "../../lib/target-arg.js"; import type { SentryProject } from "../../types/index.js"; type ViewFlags = { @@ -281,7 +281,7 @@ export const viewCommand = buildCommand({ { placeholder: "org/project", brief: "/, (search), or omit for auto-detect", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, optional: true, }, ], @@ -299,13 +299,11 @@ export const viewCommand = buildCommand({ async *func( this: SentryContext, flags: ViewFlags, - target?: OrgProjectTargetArg, + parsed: ParsedOrgProject = AUTO_DETECT_TARGET, ) { applyFreshFlag(flags); const { cwd } = this; - const parsed = parsedOrgProject(target); - let resolvedTargets: ResolvedTarget[]; let footer: string | undefined; diff --git a/packages/cli/src/commands/replay/list.ts b/packages/cli/src/commands/replay/list.ts index 6ee12dede..e21c18b30 100644 --- a/packages/cli/src/commands/replay/list.ts +++ b/packages/cli/src/commands/replay/list.ts @@ -10,7 +10,12 @@ import { listReplays, type ReplaySortValue, } from "../../lib/api-client.js"; -import { validateLimit } from "../../lib/arg-parsing.js"; +import { + validateLimit, + AUTO_DETECT_TARGET, + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -46,11 +51,6 @@ import { } from "../../lib/replay-search.js"; import { resolveOrgOptionalTarget } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; -import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, -} from "../../lib/target-arg.js"; import { appendPeriodHint, serializeTimeRange, @@ -285,7 +285,7 @@ export const listCommand = buildListCommand("replay", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, optional: true, }, ], @@ -330,18 +330,14 @@ export const listCommand = buildListCommand("replay", { async *func( this: SentryContext, flags: ListFlags, - target?: OrgProjectTargetArg, + target: ParsedOrgProject = AUTO_DETECT_TARGET, ) { const { cwd } = this; const timeRange = flags.period; const environment = parseReplayEnvironmentFilter(flags.environment); const { query } = flags; - const resolved = await resolveOrgOptionalTarget( - parsedOrgProject(target), - cwd, - COMMAND_NAME, - ); + const resolved = await resolveOrgOptionalTarget(target, cwd, COMMAND_NAME); const contextKey = buildPaginationContextKey( "replay", diff --git a/packages/cli/src/commands/trace/list.ts b/packages/cli/src/commands/trace/list.ts index b7a644682..612d597bd 100644 --- a/packages/cli/src/commands/trace/list.ts +++ b/packages/cli/src/commands/trace/list.ts @@ -6,7 +6,12 @@ import type { SentryContext } from "../../context.js"; import { listTransactions } from "../../lib/api-client.js"; -import { validateLimit } from "../../lib/arg-parsing.js"; +import { + validateLimit, + AUTO_DETECT_TARGET, + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -34,11 +39,6 @@ import { resolveProjectBoundTarget, } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; -import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, -} from "../../lib/target-arg.js"; import { appendPeriodHint, serializeTimeRange, @@ -229,7 +229,7 @@ export const listCommand = buildListCommand("trace", { { placeholder: "org/project", brief: "/ or (search)", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, optional: true, }, ], @@ -265,7 +265,7 @@ export const listCommand = buildListCommand("trace", { async *func( this: SentryContext, flags: ListFlags, - target?: OrgProjectTargetArg, + target: ParsedOrgProject = AUTO_DETECT_TARGET, ) { const { cwd } = this; const timeRange = flags.period; @@ -273,7 +273,7 @@ export const listCommand = buildListCommand("trace", { // Resolve org/project from positional arg, config, or DSN auto-detection const { org, project } = await resolveProjectBoundTarget( - parsedOrgProject(target), + target, cwd, COMMAND_NAME, ); diff --git a/packages/cli/src/lib/arg-parsing.ts b/packages/cli/src/lib/arg-parsing.ts index 0fa732f39..36eb78e17 100644 --- a/packages/cli/src/lib/arg-parsing.ts +++ b/packages/cli/src/lib/arg-parsing.ts @@ -7,6 +7,10 @@ */ import type { LogSortDirection } from "./api/logs.js"; +import { + ORG_PROJECT_TARGET, + withCompletionTarget, +} from "./completion-target.js"; import { ContextError, ValidationError, validationError } from "./errors.js"; import { validateResourceId } from "./input-validation.js"; @@ -961,6 +965,22 @@ export function parseOrgProjectArg( return parsed; } +/** What an omitted `[/][]` positional means. */ +export const AUTO_DETECT_TARGET: ParsedOrgProject = Object.freeze({ + type: ProjectSpecificationType.AutoDetect, +}); + +/** + * Positional parser for `[/][]` targets. The command receives + * the {@link parseOrgProjectArg} result and completes like + * `parseOrgProjectTarget`. Use {@link AUTO_DETECT_TARGET} as the default + * for an optional positional. + */ +export const parseOrgProjectPositional = withCompletionTarget( + ORG_PROJECT_TARGET, + (input: string): ParsedOrgProject => parseOrgProjectArg(input), +); + /** * Parsed issue argument types - flattened for ergonomics. * diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 36584bb84..1c29b9b4a 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -65,7 +65,11 @@ import { parseLogLevel, setLogLevel, } from "./logger.js"; -import { rawPositional } from "./target-arg.js"; +import { + deferPositionalParsers, + rawPositional, + throwDeferredPositionalError, +} from "./positional-parse.js"; import { setArgsContext, setFlagContext, withTracing } from "./telemetry.js"; import { applyTipsPreference } from "./tips.js"; @@ -557,10 +561,14 @@ export function buildCommand< } } + // Positional parsers run in Stricli's scanner, which would turn a thrown + // CliError into a generic argument error. Defer those errors to the + // wrapped func below so they surface like any other command error. const mergedParams = { ...existingParams, flags: mergedFlags, aliases: mergedAliases, + positional: deferPositionalParsers(existingParams.positional), }; /** @@ -789,6 +797,9 @@ export function buildCommand< // `sentry issue list help`). Without this, the auth prompt would fire // before the help-recovery path could show the command's help text. try { + // Invalid positional input fails before auth: it needs no account. + throwDeferredPositionalError(args); + // A usable stored account defers the check: the command's target may // select it even when the account in effect is logged out. if (requiresAuth && !getAuthConfig() && !hasUsableStoredAccount()) { diff --git a/packages/cli/src/lib/completion-target.ts b/packages/cli/src/lib/completion-target.ts index b13812eec..26245c7a7 100644 --- a/packages/cli/src/lib/completion-target.ts +++ b/packages/cli/src/lib/completion-target.ts @@ -9,7 +9,7 @@ * * {@link parseOrgTarget} and {@link parseOrgProjectTarget} return their input * unchanged and only mark the kind. Parsers that also parse the value, such - * as `parseOrgProjectTargetArg` in `target-arg.ts`, use the same tag. + * as `parseOrgProjectPositional` in `arg-parsing.ts`, use the same tag. */ /** Completes organization slugs only (e.g. `org`, `org/version`, `org/id`). */ diff --git a/packages/cli/src/lib/list-command.ts b/packages/cli/src/lib/list-command.ts index 87adec7ec..a8155091d 100644 --- a/packages/cli/src/lib/list-command.ts +++ b/packages/cli/src/lib/list-command.ts @@ -44,8 +44,8 @@ import { type ListResult, type OrgListConfig, } from "./org-list.js"; +import { rawPositional } from "./positional-parse.js"; import { disableResponseCache } from "./response-cache.js"; -import { rawPositional } from "./target-arg.js"; import { PERIOD_BRIEF, parsePeriod } from "./time-range.js"; import { shellQuote } from "./utils.js"; @@ -619,7 +619,7 @@ export function buildListCommand< applyFreshFlag(flags as unknown as { readonly fresh: boolean }); // The first positional arg is always the target (org/project pattern), - // either a raw string or a TargetArg. Intercept it to handle plural + // either a raw string or a parsed value. Intercept it to handle plural // alias confusion. const rawTarget = rawPositional(args[0]); if (typeof rawTarget === "string") { diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts new file mode 100644 index 000000000..5ed760f57 --- /dev/null +++ b/packages/cli/src/lib/positional-parse.ts @@ -0,0 +1,128 @@ +/** + * Positional parsers that throw CliErrors. + * + * Stricli runs positional parsers in its argument scanner and reports a + * thrown error as a generic "Failed to parse" argument error (exit -4), + * bypassing our CliError formatting, exit codes, and telemetry. + * `buildCommand` wraps every positional parser with + * {@link deferPositionalErrors}: a {@link CliError} becomes a + * {@link DeferredPositionalError} value, which the command wrapper throws + * before `func` runs, so the error surfaces like any other command error. + * + * The wrapper also remembers the raw input of object results, so code that + * needs what the user typed (plural alias interception, `help` recovery, + * telemetry) can read it with {@link rawPositional}. + */ + +import { + getCompletionTarget, + withCompletionTarget, +} from "./completion-target.js"; +import { CliError } from "./errors.js"; + +/** A positional whose parser threw a {@link CliError}. */ +export class DeferredPositionalError { + readonly raw: string; + readonly error: CliError; + + constructor(raw: string, error: CliError) { + this.raw = raw; + this.error = error; + } +} + +/** Raw input of each object a wrapped positional parser returned. */ +const rawInputs = new WeakMap(); + +/** A Stricli positional parser. */ +type PositionalParser = (this: unknown, input: string) => unknown; + +/** + * Wrap a positional parser so a {@link CliError} it throws is returned as a + * {@link DeferredPositionalError} instead. Other errors still propagate. + * Keeps the parser's completion target. + */ +export function deferPositionalErrors( + parse: PositionalParser, +): PositionalParser { + const wrapped = function (this: unknown, input: string): unknown { + let value: unknown; + try { + value = parse.call(this, input); + } catch (error) { + if (error instanceof CliError) { + return new DeferredPositionalError(input, error); + } + throw error; + } + if (typeof value === "object" && value !== null) { + rawInputs.set(value, input); + } + return value; + }; + const kind = getCompletionTarget(parse); + return kind ? withCompletionTarget(kind, wrapped) : wrapped; +} + +/** A Stricli positional definition — subset that carries the parsers. */ +export type PositionalDef = + | { kind: "tuple"; parameters: readonly { parse: PositionalParser }[] } + | { kind: "array"; parameter: { parse: PositionalParser } }; + +/** + * Wrap every parser of a command's positional definition with + * {@link deferPositionalErrors}. + * + * @returns A copy of `positional` with wrapped parsers, or `positional` + * itself when the command takes no positionals + */ +export function deferPositionalParsers

(positional: P): P { + const def = positional as PositionalDef | undefined; + if (!def) { + return positional; + } + if (def.kind === "array") { + return { + ...def, + parameter: { + ...def.parameter, + parse: deferPositionalErrors(def.parameter.parse), + }, + } as P; + } + return { + ...def, + parameters: def.parameters.map((param) => ({ + ...param, + parse: deferPositionalErrors(param.parse), + })), + } as P; +} + +/** + * Throw the first {@link DeferredPositionalError} among a command's + * positional values. + */ +export function throwDeferredPositionalError(args: readonly unknown[]): void { + for (const arg of args) { + if (arg instanceof DeferredPositionalError) { + throw arg.error; + } + } +} + +/** + * What the user typed for a positional value. + * + * @returns The raw input of a parsed object or deferred error, otherwise + * `value` itself + */ +export function rawPositional(value: unknown): unknown { + if (value instanceof DeferredPositionalError) { + return value.raw; + } + if (typeof value === "object" && value !== null) { + return rawInputs.get(value) ?? value; + } + return value; +} diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index 47ec012f4..82bdc5f05 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -19,6 +19,7 @@ import type { Span } from "@sentry/core"; import type { Writer } from "../types/index.js"; import { type AsyncChannel, createAsyncChannel } from "./async-channel.js"; import { withEnv } from "./env.js"; +import type { PositionalDef } from "./positional-parse.js"; import { SentryError, type SentryOptions } from "./sdk-types.js"; /** CLI flag names/aliases that trigger infinite streaming output. */ @@ -76,14 +77,6 @@ export type FlagDef = { variadic?: boolean; }; -/** One positional parameter — subset needed to run its parser. */ -type PositionalParamDef = { parse: (this: unknown, input: string) => unknown }; - -/** Stricli positional definition shape — subset needed by {@link applyPositionalParsers}. */ -type PositionalDef = - | { kind: "tuple"; parameters: readonly PositionalParamDef[] } - | { kind: "array"; parameter: PositionalParamDef }; - /** Resolved command: handler function + parameter definitions. */ type ResolvedCommand = { handler: CommandHandler; @@ -147,14 +140,15 @@ async function resolveCommand(path: string[]): Promise { * Like flag defaults ({@link applyFlagDefaults}), positional parsing normally * happens in Stricli's argument scanner, which the SDK invoke path bypasses. * Without this, a command whose positional parses its own value (e.g. - * `parseOrgProjectTargetArg`) would receive a bare string. + * `parseOrgProjectPositional`) would receive a bare string. * - * @param args - Positional strings from the SDK caller + * @param args - Positional strings from the caller; `undefined` marks an + * omitted optional positional and is passed through unparsed * @param positionalDef - The command's Stricli positional definition * @param context - `this` for each parser, as Stricli passes it */ export function applyPositionalParsers( - args: readonly string[], + args: readonly (string | undefined)[], positionalDef: PositionalDef | undefined, context: unknown, ): unknown[] { @@ -163,7 +157,7 @@ export function applyPositionalParsers( positionalDef?.kind === "array" ? positionalDef.parameter : positionalDef?.parameters[index]; - return param ? param.parse.call(context, arg) : arg; + return param && arg !== undefined ? param.parse.call(context, arg) : arg; }); } diff --git a/packages/cli/src/lib/target-arg.ts b/packages/cli/src/lib/target-arg.ts deleted file mode 100644 index b319c1c46..000000000 --- a/packages/cli/src/lib/target-arg.ts +++ /dev/null @@ -1,79 +0,0 @@ -/** - * Target positionals that parse their own value. - * - * A positional that uses one of these parsers reaches the command function - * as a {@link TargetArg} instead of a raw string, so the target syntax is - * defined once, next to the positional, rather than re-parsed in every - * command's `func`. - * - * Parsing is lazy: Stricli only wraps the input, and the parse runs on the - * first {@link TargetArg.parsed} access inside `func`. Invalid input therefore - * still throws the parser's `CliError` subclass from the command, with - * its exit code, telemetry, and `--json` error output, instead of Stricli's - * generic argument-parse error. Code that needs what the user typed (plural - * alias interception, `help` recovery, telemetry) reads {@link TargetArg.raw} - * through {@link rawPositional}. - */ - -import { type ParsedOrgProject, parseOrgProjectArg } from "./arg-parsing.js"; -import { - ORG_PROJECT_TARGET, - withCompletionTarget, -} from "./completion-target.js"; - -/** A scanned target positional: the raw input and its lazily parsed value. */ -export class TargetArg { - readonly raw: string; - readonly #parse: (raw: string) => T; - #parsed: { value: T } | undefined; - - constructor(raw: string, parse: (raw: string) => T) { - this.raw = raw; - this.#parse = parse; - } - - /** - * The parsed target. Parses on first access. - * - * @throws The parser's error (usually a `ValidationError`) for invalid input - */ - get parsed(): T { - this.#parsed ??= { value: this.#parse(this.raw) }; - return this.#parsed.value; - } -} - -/** An `[/][]` positional, parsed by `parseOrgProjectArg`. */ -export type OrgProjectTargetArg = TargetArg; - -/** - * Positional parser for `/`, `/`, ``, and Sentry - * URLs. Completes like `parseOrgProjectTarget`. - */ -export const parseOrgProjectTargetArg = withCompletionTarget( - ORG_PROJECT_TARGET, - (input: string): OrgProjectTargetArg => - new TargetArg(input, (raw) => parseOrgProjectArg(raw)), -); - -/** - * The parsed value of an `[/][]` positional. - * - * @returns Auto-detect when the positional was omitted, as - * `parseOrgProjectArg(undefined)` does - * @throws The parser's error for invalid input - */ -export function parsedOrgProject( - target: OrgProjectTargetArg | undefined, -): ParsedOrgProject { - return target === undefined ? { type: "auto-detect" } : target.parsed; -} - -/** - * What the user typed for a positional value. - * - * @returns {@link TargetArg.raw} for a target positional, otherwise `value` - */ -export function rawPositional(value: unknown): unknown { - return value instanceof TargetArg ? value.raw : value; -} diff --git a/packages/cli/test/commands/feedback/list.test.ts b/packages/cli/test/commands/feedback/list.test.ts index 87df0b8f0..196f4d1b3 100644 --- a/packages/cli/test/commands/feedback/list.test.ts +++ b/packages/cli/test/commands/feedback/list.test.ts @@ -5,7 +5,6 @@ import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { listCommand } from "../../../src/commands/feedback/list.js"; import { ApiError, ValidationError } from "../../../src/lib/errors.js"; -import { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import type { SentryFeedback } from "../../../src/types/index.js"; @@ -50,6 +49,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 { loadCommandFunc } from "../../helpers.js"; function sampleFeedback( overrides: Partial = {}, @@ -131,7 +131,7 @@ describe("feedback list", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -141,7 +141,7 @@ describe("feedback list", () => { json: true, fresh: false, }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); expect(listFeedbackSpy).toHaveBeenCalledWith("test-org", "test-project", { @@ -173,7 +173,7 @@ describe("feedback list", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -185,7 +185,7 @@ describe("feedback list", () => { json: false, fresh: false, }, - parseOrgProjectTargetArg("test-org/"), + "test-org/", ); expect(listFeedbackSpy).toHaveBeenCalledWith("test-org", "", { @@ -218,7 +218,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [sampleFeedback()] }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call(context, { status: "all", limit: 25, @@ -246,7 +246,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [sampleFeedback()] }); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -256,7 +256,7 @@ describe("feedback list", () => { json: true, fresh: false, }, - parseOrgProjectTargetArg("test-project"), + "test-project", ); expect(resolveTargetSpy).toHaveBeenCalledWith( @@ -277,7 +277,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call(context, { status: "unresolved", limit: 25, @@ -298,7 +298,7 @@ describe("feedback list", () => { ); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await expect( func.call(context, { status: "unresolved", diff --git a/packages/cli/test/commands/project/delete.test.ts b/packages/cli/test/commands/project/delete.test.ts index 4b39926f2..56591643a 100644 --- a/packages/cli/test/commands/project/delete.test.ts +++ b/packages/cli/test/commands/project/delete.test.ts @@ -112,8 +112,8 @@ 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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import type { SentryProject } from "../../../src/types/index.js"; +import { loadCommandFunc } from "../../helpers.js"; const sampleProject: SentryProject = { id: "999", @@ -178,12 +178,8 @@ describe("project delete", () => { test("deletes project with explicit org/project and --yes", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + const func = await loadCommandFunc(deleteCommand); + await func.call(context, defaultFlags, "acme-corp/my-app"); expect(getProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -196,12 +192,8 @@ describe("project delete", () => { test("delegates to resolveProjectBoundTarget for resolution", async () => { const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + const func = await loadCommandFunc(deleteCommand); + await func.call(context, defaultFlags, "acme-corp/my-app"); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -216,8 +208,8 @@ describe("project delete", () => { test("resolves bare slug via resolveProjectBoundTarget", async () => { const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call(context, defaultFlags, parseOrgProjectTargetArg("my-app")); + const func = await loadCommandFunc(deleteCommand); + await func.call(context, defaultFlags, "my-app"); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -232,15 +224,11 @@ describe("project delete", () => { test("errors in non-interactive mode without --yes", async () => { const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); // isatty(0) returns false in test environments (non-TTY) await expect( - func.call( - context, - { ...defaultFlags, yes: false }, - parseOrgProjectTargetArg("acme-corp/my-app"), - ), + func.call(context, { ...defaultFlags, yes: false }, "acme-corp/my-app"), ).rejects.toThrow("non-interactive mode"); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -252,14 +240,10 @@ describe("project delete", () => { ); const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await expect( - func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ), + func.call(context, defaultFlags, "acme-corp/my-app"), ).rejects.toThrow(ApiError); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -277,14 +261,10 @@ describe("project delete", () => { }); const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); try { - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + await func.call(context, defaultFlags, "acme-corp/my-app"); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -308,14 +288,10 @@ describe("project delete", () => { }); const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); try { - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + await func.call(context, defaultFlags, "acme-corp/my-app"); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -334,14 +310,10 @@ describe("project delete", () => { getOrganizationSpy.mockRejectedValue(new Error("network error")); const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); try { - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + await func.call(context, defaultFlags, "acme-corp/my-app"); expect.unreachable("should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ApiError); @@ -355,12 +327,8 @@ describe("project delete", () => { test("verifies project exists before attempting delete", async () => { const { context } = createMockContext(); - const func = await deleteCommand.loader(); - await func.call( - context, - defaultFlags, - parseOrgProjectTargetArg("acme-corp/my-app"), - ); + const func = await loadCommandFunc(deleteCommand); + await func.call(context, defaultFlags, "acme-corp/my-app"); // getProject must be called before deleteProject const getProjectOrder = getProjectSpy.mock.invocationCallOrder[0]; @@ -372,11 +340,11 @@ describe("project delete", () => { test("dry-run shows what would be deleted without calling deleteProject", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { ...defaultFlags, "dry-run": true }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(getProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -390,11 +358,11 @@ describe("project delete", () => { test("dry-run outputs JSON when --json is also set", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { ...defaultFlags, "dry-run": true, json: true }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -410,11 +378,11 @@ describe("project delete", () => { test("JSON output for actual deletion", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { ...defaultFlags, json: true }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -428,12 +396,12 @@ describe("project delete", () => { test("rejects auto-detect target", async () => { const { context } = createMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); // Empty string triggers auto-detect in parseOrgProjectArg - await expect( - func.call(context, defaultFlags, parseOrgProjectTargetArg("")), - ).rejects.toThrow(ContextError); + await expect(func.call(context, defaultFlags, "")).rejects.toThrow( + ContextError, + ); expect(deleteProjectSpy).not.toHaveBeenCalled(); }); @@ -494,11 +462,11 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("acme-corp/my-app"); const { context, stdoutWrite } = createPromptMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { yes: false, "dry-run": false }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(deleteProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -510,11 +478,11 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("wrong-org/wrong-project"); const { context } = createPromptMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { yes: false, "dry-run": false }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -526,11 +494,11 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue(Symbol("clack:cancel")); const { context } = createPromptMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { yes: false, "dry-run": false }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -541,11 +509,11 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue(""); const { context } = createPromptMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { yes: false, "dry-run": false }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(deleteProjectSpy).not.toHaveBeenCalled(); @@ -555,11 +523,11 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("acme-corp/my-app"); const { context } = createPromptMockContext(); - const func = await deleteCommand.loader(); + const func = await loadCommandFunc(deleteCommand); await func.call( context, { yes: false, "dry-run": false }, - parseOrgProjectTargetArg("acme-corp/my-app"), + "acme-corp/my-app", ); expect(mockPrompt).toHaveBeenCalledWith( diff --git a/packages/cli/test/commands/project/view.func.test.ts b/packages/cli/test/commands/project/view.func.test.ts index b718ea27c..e6699332c 100644 --- a/packages/cli/test/commands/project/view.func.test.ts +++ b/packages/cli/test/commands/project/view.func.test.ts @@ -51,8 +51,8 @@ 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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import type { ProjectKey, SentryProject } from "../../../src/types/sentry.js"; +import { loadCommandFunc } from "../../helpers.js"; const sampleProject: SentryProject = { id: "42", @@ -111,12 +111,8 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: true, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -133,11 +129,11 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await func.call( context, { json: false, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), + "my-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -149,12 +145,8 @@ describe("viewCommand.func", () => { openInBrowserSpy.mockResolvedValue(undefined); const { context } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: false, web: true }, - parseOrgProjectTargetArg("my-org/test-project"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: false, web: true }, "my-org/test-project"); expect(openInBrowserSpy).toHaveBeenCalled(); // Should NOT fetch project details when using --web @@ -180,7 +172,7 @@ describe("viewCommand.func", () => { }); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); try { // No target arg triggers AutoDetect @@ -203,12 +195,8 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: true, web: false }, - parseOrgProjectTargetArg("frontend"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: true, web: false }, "frontend"); expect(resolveProjectBoundSlugSpy).toHaveBeenCalledWith( "frontend", @@ -224,14 +212,10 @@ describe("viewCommand.func", () => { test("org-only target (org/) throws ContextError", async () => { const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); try { - await func.call( - context, - { json: false, web: false }, - parseOrgProjectTargetArg("my-org/"), - ); + await func.call(context, { json: false, web: false }, "my-org/"); expect.unreachable("Should have thrown"); } catch (error) { expect(error).toBeInstanceOf(ContextError); @@ -261,7 +245,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); // No target arg triggers AutoDetect await func.call(context, { json: false, web: false }); @@ -275,7 +259,7 @@ describe("viewCommand.func", () => { resolveAllTargetsSpy.mockResolvedValue({ targets: [] }); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await expect( func.call(context, { json: false, web: false }), @@ -289,7 +273,7 @@ describe("viewCommand.func", () => { }); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); try { await func.call(context, { json: false, web: false }); @@ -315,14 +299,10 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await expect( - func.call( - context, - { json: false, web: false }, - parseOrgProjectTargetArg("my-org/bad-project"), - ), + func.call(context, { json: false, web: false }, "my-org/bad-project"), ).rejects.toThrow("404 Not Found"); expect(getProjectSpy).toHaveBeenCalledWith("my-org", "bad-project"); @@ -340,14 +320,10 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await expect( - func.call( - context, - { json: false, web: false }, - parseOrgProjectTargetArg("frontend"), - ), + func.call(context, { json: false, web: false }, "frontend"), ).rejects.toThrow("403 Forbidden"); }); @@ -373,7 +349,7 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await expect( func.call(context, { json: false, web: false }), @@ -387,14 +363,10 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); await expect( - func.call( - context, - { json: false, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), - ), + func.call(context, { json: false, web: false }, "my-org/test-project"), ).rejects.toThrow(AuthError); }); @@ -411,12 +383,8 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: true, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -437,12 +405,8 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: true, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -456,12 +420,8 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); - await func.call( - context, - { json: true, web: false }, - parseOrgProjectTargetArg("my-org/test-project"), - ); + const func = await loadCommandFunc(viewCommand); + await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); const parsed = JSON.parse(output); @@ -478,7 +438,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await viewCommand.loader(); + const func = await loadCommandFunc(viewCommand); // `fields` is auto-injected by the buildCommand wrapper; pass through // the flag shape the wrapper would forward. await func.call( @@ -488,7 +448,7 @@ describe("viewCommand.func", () => { web: false, fields: ["slug", "organization.slug"], }, - parseOrgProjectTargetArg("my-org/test-project"), + "my-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); diff --git a/packages/cli/test/commands/replay/list.test.ts b/packages/cli/test/commands/replay/list.test.ts index 41371f20f..ec5980395 100644 --- a/packages/cli/test/commands/replay/list.test.ts +++ b/packages/cli/test/commands/replay/list.test.ts @@ -48,12 +48,12 @@ 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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import { REPLAY_LIST_FIELDS, type ReplayListItem, } from "../../../src/types/index.js"; +import { loadCommandFunc } from "../../helpers.js"; describe("parseSort", () => { test("accepts supported sort values", () => { @@ -136,11 +136,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 25, json: true, period: parsePeriod("7d"), sort: "-started_at" }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ); expect(listReplaysSpy).toHaveBeenCalledWith("test-org", { @@ -171,7 +171,7 @@ describe("listCommand.func", () => { ); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await expect( func.call( context, @@ -182,7 +182,7 @@ describe("listCommand.func", () => { sort: "-started_at", query: "bad:::query", }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ), ).rejects.toBeInstanceOf(ValidationError); }); @@ -194,7 +194,7 @@ describe("listCommand.func", () => { ); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await expect( func.call( context, @@ -204,7 +204,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ), ).rejects.toBeInstanceOf(ApiError); }); @@ -217,7 +217,7 @@ describe("listCommand.func", () => { }); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -227,7 +227,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ); expect(listReplaysSpy).toHaveBeenCalledWith("test-org", { @@ -247,7 +247,7 @@ describe("listCommand.func", () => { listReplaysSpy.mockResolvedValue({ data: sampleReplays }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -256,7 +256,7 @@ describe("listCommand.func", () => { period: parsePeriod("7d"), sort: "-started_at", }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ); const output = stdoutWrite.mock.calls.map((call) => call[0]).join(""); @@ -274,7 +274,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -283,7 +283,7 @@ describe("listCommand.func", () => { period: parsePeriod(LIST_PERIOD_FLAG.default), sort: "-started_at", }, - parseOrgProjectTargetArg("test-org/cli"), + "test-org/cli", ); const output = stdoutWrite.mock.calls.map((call) => call[0]).join(""); diff --git a/packages/cli/test/commands/trace/list.test.ts b/packages/cli/test/commands/trace/list.test.ts index ae9442eac..ed37c049b 100644 --- a/packages/cli/test/commands/trace/list.test.ts +++ b/packages/cli/test/commands/trace/list.test.ts @@ -57,9 +57,9 @@ 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 { parseOrgProjectTargetArg } from "../../../src/lib/target-arg.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import type { TransactionListItem } from "../../../src/types/sentry.js"; +import { loadCommandFunc } from "../../helpers.js"; // Reference paginationDb early to prevent import stripping by auto-organize const _paginationDbRef = paginationDb; @@ -330,11 +330,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -350,11 +350,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -369,11 +369,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -384,11 +384,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -406,11 +406,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 2, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -422,11 +422,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 100, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -438,11 +438,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [sampleTraces[0]] }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -454,7 +454,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -464,7 +464,7 @@ describe("listCommand.func", () => { query: "transaction:GET", period: parsePeriod("7d"), }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); expect(listTransactionsSpy).toHaveBeenCalledWith( @@ -488,11 +488,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -511,11 +511,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "duration", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -528,11 +528,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const callArgs = listTransactionsSpy.mock.calls[0]; @@ -546,11 +546,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -565,7 +565,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { @@ -575,7 +575,7 @@ describe("listCommand.func", () => { query: 'transaction:"POST /checkout"', period: parsePeriod("7d"), }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -587,11 +587,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -605,11 +605,11 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("24h") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); expect(listTransactionsSpy).toHaveBeenCalledWith( @@ -628,11 +628,11 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await listCommand.loader(); + const func = await loadCommandFunc(listCommand); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("30d") }, - parseOrgProjectTargetArg("test-org/test-project"), + "test-org/test-project", ); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); diff --git a/packages/cli/test/helpers.ts b/packages/cli/test/helpers.ts index 66fee55d2..c1d15424f 100644 --- a/packages/cli/test/helpers.ts +++ b/packages/cli/test/helpers.ts @@ -7,6 +7,7 @@ import { mkdirSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; +import type { Command, CommandContext } from "@stricli/core"; import { afterEach, beforeEach } from "vitest"; import { resetAccountSelection, @@ -16,6 +17,8 @@ import { resetIdentityFingerprintCache, } from "../src/lib/db/auth.js"; import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js"; +import type { PositionalDef } from "../src/lib/positional-parse.js"; +import { applyPositionalParsers } from "../src/lib/sdk-invoke.js"; import { resetTipsCache } from "../src/lib/tips.js"; // re-exporting a single constant, not a barrel @@ -347,3 +350,33 @@ export function installFetchMock( }, }; } + +/** A loaded command func that takes raw positional strings. */ +export type RawArgsCommandFunc = ( + this: CONTEXT, + flags: object, + ...args: (string | undefined)[] +) => Promise; + +/** + * Load a command's func for direct calls with raw CLI strings. + * + * Each positional runs through the command's own parser first, as Stricli's + * scanner does, so a parsing positional (e.g. `parseOrgProjectPositional`) + * reaches the command as its parsed value. Pass `undefined` for an omitted + * optional positional. + */ +export async function loadCommandFunc( + command: Command, +): Promise> { + const loaded = await command.loader(); + const func = typeof loaded === "function" ? loaded : loaded.default; + const positional = command.parameters.positional as PositionalDef | undefined; + return async function (this: CONTEXT, flags, ...args) { + await func.call( + this, + flags as never, + ...(applyPositionalParsers(args, positional, this) as never[]), + ); + }; +} diff --git a/packages/cli/test/lib/help-positional.test.ts b/packages/cli/test/lib/help-positional.test.ts index 380755ff4..ede51691c 100644 --- a/packages/cli/test/lib/help-positional.test.ts +++ b/packages/cli/test/lib/help-positional.test.ts @@ -144,7 +144,7 @@ describe("help recovery on ResolutionError", () => { }); test("sentry project view help → recovers through a parsed target positional", async () => { - // project view's positional reaches func as a TargetArg, not a string + // project view's positional reaches func parsed, not as a string const { stdout, stderr } = await runCommand(["project", "view", "help"]); expect(stdout).toContain("sentry project view"); diff --git a/packages/cli/test/lib/target-arg.test.ts b/packages/cli/test/lib/positional-parse.test.ts similarity index 65% rename from packages/cli/test/lib/target-arg.test.ts rename to packages/cli/test/lib/positional-parse.test.ts index b4c5cbf85..a6d2312b3 100644 --- a/packages/cli/test/lib/target-arg.test.ts +++ b/packages/cli/test/lib/positional-parse.test.ts @@ -1,5 +1,5 @@ /** - * Target positionals that parse their own value (src/lib/target-arg.ts). + * Positional parsers that throw CliErrors (src/lib/positional-parse.ts). * * Runs a test command through Stricli's `run()` so the positional parser, * the command wrapper, and exit-code handling are exercised together. @@ -7,20 +7,23 @@ import { buildApplication, type CommandContext, run } from "@stricli/core"; import { describe, expect, test } from "vitest"; -import type { ParsedOrgProject } from "../../src/lib/arg-parsing.js"; +import { + AUTO_DETECT_TARGET, + type ParsedOrgProject, + parseOrgProjectPositional, +} from "../../src/lib/arg-parsing.js"; import { buildCommand } from "../../src/lib/command.js"; import { getCompletionTarget, ORG_PROJECT_TARGET, } from "../../src/lib/completion-target.js"; import { EXIT, getExitCode, ValidationError } from "../../src/lib/errors.js"; -import { buildRouteMap } from "../../src/lib/route-map.js"; import { - type OrgProjectTargetArg, - parsedOrgProject, - parseOrgProjectTargetArg, + DeferredPositionalError, + deferPositionalErrors, rawPositional, -} from "../../src/lib/target-arg.js"; +} from "../../src/lib/positional-parse.js"; +import { buildRouteMap } from "../../src/lib/route-map.js"; type TestContext = CommandContext & { process: { exitCode?: number }; @@ -36,7 +39,7 @@ async function runWithTarget(args: string[]): Promise<{ const parsed: ParsedOrgProject[] = []; const command = buildCommand< Record, - [OrgProjectTargetArg?], + [ParsedOrgProject?], TestContext >({ docs: { brief: "Takes a target" }, @@ -48,15 +51,15 @@ async function runWithTarget(args: string[]): Promise<{ { placeholder: "org/project", brief: "Target", - parse: parseOrgProjectTargetArg, + parse: parseOrgProjectPositional, optional: true, }, ], }, }, // oxlint-disable-next-line require-yield -- test command only records the parsed target - async *func(_flags, target) { - parsed.push(parsedOrgProject(target)); + async *func(_flags, target = AUTO_DETECT_TARGET) { + parsed.push(target); }, }); const app = buildApplication( @@ -82,7 +85,7 @@ async function runWithTarget(args: string[]): Promise<{ return { parsed, stderr, exitCode: context.process.exitCode }; } -describe("parseOrgProjectTargetArg", () => { +describe("parseOrgProjectPositional", () => { test("hands the command a parsed target", async () => { const { parsed } = await runWithTarget(["acme/web"]); expect(parsed).toEqual([{ type: "explicit", org: "acme", project: "web" }]); @@ -93,7 +96,7 @@ describe("parseOrgProjectTargetArg", () => { expect(parsed).toEqual([{ type: "auto-detect" }]); }); - test("invalid input fails inside the command with the CliError exit code", async () => { + test("invalid input fails with the CliError exit code, not Stricli's", async () => { const { parsed, stderr, exitCode } = await runWithTarget(["@latest"]); expect(parsed).toEqual([]); expect(exitCode).toBe(EXIT.VALIDATION); @@ -102,22 +105,42 @@ describe("parseOrgProjectTargetArg", () => { expect(stderr).not.toContain("Failed to parse"); }); - test("parses lazily, on first access", () => { - const target = parseOrgProjectTargetArg("@latest"); - expect(target.raw).toBe("@latest"); - expect(() => target.parsed).toThrow(ValidationError); - }); - test("keeps completing org/project slugs", () => { - expect(getCompletionTarget(parseOrgProjectTargetArg)).toBe( + expect(getCompletionTarget(parseOrgProjectPositional)).toBe( ORG_PROJECT_TARGET, ); }); }); +describe("deferPositionalErrors", () => { + const parse = deferPositionalErrors(parseOrgProjectPositional); + + test("returns a CliError as a deferred value", () => { + const value = parse("@latest"); + expect(value).toBeInstanceOf(DeferredPositionalError); + expect((value as DeferredPositionalError).error).toBeInstanceOf( + ValidationError, + ); + }); + + test("lets other errors propagate", () => { + const failing = deferPositionalErrors(() => { + throw new TypeError("boom"); + }); + expect(() => failing("x")).toThrow(TypeError); + }); + + test("keeps the completion target", () => { + expect(getCompletionTarget(parse)).toBe(ORG_PROJECT_TARGET); + }); +}); + describe("rawPositional", () => { - test("returns what the user typed for a target", () => { - expect(rawPositional(parseOrgProjectTargetArg("acme/"))).toBe("acme/"); + const parse = deferPositionalErrors(parseOrgProjectPositional); + + test("returns what the user typed", () => { + expect(rawPositional(parse("acme/"))).toBe("acme/"); + expect(rawPositional(parse("@latest"))).toBe("@latest"); expect(rawPositional("plain")).toBe("plain"); expect(rawPositional(undefined)).toBeUndefined(); }); diff --git a/packages/cli/test/lib/sdk-invoke.test.ts b/packages/cli/test/lib/sdk-invoke.test.ts index d25297bf5..17d36eaa4 100644 --- a/packages/cli/test/lib/sdk-invoke.test.ts +++ b/packages/cli/test/lib/sdk-invoke.test.ts @@ -14,7 +14,7 @@ import { type FlagDef, parseOutput, } from "../../src/lib/sdk-invoke.js"; -import { parseOrgProjectTargetArg } from "../../src/lib/target-arg.js"; +import { parseOrgProjectPositional } from "../../src/lib/arg-parsing.js"; import { useTestConfigDir } from "../helpers.js"; useTestConfigDir("sdk-invoke-"); @@ -240,11 +240,11 @@ describe("applyPositionalParsers", () => { ["acme/web", "checkout"], { kind: "tuple", - parameters: [{ parse: parseOrgProjectTargetArg }, { parse: String }], + parameters: [{ parse: parseOrgProjectPositional }, { parse: String }], }, {}, ); - expect(target).toEqual(parseOrgProjectTargetArg("acme/web")); + expect(target).toEqual(parseOrgProjectPositional("acme/web")); expect(name).toBe("checkout"); }); From bd557cfb4c4877124a84186cda87cf452591e063 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 13:39:06 +0000 Subject: [PATCH 3/6] ref(cli): Parse target positionals lazily behind the parsed type Review on #1594 asked for lazy parsing that stays transparent to commands, tests, and the SDK. parseOrgProjectPositional now hands func a stand-in typed as ParsedOrgProject that runs parseOrgProjectArg on first property access. Commands read it like the parsed value, but the parse (and its URL-context side effect) runs inside func after the auth guard and rc URL check, as on main. Bad input throws its CliError from func, so there is nothing to defer out of Stricli's scanner. buildCommand runs these lazy parsers on positionals that arrive as plain strings, so tests calling command.loader() and the SDK invoke path pass raw strings again. That drops DeferredPositionalError, loadCommandFunc, and the SDK's applyPositionalParsers. Refs #1564 Co-Authored-By: BYK --- packages/cli/AGENTS.md | 21 +- packages/cli/src/lib/arg-parsing.ts | 17 +- packages/cli/src/lib/command.ts | 22 +-- packages/cli/src/lib/positional-parse.ts | 184 +++++++++--------- packages/cli/src/lib/sdk-invoke.ts | 54 +---- .../cli/test/commands/feedback/list.test.ts | 13 +- .../cli/test/commands/project/delete.test.ts | 37 ++-- .../test/commands/project/view.func.test.ts | 35 ++-- .../cli/test/commands/replay/list.test.ts | 13 +- packages/cli/test/commands/trace/list.test.ts | 33 ++-- packages/cli/test/helpers.ts | 33 ---- .../cli/test/lib/positional-parse.test.ts | 52 +++-- packages/cli/test/lib/sdk-invoke.test.ts | 67 +------ 13 files changed, 218 insertions(+), 363 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index dcd4b7b89..9d8efdc71 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -241,17 +241,16 @@ Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` **Parsed target positionals:** A single `[/][]` positional can use `parseOrgProjectPositional` from `src/lib/arg-parsing.ts`. `func` then -receives the `ParsedOrgProject` directly (default an optional positional to -`AUTO_DETECT_TARGET`) and never calls `parseOrgProjectArg` itself. -`buildCommand` wraps every positional parser (`src/lib/positional-parse.ts`) -so a `CliError` it throws is rethrown by the command wrapper before `func` -runs, keeping its exit code, telemetry, and `--json` output instead of -Stricli's generic argument error. Use `rawPositional(value)` for what the user -typed. Tests that call `func` directly load it with `loadCommandFunc(command)` -from `test/helpers.ts` and pass raw strings; it runs the command's own -positional parsers, as does the SDK invoke path (`applyPositionalParsers`). -Positionals whose meaning depends on how many arguments were given, such as -`[//]`, keep parsing in `func`. +takes a `ParsedOrgProject` (default an optional positional to +`AUTO_DETECT_TARGET`) and never calls `parseOrgProjectArg` itself. The value +parses on first property access (`lazyPositional` in +`src/lib/positional-parse.ts`), so invalid input still throws the parser's +`CliError` from inside `func`, after the auth guard, with its exit code. Use +`rawPositional(value)` for what the user typed; it never parses. +`buildCommand` runs these parsers on positionals that arrive as strings, so the +SDK invoke path and tests that call `command.loader()` directly keep passing +raw strings. Positionals whose meaning depends on how many arguments were +given, such as `[//]`, keep parsing in `func`. **Shell completion:** Target positionals that take an org or project use a completion-aware parser from `src/lib/completion-target.ts` instead of diff --git a/packages/cli/src/lib/arg-parsing.ts b/packages/cli/src/lib/arg-parsing.ts index 36eb78e17..a551e532d 100644 --- a/packages/cli/src/lib/arg-parsing.ts +++ b/packages/cli/src/lib/arg-parsing.ts @@ -7,12 +7,10 @@ */ import type { LogSortDirection } from "./api/logs.js"; -import { - ORG_PROJECT_TARGET, - withCompletionTarget, -} from "./completion-target.js"; +import { ORG_PROJECT_TARGET } from "./completion-target.js"; import { ContextError, ValidationError, validationError } from "./errors.js"; import { validateResourceId } from "./input-validation.js"; +import { lazyPositional } from "./positional-parse.js"; import type { ParsedSentryUrl } from "./sentry-url-parser.js"; import { applySentryUrlContext, parseSentryUrl } from "./sentry-url-parser.js"; @@ -972,13 +970,14 @@ export const AUTO_DETECT_TARGET: ParsedOrgProject = Object.freeze({ /** * Positional parser for `[/][]` targets. The command receives - * the {@link parseOrgProjectArg} result and completes like - * `parseOrgProjectTarget`. Use {@link AUTO_DETECT_TARGET} as the default - * for an optional positional. + * a {@link ParsedOrgProject} that runs {@link parseOrgProjectArg} on first + * property access (see `lazyPositional`), and the positional completes like + * `parseOrgProjectTarget`. Use {@link AUTO_DETECT_TARGET} as the default for + * an optional positional. */ -export const parseOrgProjectPositional = withCompletionTarget( +export const parseOrgProjectPositional = lazyPositional( ORG_PROJECT_TARGET, - (input: string): ParsedOrgProject => parseOrgProjectArg(input), + (raw): ParsedOrgProject => parseOrgProjectArg(raw), ); /** diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 1c29b9b4a..7668306d8 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -65,11 +65,7 @@ import { parseLogLevel, setLogLevel, } from "./logger.js"; -import { - deferPositionalParsers, - rawPositional, - throwDeferredPositionalError, -} from "./positional-parse.js"; +import { applyLazyPositionals, rawPositional } from "./positional-parse.js"; import { setArgsContext, setFlagContext, withTracing } from "./telemetry.js"; import { applyTipsPreference } from "./tips.js"; @@ -561,14 +557,10 @@ export function buildCommand< } } - // Positional parsers run in Stricli's scanner, which would turn a thrown - // CliError into a generic argument error. Defer those errors to the - // wrapped func below so they surface like any other command error. const mergedParams = { ...existingParams, flags: mergedFlags, aliases: mergedAliases, - positional: deferPositionalParsers(existingParams.positional), }; /** @@ -715,8 +707,15 @@ export function buildCommand< const wrappedFunc = async function ( this: CONTEXT, flags: Record, - ...args: unknown[] + ...positionalArgs: unknown[] ) { + // Callers that skip Stricli's scanner (SDK invoke, tests) pass raw + // strings; give lazily parsed positionals the value Stricli would. + const args = applyLazyPositionals( + positionalArgs, + existingParams.positional, + ); + applyLoggingFlags( flags[LOG_LEVEL_KEY] as LogLevelName | undefined, flags.verbose as boolean, @@ -797,9 +796,6 @@ export function buildCommand< // `sentry issue list help`). Without this, the auth prompt would fire // before the help-recovery path could show the command's help text. try { - // Invalid positional input fails before auth: it needs no account. - throwDeferredPositionalError(args); - // A usable stored account defers the check: the command's target may // select it even when the account in effect is logged out. if (requiresAuth && !getAuthConfig() && !hasUsableStoredAccount()) { diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts index 5ed760f57..fe8c3e7f8 100644 --- a/packages/cli/src/lib/positional-parse.ts +++ b/packages/cli/src/lib/positional-parse.ts @@ -1,126 +1,126 @@ /** - * Positional parsers that throw CliErrors. + * Positionals whose value parses lazily. * - * Stricli runs positional parsers in its argument scanner and reports a - * thrown error as a generic "Failed to parse" argument error (exit -4), - * bypassing our CliError formatting, exit codes, and telemetry. - * `buildCommand` wraps every positional parser with - * {@link deferPositionalErrors}: a {@link CliError} becomes a - * {@link DeferredPositionalError} value, which the command wrapper throws - * before `func` runs, so the error surfaces like any other command error. + * A parser made with {@link lazyPositional} gives the command a stand-in + * for the parsed value. The stand-in has the parsed value's type, so `func` + * reads it like any other value (`target.type`, `target.org`), but the parse + * only runs on the first property access inside `func`. Until then nothing + * can fail and nothing runs early: * - * The wrapper also remembers the raw input of object results, so code that - * needs what the user typed (plural alias interception, `help` recovery, - * telemetry) can read it with {@link rawPositional}. + * - Stricli's argument scanner never sees an error, so invalid input still + * throws the parser's `CliError` from the command, with its exit code, + * telemetry, and `--json` error output, instead of Stricli's generic + * "Failed to parse" (exit -4). + * - Parser side effects, such as `parseOrgProjectArg` applying the host of a + * pasted Sentry URL, run after the auth guard and rc URL check, as they did + * when `func` called the parser itself. + * + * `buildCommand` also runs these parsers on positionals that reach the + * command as plain strings (the SDK invoke path and tests calling the loaded + * func), so every caller can keep passing what the user typed. + * + * Code that needs the raw input (plural alias interception, `help` recovery, + * telemetry) reads it with {@link rawPositional}, which never parses. */ import { - getCompletionTarget, + type CompletionTarget, withCompletionTarget, } from "./completion-target.js"; -import { CliError } from "./errors.js"; - -/** A positional whose parser threw a {@link CliError}. */ -export class DeferredPositionalError { - readonly raw: string; - readonly error: CliError; - - constructor(raw: string, error: CliError) { - this.raw = raw; - this.error = error; - } -} -/** Raw input of each object a wrapped positional parser returned. */ +/** Raw input behind each stand-in. */ const rawInputs = new WeakMap(); -/** A Stricli positional parser. */ -type PositionalParser = (this: unknown, input: string) => unknown; +/** Parsers made by {@link lazyPositional}. */ +const lazyParsers = new WeakSet(); /** - * Wrap a positional parser so a {@link CliError} it throws is returned as a - * {@link DeferredPositionalError} instead. Other errors still propagate. - * Keeps the parser's completion target. + * A stand-in for `parse(raw)` that parses on first property access. + * + * `then` reads as `undefined` without parsing: Stricli awaits parser + * results, and a promise checks whether a resolved value is thenable. */ -export function deferPositionalErrors( - parse: PositionalParser, -): PositionalParser { - const wrapped = function (this: unknown, input: string): unknown { - let value: unknown; - try { - value = parse.call(this, input); - } catch (error) { - if (error instanceof CliError) { - return new DeferredPositionalError(input, error); - } - throw error; - } - if (typeof value === "object" && value !== null) { - rawInputs.set(value, input); - } - return value; +function lazyValue( + raw: string, + parse: (raw: string) => T, +): T { + let parsed: { value: T } | undefined; + const value = (): T => { + parsed ??= { value: parse(raw) }; + return parsed.value; }; - const kind = getCompletionTarget(parse); - return kind ? withCompletionTarget(kind, wrapped) : wrapped; + const standIn = new Proxy({} as T, { + get: (_target, key) => + key === "then" ? undefined : Reflect.get(value(), key), + set: (_target, key, newValue) => Reflect.set(value(), key, newValue), + has: (_target, key) => Reflect.has(value(), key), + ownKeys: () => Reflect.ownKeys(value()), + getOwnPropertyDescriptor: (_target, key) => { + const descriptor = Reflect.getOwnPropertyDescriptor(value(), key); + // The empty proxy target lacks the key, so it must report configurable + return descriptor && { ...descriptor, configurable: true }; + }, + }); + rawInputs.set(standIn, raw); + return standIn; } -/** A Stricli positional definition — subset that carries the parsers. */ -export type PositionalDef = - | { kind: "tuple"; parameters: readonly { parse: PositionalParser }[] } - | { kind: "array"; parameter: { parse: PositionalParser } }; - /** - * Wrap every parser of a command's positional definition with - * {@link deferPositionalErrors}. + * Build a positional parser that hands the command a lazily parsed value. * - * @returns A copy of `positional` with wrapped parsers, or `positional` - * itself when the command takes no positionals + * @param kind - Shell completion kind for the positional + * @param parse - Parses the raw input; may throw a `CliError` */ -export function deferPositionalParsers

(positional: P): P { - const def = positional as PositionalDef | undefined; - if (!def) { - return positional; - } - if (def.kind === "array") { - return { - ...def, - parameter: { - ...def.parameter, - parse: deferPositionalErrors(def.parameter.parse), - }, - } as P; - } - return { - ...def, - parameters: def.parameters.map((param) => ({ - ...param, - parse: deferPositionalErrors(param.parse), - })), - } as P; +export function lazyPositional( + kind: CompletionTarget, + parse: (raw: string) => T, +): (input: string) => T { + const parser = withCompletionTarget(kind, (input: string) => + lazyValue(input, parse), + ); + lazyParsers.add(parser); + return parser; } +/** A Stricli positional definition — the subset that carries the parsers. */ +type PositionalDef = + | { kind: "tuple"; parameters: readonly { parse?: unknown }[] } + | { kind: "array"; parameter: { parse?: unknown } }; + /** - * Throw the first {@link DeferredPositionalError} among a command's - * positional values. + * Run {@link lazyPositional} parsers on positionals that are still strings. + * + * Stricli already ran them; callers that skip its scanner (the SDK invoke + * path, tests calling the loaded func) pass raw strings. Lazy parsers never + * throw and have no side effects, so this is safe to run up front. Other + * positionals are left as they are. */ -export function throwDeferredPositionalError(args: readonly unknown[]): void { - for (const arg of args) { - if (arg instanceof DeferredPositionalError) { - throw arg.error; - } +export function applyLazyPositionals( + args: unknown[], + positional: unknown, +): unknown[] { + const def = positional as PositionalDef | undefined; + if (!def) { + return args; } + return args.map((arg, index) => { + const parse = + def.kind === "array" ? def.parameter.parse : def.parameters[index]?.parse; + if (typeof arg !== "string" || typeof parse !== "function") { + return arg; + } + return lazyParsers.has(parse) + ? (parse as (input: string) => unknown)(arg) + : arg; + }); } /** - * What the user typed for a positional value. + * What the user typed for a positional value. Never triggers a parse. * - * @returns The raw input of a parsed object or deferred error, otherwise - * `value` itself + * @returns The raw input behind a lazily parsed value, otherwise `value` */ export function rawPositional(value: unknown): unknown { - if (value instanceof DeferredPositionalError) { - return value.raw; - } if (typeof value === "object" && value !== null) { return rawInputs.get(value) ?? value; } diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index cfe681f3c..bd44c4356 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -20,7 +20,6 @@ import type { Writer } from "../types/index.js"; import { type AsyncChannel, createAsyncChannel } from "./async-channel.js"; import { withEnv } from "./env.js"; import { stringifyUnknown } from "./errors.js"; -import type { PositionalDef } from "./positional-parse.js"; import { SentryError, type SentryOptions } from "./sdk-types.js"; /** CLI flag names/aliases that trigger infinite streaming output. */ @@ -79,21 +78,16 @@ export type FlagDef = { variadic?: boolean | string; }; -/** Resolved command: handler function + parameter definitions. */ +/** Resolved command: handler function + flag definitions for default application. */ type ResolvedCommand = { handler: CommandHandler; flagDefs: Record; - positionalDef?: PositionalDef; }; /** Cached command entries, keyed by joined path (e.g., "org.list"). */ const commandCache = new Map< string, - { - loader: () => Promise; - flagDefs: Record; - positionalDef?: PositionalDef; - } + { loader: () => Promise; flagDefs: Record } >(); /** @@ -125,42 +119,10 @@ async function resolveCommand(path: string[]): Promise { (m: any) => (typeof m === "function" ? m : m.default), ), flagDefs, - positionalDef: command.parameters?.positional, }; commandCache.set(key, cached); } - return { - handler: await cached.loader(), - flagDefs: cached.flagDefs, - positionalDef: cached.positionalDef, - }; -} - -/** - * Run a command's positional parsers on SDK-provided strings. - * - * Like flag defaults ({@link applyFlagDefaults}), positional parsing normally - * happens in Stricli's argument scanner, which the SDK invoke path bypasses. - * Without this, a command whose positional parses its own value (e.g. - * `parseOrgProjectPositional`) would receive a bare string. - * - * @param args - Positional strings from the caller; `undefined` marks an - * omitted optional positional and is passed through unparsed - * @param positionalDef - The command's Stricli positional definition - * @param context - `this` for each parser, as Stricli passes it - */ -export function applyPositionalParsers( - args: readonly (string | undefined)[], - positionalDef: PositionalDef | undefined, - context: unknown, -): unknown[] { - return args.map((arg, index) => { - const param = - positionalDef?.kind === "array" - ? positionalDef.parameter - : positionalDef?.parameters[index]; - return param && arg !== undefined ? param.parse.call(context, arg) : arg; - }); + return { handler: await cached.loader(), flagDefs: cached.flagDefs }; } /** @@ -604,8 +566,7 @@ export function buildInvoker(options?: SentryOptions) { ): Promise | AsyncIterable { if (meta?.streaming) { return executeWithStream(options, async (ctx, span) => { - const { handler, flagDefs, positionalDef } = - await resolveCommand(commandPath); + const { handler, flagDefs } = await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); @@ -614,14 +575,13 @@ export function buildInvoker(options?: SentryOptions) { await handler.call( ctx.context, { ...resolvedFlags, json: true }, - ...applyPositionalParsers(positionalArgs, positionalDef, ctx.context), + ...positionalArgs, ); }); } return executeWithCapture(options, async (ctx, span) => { - const { handler, flagDefs, positionalDef } = - await resolveCommand(commandPath); + const { handler, flagDefs } = await resolveCommand(commandPath); if (span) { const { setCommandSpanName } = await import("./telemetry.js"); setCommandSpanName(span, commandPath.join(".")); @@ -630,7 +590,7 @@ export function buildInvoker(options?: SentryOptions) { await handler.call( ctx.context, { ...resolvedFlags, json: true }, - ...applyPositionalParsers(positionalArgs, positionalDef, ctx.context), + ...positionalArgs, ); }); }; diff --git a/packages/cli/test/commands/feedback/list.test.ts b/packages/cli/test/commands/feedback/list.test.ts index 196f4d1b3..8ddce9a87 100644 --- a/packages/cli/test/commands/feedback/list.test.ts +++ b/packages/cli/test/commands/feedback/list.test.ts @@ -49,7 +49,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"; -import { loadCommandFunc } from "../../helpers.js"; function sampleFeedback( overrides: Partial = {}, @@ -131,7 +130,7 @@ describe("feedback list", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -173,7 +172,7 @@ describe("feedback list", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -218,7 +217,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [sampleFeedback()] }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call(context, { status: "all", limit: 25, @@ -246,7 +245,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [sampleFeedback()] }); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -277,7 +276,7 @@ describe("feedback list", () => { listFeedbackSpy.mockResolvedValue({ feedback: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call(context, { status: "unresolved", limit: 25, @@ -298,7 +297,7 @@ describe("feedback list", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await expect( func.call(context, { status: "unresolved", diff --git a/packages/cli/test/commands/project/delete.test.ts b/packages/cli/test/commands/project/delete.test.ts index 56591643a..dcd839625 100644 --- a/packages/cli/test/commands/project/delete.test.ts +++ b/packages/cli/test/commands/project/delete.test.ts @@ -113,7 +113,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"; import type { SentryProject } from "../../../src/types/index.js"; -import { loadCommandFunc } from "../../helpers.js"; const sampleProject: SentryProject = { id: "999", @@ -178,7 +177,7 @@ describe("project delete", () => { test("deletes project with explicit org/project and --yes", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call(context, defaultFlags, "acme-corp/my-app"); expect(getProjectSpy).toHaveBeenCalledWith("acme-corp", "my-app"); @@ -192,7 +191,7 @@ describe("project delete", () => { test("delegates to resolveProjectBoundTarget for resolution", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call(context, defaultFlags, "acme-corp/my-app"); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( @@ -208,7 +207,7 @@ describe("project delete", () => { test("resolves bare slug via resolveProjectBoundTarget", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call(context, defaultFlags, "my-app"); expect(resolveProjectBoundTargetSpy).toHaveBeenCalledWith( @@ -224,7 +223,7 @@ describe("project delete", () => { test("errors in non-interactive mode without --yes", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); // isatty(0) returns false in test environments (non-TTY) await expect( @@ -240,7 +239,7 @@ describe("project delete", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await expect( func.call(context, defaultFlags, "acme-corp/my-app"), @@ -261,7 +260,7 @@ describe("project delete", () => { }); const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); try { await func.call(context, defaultFlags, "acme-corp/my-app"); @@ -288,7 +287,7 @@ describe("project delete", () => { }); const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); try { await func.call(context, defaultFlags, "acme-corp/my-app"); @@ -310,7 +309,7 @@ describe("project delete", () => { getOrganizationSpy.mockRejectedValue(new Error("network error")); const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); try { await func.call(context, defaultFlags, "acme-corp/my-app"); @@ -327,7 +326,7 @@ describe("project delete", () => { test("verifies project exists before attempting delete", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call(context, defaultFlags, "acme-corp/my-app"); // getProject must be called before deleteProject @@ -340,7 +339,7 @@ describe("project delete", () => { test("dry-run shows what would be deleted without calling deleteProject", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { ...defaultFlags, "dry-run": true }, @@ -358,7 +357,7 @@ describe("project delete", () => { test("dry-run outputs JSON when --json is also set", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { ...defaultFlags, "dry-run": true, json: true }, @@ -378,7 +377,7 @@ describe("project delete", () => { test("JSON output for actual deletion", async () => { const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { ...defaultFlags, json: true }, @@ -396,7 +395,7 @@ describe("project delete", () => { test("rejects auto-detect target", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); // Empty string triggers auto-detect in parseOrgProjectArg await expect(func.call(context, defaultFlags, "")).rejects.toThrow( @@ -462,7 +461,7 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("acme-corp/my-app"); const { context, stdoutWrite } = createPromptMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { yes: false, "dry-run": false }, @@ -478,7 +477,7 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("wrong-org/wrong-project"); const { context } = createPromptMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { yes: false, "dry-run": false }, @@ -494,7 +493,7 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue(Symbol("clack:cancel")); const { context } = createPromptMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { yes: false, "dry-run": false }, @@ -509,7 +508,7 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue(""); const { context } = createPromptMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { yes: false, "dry-run": false }, @@ -523,7 +522,7 @@ describe("project delete — interactive confirmation", () => { mockPrompt.mockResolvedValue("acme-corp/my-app"); const { context } = createPromptMockContext(); - const func = await loadCommandFunc(deleteCommand); + const func = await deleteCommand.loader(); await func.call( context, { yes: false, "dry-run": false }, diff --git a/packages/cli/test/commands/project/view.func.test.ts b/packages/cli/test/commands/project/view.func.test.ts index e6699332c..5d385b588 100644 --- a/packages/cli/test/commands/project/view.func.test.ts +++ b/packages/cli/test/commands/project/view.func.test.ts @@ -52,7 +52,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"; import type { ProjectKey, SentryProject } from "../../../src/types/sentry.js"; -import { loadCommandFunc } from "../../helpers.js"; const sampleProject: SentryProject = { id: "42", @@ -111,7 +110,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -129,7 +128,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call( context, { json: false, web: false }, @@ -145,7 +144,7 @@ describe("viewCommand.func", () => { openInBrowserSpy.mockResolvedValue(undefined); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: false, web: true }, "my-org/test-project"); expect(openInBrowserSpy).toHaveBeenCalled(); @@ -172,7 +171,7 @@ describe("viewCommand.func", () => { }); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); try { // No target arg triggers AutoDetect @@ -195,7 +194,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: true, web: false }, "frontend"); expect(resolveProjectBoundSlugSpy).toHaveBeenCalledWith( @@ -212,7 +211,7 @@ describe("viewCommand.func", () => { test("org-only target (org/) throws ContextError", async () => { const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); try { await func.call(context, { json: false, web: false }, "my-org/"); @@ -245,7 +244,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); // No target arg triggers AutoDetect await func.call(context, { json: false, web: false }); @@ -259,7 +258,7 @@ describe("viewCommand.func", () => { resolveAllTargetsSpy.mockResolvedValue({ targets: [] }); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await expect( func.call(context, { json: false, web: false }), @@ -273,7 +272,7 @@ describe("viewCommand.func", () => { }); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); try { await func.call(context, { json: false, web: false }); @@ -299,7 +298,7 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await expect( func.call(context, { json: false, web: false }, "my-org/bad-project"), @@ -320,7 +319,7 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await expect( func.call(context, { json: false, web: false }, "frontend"), @@ -349,7 +348,7 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await expect( func.call(context, { json: false, web: false }), @@ -363,7 +362,7 @@ describe("viewCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await expect( func.call(context, { json: false, web: false }, "my-org/test-project"), @@ -383,7 +382,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -405,7 +404,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -420,7 +419,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); await func.call(context, { json: true, web: false }, "my-org/test-project"); const output = stdoutWrite.mock.calls.map((c) => c[0]).join(""); @@ -438,7 +437,7 @@ describe("viewCommand.func", () => { ); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(viewCommand); + const func = await viewCommand.loader(); // `fields` is auto-injected by the buildCommand wrapper; pass through // the flag shape the wrapper would forward. await func.call( diff --git a/packages/cli/test/commands/replay/list.test.ts b/packages/cli/test/commands/replay/list.test.ts index ec5980395..3fc175e83 100644 --- a/packages/cli/test/commands/replay/list.test.ts +++ b/packages/cli/test/commands/replay/list.test.ts @@ -53,7 +53,6 @@ import { REPLAY_LIST_FIELDS, type ReplayListItem, } from "../../../src/types/index.js"; -import { loadCommandFunc } from "../../helpers.js"; describe("parseSort", () => { test("accepts supported sort values", () => { @@ -136,7 +135,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 25, json: true, period: parsePeriod("7d"), sort: "-started_at" }, @@ -171,7 +170,7 @@ describe("listCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await expect( func.call( context, @@ -194,7 +193,7 @@ describe("listCommand.func", () => { ); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await expect( func.call( context, @@ -217,7 +216,7 @@ describe("listCommand.func", () => { }); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -247,7 +246,7 @@ describe("listCommand.func", () => { listReplaysSpy.mockResolvedValue({ data: sampleReplays }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -274,7 +273,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { diff --git a/packages/cli/test/commands/trace/list.test.ts b/packages/cli/test/commands/trace/list.test.ts index ed37c049b..fb89ef87e 100644 --- a/packages/cli/test/commands/trace/list.test.ts +++ b/packages/cli/test/commands/trace/list.test.ts @@ -59,7 +59,6 @@ vi.mock("../../../src/lib/resolve-target.js", async (importOriginal) => { import * as resolveTarget from "../../../src/lib/resolve-target.js"; import { parsePeriod } from "../../../src/lib/time-range.js"; import type { TransactionListItem } from "../../../src/types/sentry.js"; -import { loadCommandFunc } from "../../helpers.js"; // Reference paginationDb early to prevent import stripping by auto-organize const _paginationDbRef = paginationDb; @@ -330,7 +329,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, @@ -350,7 +349,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, @@ -369,7 +368,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, @@ -384,7 +383,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, @@ -406,7 +405,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 2, sort: "date", json: false, period: parsePeriod("7d") }, @@ -422,7 +421,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 100, sort: "date", json: false, period: parsePeriod("7d") }, @@ -438,7 +437,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [sampleTraces[0]] }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, @@ -454,7 +453,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -488,7 +487,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, @@ -511,7 +510,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "duration", json: false, period: parsePeriod("7d") }, @@ -528,7 +527,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, @@ -546,7 +545,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("7d") }, @@ -565,7 +564,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { @@ -587,7 +586,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: sampleTraces }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: true, period: parsePeriod("7d") }, @@ -605,7 +604,7 @@ describe("listCommand.func", () => { listTransactionsSpy.mockResolvedValue({ data: [] }); const { context } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("24h") }, @@ -628,7 +627,7 @@ describe("listCommand.func", () => { }); const { context, stdoutWrite } = createMockContext(); - const func = await loadCommandFunc(listCommand); + const func = await listCommand.loader(); await func.call( context, { limit: 20, sort: "date", json: false, period: parsePeriod("30d") }, diff --git a/packages/cli/test/helpers.ts b/packages/cli/test/helpers.ts index 57a7fadc3..078a953b6 100644 --- a/packages/cli/test/helpers.ts +++ b/packages/cli/test/helpers.ts @@ -7,7 +7,6 @@ import { mkdirSync } from "node:fs"; import { mkdtemp, rm } from "node:fs/promises"; import { join } from "node:path"; -import type { Command, CommandContext } from "@stricli/core"; import { afterEach, beforeEach } from "vitest"; import { resetAccountSelection, @@ -18,8 +17,6 @@ import { } from "../src/lib/db/auth.js"; import { CONFIG_DIR_ENV_VAR, closeDatabase } from "../src/lib/db/index.js"; import { ENV_VAR_AGENTS } from "../src/lib/detect-agent.js"; -import type { PositionalDef } from "../src/lib/positional-parse.js"; -import { applyPositionalParsers } from "../src/lib/sdk-invoke.js"; import { resetTipsCache } from "../src/lib/tips.js"; // re-exporting a single constant, not a barrel @@ -354,36 +351,6 @@ export function installFetchMock( }; } -/** A loaded command func that takes raw positional strings. */ -export type RawArgsCommandFunc = ( - this: CONTEXT, - flags: object, - ...args: (string | undefined)[] -) => Promise; - -/** - * Load a command's func for direct calls with raw CLI strings. - * - * Each positional runs through the command's own parser first, as Stricli's - * scanner does, so a parsing positional (e.g. `parseOrgProjectPositional`) - * reaches the command as its parsed value. Pass `undefined` for an omitted - * optional positional. - */ -export async function loadCommandFunc( - command: Command, -): Promise> { - const loaded = await command.loader(); - const func = typeof loaded === "function" ? loaded : loaded.default; - const positional = command.parameters.positional as PositionalDef | undefined; - return async function (this: CONTEXT, flags, ...args) { - await func.call( - this, - flags as never, - ...(applyPositionalParsers(args, positional, this) as never[]), - ); - }; -} - /** Env vars `detectAgent()` reads besides the {@link ENV_VAR_AGENTS} table. */ const AGENT_ENV_VARS = [ "AI_AGENT", diff --git a/packages/cli/test/lib/positional-parse.test.ts b/packages/cli/test/lib/positional-parse.test.ts index a6d2312b3..f808033d4 100644 --- a/packages/cli/test/lib/positional-parse.test.ts +++ b/packages/cli/test/lib/positional-parse.test.ts @@ -1,5 +1,5 @@ /** - * Positional parsers that throw CliErrors (src/lib/positional-parse.ts). + * Lazily parsed positionals (src/lib/positional-parse.ts). * * Runs a test command through Stricli's `run()` so the positional parser, * the command wrapper, and exit-code handling are exercised together. @@ -18,11 +18,7 @@ import { ORG_PROJECT_TARGET, } from "../../src/lib/completion-target.js"; import { EXIT, getExitCode, ValidationError } from "../../src/lib/errors.js"; -import { - DeferredPositionalError, - deferPositionalErrors, - rawPositional, -} from "../../src/lib/positional-parse.js"; +import { rawPositional } from "../../src/lib/positional-parse.js"; import { buildRouteMap } from "../../src/lib/route-map.js"; type TestContext = CommandContext & { @@ -59,7 +55,8 @@ async function runWithTarget(args: string[]): Promise<{ }, // oxlint-disable-next-line require-yield -- test command only records the parsed target async *func(_flags, target = AUTO_DETECT_TARGET) { - parsed.push(target); + // Copying reads every property, which runs the lazy parse + parsed.push({ ...target }); }, }); const app = buildApplication( @@ -96,7 +93,7 @@ describe("parseOrgProjectPositional", () => { expect(parsed).toEqual([{ type: "auto-detect" }]); }); - test("invalid input fails with the CliError exit code, not Stricli's", async () => { + test("invalid input fails inside the command with the CliError exit code", async () => { const { parsed, stderr, exitCode } = await runWithTarget(["@latest"]); expect(parsed).toEqual([]); expect(exitCode).toBe(EXIT.VALIDATION); @@ -112,35 +109,34 @@ describe("parseOrgProjectPositional", () => { }); }); -describe("deferPositionalErrors", () => { - const parse = deferPositionalErrors(parseOrgProjectPositional); - - test("returns a CliError as a deferred value", () => { - const value = parse("@latest"); - expect(value).toBeInstanceOf(DeferredPositionalError); - expect((value as DeferredPositionalError).error).toBeInstanceOf( - ValidationError, - ); +describe("lazy parsing", () => { + test("parses on first property access", () => { + const target = parseOrgProjectPositional("@latest"); + expect(() => target.type).toThrow(ValidationError); }); - test("lets other errors propagate", () => { - const failing = deferPositionalErrors(() => { - throw new TypeError("boom"); + test("reads like the parsed value", () => { + const target = parseOrgProjectPositional("acme/web"); + expect(target).toEqual({ type: "explicit", org: "acme", project: "web" }); + expect({ ...target }).toEqual({ + type: "explicit", + org: "acme", + project: "web", }); - expect(() => failing("x")).toThrow(TypeError); + expect("org" in target).toBe(true); }); - test("keeps the completion target", () => { - expect(getCompletionTarget(parse)).toBe(ORG_PROJECT_TARGET); + test("is not a thenable, so awaiting it does not parse", async () => { + // Stricli awaits parser results; a parse here would fail in its scanner. + const target = parseOrgProjectPositional("@latest"); + await expect(Promise.resolve(target)).resolves.toBe(target); }); }); describe("rawPositional", () => { - const parse = deferPositionalErrors(parseOrgProjectPositional); - - test("returns what the user typed", () => { - expect(rawPositional(parse("acme/"))).toBe("acme/"); - expect(rawPositional(parse("@latest"))).toBe("@latest"); + test("returns what the user typed without parsing", () => { + expect(rawPositional(parseOrgProjectPositional("acme/"))).toBe("acme/"); + expect(rawPositional(parseOrgProjectPositional("@latest"))).toBe("@latest"); expect(rawPositional("plain")).toBe("plain"); expect(rawPositional(undefined)).toBeUndefined(); }); diff --git a/packages/cli/test/lib/sdk-invoke.test.ts b/packages/cli/test/lib/sdk-invoke.test.ts index 1ba7fa4b0..a843c55fd 100644 --- a/packages/cli/test/lib/sdk-invoke.test.ts +++ b/packages/cli/test/lib/sdk-invoke.test.ts @@ -1,20 +1,17 @@ /** * Unit tests for the SDK invoke layer. * - * Focuses on `applyFlagDefaults` and `applyPositionalParsers`, which - * replicate Stricli's default application and positional parsing for the SDK - * direct-invoke path (bypasses Stricli parsing). + * Focuses on `applyFlagDefaults` which replicates Stricli's default + * application for the SDK direct-invoke path (bypasses Stricli parsing). */ import { describe, expect, test } from "vitest"; import { applyFlagDefaults, - applyPositionalParsers, buildInvoker, type FlagDef, parseOutput, } from "../../src/lib/sdk-invoke.js"; -import { parseOrgProjectPositional } from "../../src/lib/arg-parsing.js"; import { useTestConfigDir } from "../helpers.js"; useTestConfigDir("sdk-invoke-"); @@ -264,64 +261,10 @@ describe("parseOutput: binary output", () => { }); }); -// --------------------------------------------------------------------------- -// applyPositionalParsers -// --------------------------------------------------------------------------- - -describe("applyPositionalParsers", () => { - test("runs tuple parsers by position", () => { - const [target, name] = applyPositionalParsers( - ["acme/web", "checkout"], - { - kind: "tuple", - parameters: [{ parse: parseOrgProjectPositional }, { parse: String }], - }, - {}, - ); - expect(target).toEqual(parseOrgProjectPositional("acme/web")); - expect(name).toBe("checkout"); - }); - - test("runs the array parser on every value", () => { - const parsed = applyPositionalParsers( - ["1", "2"], - { kind: "array", parameter: { parse: Number } }, - {}, - ); - expect(parsed).toEqual([1, 2]); - }); - - test("passes the context as `this`, like Stricli", () => { - const context = { name: "ctx" }; - const [seen] = applyPositionalParsers( - ["x"], - { - kind: "tuple", - parameters: [ - { - parse(this: unknown) { - return this; - }, - }, - ], - }, - context, - ); - expect(seen).toBe(context); - }); - - test("leaves values without a parameter definition unchanged", () => { - expect(applyPositionalParsers(["a"], undefined, {})).toEqual(["a"]); - expect( - applyPositionalParsers(["a", "b"], { kind: "tuple", parameters: [] }, {}), - ).toEqual(["a", "b"]); - }); -}); - -describe("buildInvoker: positional parsing", () => { +describe("buildInvoker: lazily parsed positionals", () => { test("a command's target positional is parsed and validated", async () => { - // Without positional parsing, project view would get the bare string - // and silently fall back to auto-detection instead of rejecting it. + // The SDK passes raw strings; the command wrapper parses the target, so + // project view rejects it instead of falling back to auto-detection. const invoke = buildInvoker({ token: "sntrys_test" }); await expect( invoke(["project", "view"], {}, ["@latest"]) as Promise, From 6eadff96145d167f6f0736662a4a7fc9a5783772 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:11:13 +0000 Subject: [PATCH 4/6] ref(cli): Bind target resolution to the positional as a lazy promise Target positionals now carry their resolver. A command declares `orgOptionalTarget(COMMAND_NAME)`, `projectBoundTarget(COMMAND_NAME)`, or `orgProjectTarget(resolve)` as the parser and gets back a value that reads like a `ParsedOrgProject` plus a `resolved` promise. `func` awaits `target.resolved` instead of choosing and calling a resolver. Parsing still runs on the first property read and resolution on the first `resolved` read, both inside `func`, so the auth guard, rc URL check, and pasted-URL host handling keep main's order. Omitted optional targets get a stand-in too, so there is no default value to remember. Refs #1564 Co-Authored-By: Burak Yigit Kaya --- packages/cli/AGENTS.md | 34 +++-- packages/cli/src/commands/feedback/list.ts | 22 +-- packages/cli/src/commands/project/delete.ts | 36 +++-- packages/cli/src/commands/project/view.ts | 142 ++++++++++-------- packages/cli/src/commands/replay/list.ts | 19 +-- packages/cli/src/commands/trace/list.ts | 27 ++-- packages/cli/src/lib/arg-parsing.ts | 19 --- packages/cli/src/lib/command.ts | 4 +- packages/cli/src/lib/completion-target.ts | 2 +- packages/cli/src/lib/list-command.ts | 6 +- packages/cli/src/lib/positional-parse.ts | 136 +++++++++++------ packages/cli/src/lib/target-positional.ts | 72 +++++++++ .../cli/test/lib/positional-parse.test.ts | 113 +++++++++----- 13 files changed, 392 insertions(+), 240 deletions(-) create mode 100644 packages/cli/src/lib/target-positional.ts diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 9d8efdc71..ac21ee7fb 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -239,25 +239,33 @@ const parsed = parseOrgProjectArg(targetArg); Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` -**Parsed target positionals:** A single `[/][]` positional can -use `parseOrgProjectPositional` from `src/lib/arg-parsing.ts`. `func` then -takes a `ParsedOrgProject` (default an optional positional to -`AUTO_DETECT_TARGET`) and never calls `parseOrgProjectArg` itself. The value -parses on first property access (`lazyPositional` in -`src/lib/positional-parse.ts`), so invalid input still throws the parser's -`CliError` from inside `func`, after the auth guard, with its exit code. Use -`rawPositional(value)` for what the user typed; it never parses. -`buildCommand` runs these parsers on positionals that arrive as strings, so the -SDK invoke path and tests that call `command.loader()` directly keep passing -raw strings. Positionals whose meaning depends on how many arguments were -given, such as `[//]`, keep parsing in `func`. +**Target positionals:** A single `[/][]` positional uses a +parser from `src/lib/target-positional.ts`, which binds the resolver: +`orgOptionalTarget(COMMAND_NAME)` (org, optional project), +`projectBoundTarget(COMMAND_NAME)` (one org and project), or +`orgProjectTarget(resolve)` for a command-specific resolver (see +`project view` and `project delete`). `func` takes the matching type +(`OrgOptionalTarget`, `ProjectBoundTarget`, `OrgProjectTarget`), reads the +parsed form as `target.type` / `target.org`, and gets the org and project with +`await target.resolved`. It never calls `parseOrgProjectArg` or a resolver +itself, and an omitted target arrives as auto-detect. + +Both steps are lazy (`lazyPositional` in `src/lib/positional-parse.ts`): the +parse runs on the first property read and the resolver on the first +`resolved` read, both inside `func` after the auth guard. Invalid input still +throws the parser's `CliError` with its exit code. Use `rawPositional(value)` +for what the user typed; it never parses. `buildCommand` runs these parsers on +positionals that arrive as strings or are omitted, so the SDK invoke path and +tests that call `command.loader()` keep passing raw strings. Positionals whose +meaning depends on how many arguments were given, such as +`[//]`, keep parsing in `func`. **Shell completion:** Target positionals that take an org or project 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. `LIST_TARGET_POSITIONAL` and `LIST_ORG_TARGET_POSITIONAL` already use them. A parser tagged with -`withCompletionTarget`, such as `parseOrgProjectPositional`, completes the same +`withCompletionTarget`, such as the `target-positional.ts` parsers, completes the same way. `pnpm run generate:completions` collects these into the gitignored `src/generated/completion-targets.json` that the `__complete` fast path reads. diff --git a/packages/cli/src/commands/feedback/list.ts b/packages/cli/src/commands/feedback/list.ts index 83c23a3d6..3081586bb 100644 --- a/packages/cli/src/commands/feedback/list.ts +++ b/packages/cli/src/commands/feedback/list.ts @@ -10,12 +10,7 @@ import { getProject, listFeedback, } from "../../lib/api-client.js"; -import { - validateLimit, - AUTO_DETECT_TARGET, - type ParsedOrgProject, - parseOrgProjectPositional, -} from "../../lib/arg-parsing.js"; +import { validateLimit } from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -39,10 +34,13 @@ import { jsonTransformListResult } from "../../lib/org-list.js"; import { withProgress } from "../../lib/polling.js"; import { type ResolvedOrgOptionalTarget, - resolveOrgOptionalTarget, toNumericId, } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type OrgOptionalTarget, + orgOptionalTarget, +} from "../../lib/target-positional.js"; import { appendPeriodHint, PERIOD_BRIEF, @@ -163,7 +161,7 @@ export const listCommand = buildListCommand("feedback", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectPositional, + parse: orgOptionalTarget(COMMAND_NAME), optional: true, }, ], @@ -203,13 +201,9 @@ export const listCommand = buildListCommand("feedback", { async *func( this: SentryContext, flags: ListFlags, - target: ParsedOrgProject = AUTO_DETECT_TARGET, + target: OrgOptionalTarget, ) { - const resolved = await resolveOrgOptionalTarget( - target, - this.cwd, - COMMAND_NAME, - ); + const resolved = await target.resolved; const projectId = await projectIdFor(resolved); const contextKey = buildPaginationContextKey( "feedback", diff --git a/packages/cli/src/commands/project/delete.ts b/packages/cli/src/commands/project/delete.ts index 975c5b237..e06d73ed5 100644 --- a/packages/cli/src/commands/project/delete.ts +++ b/packages/cli/src/commands/project/delete.ts @@ -19,10 +19,6 @@ */ import type { SentryContext } from "../../context.js"; -import { - type ParsedOrgProject, - parseOrgProjectPositional, -} from "../../lib/arg-parsing.js"; import { deleteProject, getOrganization, @@ -44,12 +40,29 @@ import { } from "../../lib/mutate-command.js"; import { resolveProjectBoundTarget } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; +import { + orgProjectTarget, + type ProjectBoundTarget, +} from "../../lib/target-positional.js"; const log = logger.withTag("project.delete"); /** Command name used in error messages and resolution hints */ const COMMAND_NAME = "project delete"; +/** + * The project to delete. Blocks auto-detect for safety: destructive commands + * require an explicit target. + */ +const deleteTarget = orgProjectTarget(async (parsed, { cwd }) => { + requireExplicitTarget( + parsed, + "Project target", + `sentry ${COMMAND_NAME} /`, + ); + return await resolveProjectBoundTarget(parsed, cwd, COMMAND_NAME); +}); + /** * Build an actionable 403 error by checking the user's org role. * @@ -171,7 +184,7 @@ export const deleteCommand = buildDeleteCommand({ { placeholder: "org/project", brief: "/ or (search across orgs)", - parse: parseOrgProjectPositional, + parse: deleteTarget, }, ], }, @@ -179,18 +192,9 @@ export const deleteCommand = buildDeleteCommand({ async *func( this: SentryContext, flags: DeleteFlags, - parsed: ParsedOrgProject, + target: ProjectBoundTarget, ) { - const { cwd } = this; - - // Block auto-detect for safety — destructive commands require explicit targets - requireExplicitTarget( - parsed, - "Project target", - `sentry ${COMMAND_NAME} /`, - ); - - const resolved = await resolveProjectBoundTarget(parsed, cwd, COMMAND_NAME); + const resolved = await target.resolved; const { org: orgSlug, project: projectSlug } = resolved; // Use already-fetched project data from project-search, or fetch for diff --git a/packages/cli/src/commands/project/view.ts b/packages/cli/src/commands/project/view.ts index a494a34f0..f3f6c1b64 100644 --- a/packages/cli/src/commands/project/view.ts +++ b/packages/cli/src/commands/project/view.ts @@ -12,10 +12,8 @@ import { tryGetPrimaryDsn, } from "../../lib/api-client.js"; import { - ProjectSpecificationType, - AUTO_DETECT_TARGET, type ParsedOrgProject, - parseOrgProjectPositional, + ProjectSpecificationType, } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; @@ -35,6 +33,11 @@ import { resolveProjectBoundSlug, } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; +import { + type OrgProjectTarget, + orgProjectTarget, + type TargetContext, +} from "../../lib/target-positional.js"; import type { SentryProject } from "../../types/index.js"; type ViewFlags = { @@ -258,6 +261,72 @@ function formatProjectViewHuman(entries: ProjectViewEntry[]): string { return parts.join("\n"); } +/** Project view targets: one for an explicit or searched project, all detected ones for auto-detect. */ +type ViewTargets = { targets: ResolvedTarget[]; footer?: string }; + +/** Resolve the `project view` positional to the projects to show. */ +async function resolveViewTargets( + parsed: ParsedOrgProject, + { cwd }: TargetContext, +): Promise { + switch (parsed.type) { + case ProjectSpecificationType.Explicit: + // Direct org/project - single target, no multi-target resolution + return { + targets: [ + { + org: parsed.org, + project: parsed.project, + orgDisplay: parsed.org, + projectDisplay: parsed.project, + }, + ], + }; + + case ProjectSpecificationType.ProjectSearch: { + // Search for project across all orgs - single target + const resolved = await resolveProjectBoundSlug( + parsed.projectSlug, + USAGE_HINT, + `sentry project view /${parsed.projectSlug}`, + parsed.originalSlug, + ); + return { + targets: [ + { + org: resolved.org, + project: resolved.project, + orgDisplay: resolved.org, + projectDisplay: resolved.project, + projectData: resolved.projectData, + }, + ], + }; + } + + case ProjectSpecificationType.OrgAll: + throw new ContextError( + "Specific project", + `sentry project view ${parsed.org}/`, + ["Specify the full org/project target, not just the organization"], + ); + + case ProjectSpecificationType.AutoDetect: { + // Auto-detect supports monorepo multi-target resolution + const result = await resolveAllTargets({ cwd }); + + if (result.targets.length === 0) { + throw buildContextError(result.skippedSelfHosted); + } + + return { targets: result.targets, footer: result.footer }; + } + + default: + throw new ContextError("Organization and project", USAGE_HINT); + } +} + export const viewCommand = buildCommand({ docs: { brief: "View details of a project", @@ -281,7 +350,7 @@ export const viewCommand = buildCommand({ { placeholder: "org/project", brief: "/, (search), or omit for auto-detect", - parse: parseOrgProjectPositional, + parse: orgProjectTarget(resolveViewTargets), optional: true, }, ], @@ -299,70 +368,11 @@ export const viewCommand = buildCommand({ async *func( this: SentryContext, flags: ViewFlags, - parsed: ParsedOrgProject = AUTO_DETECT_TARGET, + target: OrgProjectTarget, ) { applyFreshFlag(flags); - const { cwd } = this; - - let resolvedTargets: ResolvedTarget[]; - let footer: string | undefined; - - switch (parsed.type) { - case ProjectSpecificationType.Explicit: - // Direct org/project - single target, no multi-target resolution - resolvedTargets = [ - { - org: parsed.org, - project: parsed.project, - orgDisplay: parsed.org, - projectDisplay: parsed.project, - }, - ]; - break; - - case ProjectSpecificationType.ProjectSearch: { - // Search for project across all orgs - single target - const resolved = await resolveProjectBoundSlug( - parsed.projectSlug, - USAGE_HINT, - `sentry project view /${parsed.projectSlug}`, - parsed.originalSlug, - ); - resolvedTargets = [ - { - org: resolved.org, - project: resolved.project, - orgDisplay: resolved.org, - projectDisplay: resolved.project, - projectData: resolved.projectData, - }, - ]; - break; - } - case ProjectSpecificationType.OrgAll: - throw new ContextError( - "Specific project", - `sentry project view ${parsed.org}/`, - ["Specify the full org/project target, not just the organization"], - ); - - case ProjectSpecificationType.AutoDetect: { - // Auto-detect supports monorepo multi-target resolution - const result = await resolveAllTargets({ cwd }); - - if (result.targets.length === 0) { - throw buildContextError(result.skippedSelfHosted); - } - - resolvedTargets = result.targets; - footer = result.footer; - break; - } - - default: - throw new ContextError("Organization and project", USAGE_HINT); - } + const { targets: resolvedTargets, footer } = await target.resolved; if (flags.web) { await handleWebView(resolvedTargets); @@ -376,7 +386,7 @@ export const viewCommand = buildCommand({ let dsns: (string | null)[]; let targets: ResolvedTarget[]; - if (parsed.type === ProjectSpecificationType.AutoDetect) { + if (target.type === ProjectSpecificationType.AutoDetect) { const fetched = await fetchAllProjectDetails(resolvedTargets); projects = fetched.projects; dsns = fetched.dsns; diff --git a/packages/cli/src/commands/replay/list.ts b/packages/cli/src/commands/replay/list.ts index e21c18b30..c88be8c96 100644 --- a/packages/cli/src/commands/replay/list.ts +++ b/packages/cli/src/commands/replay/list.ts @@ -10,12 +10,7 @@ import { listReplays, type ReplaySortValue, } from "../../lib/api-client.js"; -import { - validateLimit, - AUTO_DETECT_TARGET, - type ParsedOrgProject, - parseOrgProjectPositional, -} from "../../lib/arg-parsing.js"; +import { validateLimit } from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -49,8 +44,11 @@ import { getReplayUserLabel, parseReplayEnvironmentFilter, } from "../../lib/replay-search.js"; -import { resolveOrgOptionalTarget } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type OrgOptionalTarget, + orgOptionalTarget, +} from "../../lib/target-positional.js"; import { appendPeriodHint, serializeTimeRange, @@ -285,7 +283,7 @@ export const listCommand = buildListCommand("replay", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: parseOrgProjectPositional, + parse: orgOptionalTarget(COMMAND_NAME), optional: true, }, ], @@ -330,14 +328,13 @@ export const listCommand = buildListCommand("replay", { async *func( this: SentryContext, flags: ListFlags, - target: ParsedOrgProject = AUTO_DETECT_TARGET, + target: OrgOptionalTarget, ) { - const { cwd } = this; const timeRange = flags.period; const environment = parseReplayEnvironmentFilter(flags.environment); const { query } = flags; - const resolved = await resolveOrgOptionalTarget(target, cwd, COMMAND_NAME); + const resolved = await target.resolved; const contextKey = buildPaginationContextKey( "replay", diff --git a/packages/cli/src/commands/trace/list.ts b/packages/cli/src/commands/trace/list.ts index 612d597bd..e76a79a6a 100644 --- a/packages/cli/src/commands/trace/list.ts +++ b/packages/cli/src/commands/trace/list.ts @@ -6,12 +6,7 @@ import type { SentryContext } from "../../context.js"; import { listTransactions } from "../../lib/api-client.js"; -import { - validateLimit, - AUTO_DETECT_TARGET, - type ParsedOrgProject, - parseOrgProjectPositional, -} from "../../lib/arg-parsing.js"; +import { validateLimit } from "../../lib/arg-parsing.js"; import { advancePaginationState, buildPaginationContextKey, @@ -34,11 +29,12 @@ import { TARGET_PATTERN_NOTE, } from "../../lib/list-command.js"; import { withProgress } from "../../lib/polling.js"; -import { - resolveLogProjectId, - resolveProjectBoundTarget, -} from "../../lib/resolve-target.js"; +import { resolveLogProjectId } from "../../lib/resolve-target.js"; import { sanitizeQuery } from "../../lib/search-query.js"; +import { + type ProjectBoundTarget, + projectBoundTarget, +} from "../../lib/target-positional.js"; import { appendPeriodHint, serializeTimeRange, @@ -229,7 +225,7 @@ export const listCommand = buildListCommand("trace", { { placeholder: "org/project", brief: "/ or (search)", - parse: parseOrgProjectPositional, + parse: projectBoundTarget(COMMAND_NAME), optional: true, }, ], @@ -265,18 +261,13 @@ export const listCommand = buildListCommand("trace", { async *func( this: SentryContext, flags: ListFlags, - target: ParsedOrgProject = AUTO_DETECT_TARGET, + target: ProjectBoundTarget, ) { - const { cwd } = this; const timeRange = flags.period; const { query } = flags; // Resolve org/project from positional arg, config, or DSN auto-detection - const { org, project } = await resolveProjectBoundTarget( - target, - cwd, - COMMAND_NAME, - ); + const { org, project } = await target.resolved; // Resolve slug → numeric ID so the Events query scopes via the `project` // param. `project:` only matches actively-selected projects and can // otherwise 400 with "not actively selected" (#1317). diff --git a/packages/cli/src/lib/arg-parsing.ts b/packages/cli/src/lib/arg-parsing.ts index a551e532d..0fa732f39 100644 --- a/packages/cli/src/lib/arg-parsing.ts +++ b/packages/cli/src/lib/arg-parsing.ts @@ -7,10 +7,8 @@ */ import type { LogSortDirection } from "./api/logs.js"; -import { ORG_PROJECT_TARGET } from "./completion-target.js"; import { ContextError, ValidationError, validationError } from "./errors.js"; import { validateResourceId } from "./input-validation.js"; -import { lazyPositional } from "./positional-parse.js"; import type { ParsedSentryUrl } from "./sentry-url-parser.js"; import { applySentryUrlContext, parseSentryUrl } from "./sentry-url-parser.js"; @@ -963,23 +961,6 @@ export function parseOrgProjectArg( return parsed; } -/** What an omitted `[/][]` positional means. */ -export const AUTO_DETECT_TARGET: ParsedOrgProject = Object.freeze({ - type: ProjectSpecificationType.AutoDetect, -}); - -/** - * Positional parser for `[/][]` targets. The command receives - * a {@link ParsedOrgProject} that runs {@link parseOrgProjectArg} on first - * property access (see `lazyPositional`), and the positional completes like - * `parseOrgProjectTarget`. Use {@link AUTO_DETECT_TARGET} as the default for - * an optional positional. - */ -export const parseOrgProjectPositional = lazyPositional( - ORG_PROJECT_TARGET, - (raw): ParsedOrgProject => parseOrgProjectArg(raw), -); - /** * Parsed issue argument types - flattened for ergonomics. * diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 7668306d8..112cf9259 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -710,10 +710,12 @@ export function buildCommand< ...positionalArgs: unknown[] ) { // Callers that skip Stricli's scanner (SDK invoke, tests) pass raw - // strings; give lazily parsed positionals the value Stricli would. + // strings, and omitted optional positionals are undefined; give lazily + // parsed positionals the value Stricli would. const args = applyLazyPositionals( positionalArgs, existingParams.positional, + this, ); applyLoggingFlags( diff --git a/packages/cli/src/lib/completion-target.ts b/packages/cli/src/lib/completion-target.ts index 26245c7a7..9992f8dcd 100644 --- a/packages/cli/src/lib/completion-target.ts +++ b/packages/cli/src/lib/completion-target.ts @@ -9,7 +9,7 @@ * * {@link parseOrgTarget} and {@link parseOrgProjectTarget} return their input * unchanged and only mark the kind. Parsers that also parse the value, such - * as `parseOrgProjectPositional` in `arg-parsing.ts`, use the same tag. + * as those in `target-positional.ts`, use the same tag. */ /** Completes organization slugs only (e.g. `org`, `org/version`, `org/id`). */ diff --git a/packages/cli/src/lib/list-command.ts b/packages/cli/src/lib/list-command.ts index a8155091d..71a264461 100644 --- a/packages/cli/src/lib/list-command.ts +++ b/packages/cli/src/lib/list-command.ts @@ -44,7 +44,7 @@ import { type ListResult, type OrgListConfig, } from "./org-list.js"; -import { rawPositional } from "./positional-parse.js"; +import { applyLazyPositionals, rawPositional } from "./positional-parse.js"; import { disableResponseCache } from "./response-cache.js"; import { PERIOD_BRIEF, parsePeriod } from "./time-range.js"; import { shellQuote } from "./utils.js"; @@ -629,7 +629,9 @@ export function buildListCommand< args[0] = undefined; } } - return originalFunc.call(this, flags, ...(args as unknown as ARGS)); + // A dropped lazy target becomes an omitted one (auto-detect) + const finalArgs = applyLazyPositionals(args, params.positional, this); + return originalFunc.call(this, flags, ...(finalArgs as unknown as ARGS)); } as typeof originalFunc; return buildCommand({ diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts index fe8c3e7f8..d8b66125d 100644 --- a/packages/cli/src/lib/positional-parse.ts +++ b/packages/cli/src/lib/positional-parse.ts @@ -1,11 +1,17 @@ /** - * Positionals whose value parses lazily. + * Positionals whose value parses and resolves lazily. * * A parser made with {@link lazyPositional} gives the command a stand-in - * for the parsed value. The stand-in has the parsed value's type, so `func` - * reads it like any other value (`target.type`, `target.org`), but the parse - * only runs on the first property access inside `func`. Until then nothing - * can fail and nothing runs early: + * typed as `T & { resolved: Promise }`: + * + * - Reading a property of `T` (e.g. `target.type`) runs the parse on first + * access, inside `func`. + * - Reading `resolved` starts the positional's own resolver on first access + * and returns its promise, so `func` awaits `target.resolved` instead of + * picking and calling a resolver itself. A parse error rejects it. + * + * Until `func` reads it, the stand-in does nothing, so nothing can fail and + * nothing runs early: * * - Stricli's argument scanner never sees an error, so invalid input still * throws the parser's `CliError` from the command, with its exit code, @@ -17,7 +23,8 @@ * * `buildCommand` also runs these parsers on positionals that reach the * command as plain strings (the SDK invoke path and tests calling the loaded - * func), so every caller can keep passing what the user typed. + * func) and on omitted optional ones (parsed from `undefined`), so `func` + * always gets a stand-in and every caller keeps passing what the user typed. * * Code that needs the raw input (plural alias interception, `help` recovery, * telemetry) reads it with {@link rawPositional}, which never parses. @@ -28,32 +35,53 @@ import { withCompletionTarget, } from "./completion-target.js"; -/** Raw input behind each stand-in. */ -const rawInputs = new WeakMap(); +/** A lazily parsed positional value with a lazily started resolution. */ +export type LazyPositional = T & { + /** Resolves the value on first access; rejects on a parse or resolve error. */ + readonly resolved: Promise; +}; + +/** Raw input behind each stand-in; `undefined` for an omitted positional. */ +const rawInputs = new WeakMap(); /** Parsers made by {@link lazyPositional}. */ const lazyParsers = new WeakSet(); +const RESOLVED_KEY = "resolved"; + /** * A stand-in for `parse(raw)` that parses on first property access. * * `then` reads as `undefined` without parsing: Stricli awaits parser * results, and a promise checks whether a resolved value is thenable. + * `resolved` is not an own key, so spreading or comparing the stand-in sees + * only the parsed value. */ -function lazyValue( - raw: string, - parse: (raw: string) => T, -): T { +function lazyValue( + raw: string | undefined, + parse: (raw: string | undefined) => T, + resolve: (value: T, context: C) => Promise, + context: C, +): LazyPositional { let parsed: { value: T } | undefined; + let resolved: Promise | undefined; const value = (): T => { parsed ??= { value: parse(raw) }; return parsed.value; }; - const standIn = new Proxy({} as T, { - get: (_target, key) => - key === "then" ? undefined : Reflect.get(value(), key), + const standIn = new Proxy({} as LazyPositional, { + get: (_target, key) => { + if (key === "then") { + return; + } + if (key === RESOLVED_KEY) { + resolved ??= (async () => await resolve(value(), context))(); + return resolved; + } + return Reflect.get(value(), key); + }, set: (_target, key, newValue) => Reflect.set(value(), key, newValue), - has: (_target, key) => Reflect.has(value(), key), + has: (_target, key) => key === RESOLVED_KEY || Reflect.has(value(), key), ownKeys: () => Reflect.ownKeys(value()), getOwnPropertyDescriptor: (_target, key) => { const descriptor = Reflect.getOwnPropertyDescriptor(value(), key); @@ -61,23 +89,27 @@ function lazyValue( return descriptor && { ...descriptor, configurable: true }; }, }); - rawInputs.set(standIn, raw); + rawInputs.set(standIn, { raw }); return standIn; } /** - * Build a positional parser that hands the command a lazily parsed value. + * Build a positional parser that hands the command a {@link LazyPositional}. * * @param kind - Shell completion kind for the positional - * @param parse - Parses the raw input; may throw a `CliError` + * @param parse - Parses the raw input (`undefined` when omitted); may throw a + * `CliError` + * @param resolve - Resolves the parsed value; gets the command context that + * Stricli passes to parsers as `this` */ -export function lazyPositional( +export function lazyPositional( kind: CompletionTarget, - parse: (raw: string) => T, -): (input: string) => T { - const parser = withCompletionTarget(kind, (input: string) => - lazyValue(input, parse), - ); + parse: (raw: string | undefined) => T, + resolve: (value: T, context: C) => Promise, +): (this: C, input?: string) => LazyPositional { + const parser = withCompletionTarget(kind, function (this: C, input?: string) { + return lazyValue(input, parse, resolve, this); + }); lazyParsers.add(parser); return parser; } @@ -87,30 +119,48 @@ type PositionalDef = | { kind: "tuple"; parameters: readonly { parse?: unknown }[] } | { kind: "array"; parameter: { parse?: unknown } }; +/** The {@link lazyPositional} parser at a positional index, if any. */ +function lazyParserAt( + def: PositionalDef, + index: number, +): ((this: unknown, input?: string) => unknown) | undefined { + const parse = + def.kind === "array" ? def.parameter.parse : def.parameters[index]?.parse; + return typeof parse === "function" && lazyParsers.has(parse) + ? (parse as (this: unknown, input?: string) => unknown) + : undefined; +} + /** - * Run {@link lazyPositional} parsers on positionals that are still strings. + * Give every {@link lazyPositional} positional its stand-in. + * + * Stricli already ran the parsers for typed input. This covers callers that + * skip its scanner (the SDK invoke path, tests calling the loaded func), + * which pass raw strings, and omitted optional positionals, which Stricli + * leaves `undefined`. Lazy parsers never throw and have no side effects, so + * this is safe to run up front. Other positionals are left as they are. * - * Stricli already ran them; callers that skip its scanner (the SDK invoke - * path, tests calling the loaded func) pass raw strings. Lazy parsers never - * throw and have no side effects, so this is safe to run up front. Other - * positionals are left as they are. + * @param context - Passed to each parser as `this`, as Stricli does */ export function applyLazyPositionals( - args: unknown[], + args: readonly unknown[], positional: unknown, + context: unknown, ): unknown[] { const def = positional as PositionalDef | undefined; if (!def) { - return args; + return [...args]; } - return args.map((arg, index) => { - const parse = - def.kind === "array" ? def.parameter.parse : def.parameters[index]?.parse; - if (typeof arg !== "string" || typeof parse !== "function") { - return arg; - } - return lazyParsers.has(parse) - ? (parse as (input: string) => unknown)(arg) + const count = + def.kind === "tuple" + ? Math.max(args.length, def.parameters.length) + : args.length; + return Array.from({ length: count }, (_, index) => { + const arg = args[index]; + const parse = lazyParserAt(def, index); + const missing = arg === undefined && def.kind === "tuple"; + return parse && (typeof arg === "string" || missing) + ? parse.call(context, arg) : arg; }); } @@ -118,11 +168,13 @@ export function applyLazyPositionals( /** * What the user typed for a positional value. Never triggers a parse. * - * @returns The raw input behind a lazily parsed value, otherwise `value` + * @returns The raw input behind a stand-in (`undefined` if omitted), + * otherwise `value` */ export function rawPositional(value: unknown): unknown { if (typeof value === "object" && value !== null) { - return rawInputs.get(value) ?? value; + const entry = rawInputs.get(value); + return entry ? entry.raw : value; } return value; } diff --git a/packages/cli/src/lib/target-positional.ts b/packages/cli/src/lib/target-positional.ts new file mode 100644 index 000000000..db878c9a1 --- /dev/null +++ b/packages/cli/src/lib/target-positional.ts @@ -0,0 +1,72 @@ +/** + * `[/][]` target positionals. + * + * Each parser here hands `func` an {@link OrgProjectTarget}: the + * {@link ParsedOrgProject} (parsed on first property access) plus a + * `resolved` promise bound to the resolver the positional declares. A + * command reads `target.type` when it needs the parsed form and awaits + * `target.resolved` for the org and project, without choosing or calling a + * parser or resolver itself. An omitted target arrives as auto-detect. See + * `positional-parse.ts` for when parsing and resolution run. + */ + +import { type ParsedOrgProject, parseOrgProjectArg } from "./arg-parsing.js"; +import { ORG_PROJECT_TARGET } from "./completion-target.js"; +import { type LazyPositional, lazyPositional } from "./positional-parse.js"; +import { + type ResolvedOrgOptionalTarget, + type ResolvedProjectBoundTarget, + resolveOrgOptionalTarget, + resolveProjectBoundTarget, +} from "./resolve-target.js"; + +/** A parsed `[/][]` target that resolves to `R`. */ +export type OrgProjectTarget = LazyPositional; + +/** What a target resolver gets from the command context. */ +export type TargetContext = { readonly cwd: string }; + +/** + * Positional parser for an `[/][]` target with its own + * resolver. Completes like `parseOrgProjectTarget`. + * + * @param resolve - Turns the parsed target into what `target.resolved` + * yields + */ +export function orgProjectTarget( + resolve: (parsed: ParsedOrgProject, context: TargetContext) => Promise, +): (this: TargetContext, input?: string) => OrgProjectTarget { + return lazyPositional( + ORG_PROJECT_TARGET, + (raw) => parseOrgProjectArg(raw), + resolve, + ); +} + +/** A target resolved with {@link resolveOrgOptionalTarget}. */ +export type OrgOptionalTarget = OrgProjectTarget; + +/** + * Target for commands that need an org and take an optional project. + * + * @param commandName - Command path for usage hints, e.g. `"feedback list"` + */ +export function orgOptionalTarget(commandName: string) { + return orgProjectTarget((parsed, { cwd }) => + resolveOrgOptionalTarget(parsed, cwd, commandName), + ); +} + +/** A target resolved with {@link resolveProjectBoundTarget}. */ +export type ProjectBoundTarget = OrgProjectTarget; + +/** + * Target for commands that need a single org and project. + * + * @param commandName - Command path for usage hints, e.g. `"trace list"` + */ +export function projectBoundTarget(commandName: string) { + return orgProjectTarget((parsed, { cwd }) => + resolveProjectBoundTarget(parsed, cwd, commandName), + ); +} diff --git a/packages/cli/test/lib/positional-parse.test.ts b/packages/cli/test/lib/positional-parse.test.ts index f808033d4..4bc745b87 100644 --- a/packages/cli/test/lib/positional-parse.test.ts +++ b/packages/cli/test/lib/positional-parse.test.ts @@ -1,17 +1,14 @@ /** - * Lazily parsed positionals (src/lib/positional-parse.ts). + * Lazily parsed and resolved target positionals (src/lib/positional-parse.ts, + * src/lib/target-positional.ts). * * Runs a test command through Stricli's `run()` so the positional parser, * the command wrapper, and exit-code handling are exercised together. */ import { buildApplication, type CommandContext, run } from "@stricli/core"; -import { describe, expect, test } from "vitest"; -import { - AUTO_DETECT_TARGET, - type ParsedOrgProject, - parseOrgProjectPositional, -} from "../../src/lib/arg-parsing.js"; +import { describe, expect, test, vi } from "vitest"; +import type { ParsedOrgProject } from "../../src/lib/arg-parsing.js"; import { buildCommand } from "../../src/lib/command.js"; import { getCompletionTarget, @@ -20,22 +17,39 @@ import { import { EXIT, getExitCode, ValidationError } from "../../src/lib/errors.js"; import { rawPositional } from "../../src/lib/positional-parse.js"; import { buildRouteMap } from "../../src/lib/route-map.js"; +import { + type OrgProjectTarget, + orgProjectTarget, +} from "../../src/lib/target-positional.js"; type TestContext = CommandContext & { + cwd: string; process: { exitCode?: number }; stdout: { write: (s: string) => boolean }; }; -/** Run `test cmd ...args` with an org/project target positional. */ +/** Resolver that echoes the parsed target and the context's cwd. */ +const echoResolver = vi.fn( + async (parsed: ParsedOrgProject, { cwd }: { cwd: string }) => ({ + parsed: { ...parsed }, + cwd, + }), +); + +const echoTarget = orgProjectTarget(echoResolver); + +type EchoTarget = OrgProjectTarget>>; + +/** Run `test cmd ...args`, recording what `target.resolved` yields. */ async function runWithTarget(args: string[]): Promise<{ - parsed: ParsedOrgProject[]; + resolved: unknown[]; stderr: string; exitCode: number | undefined; }> { - const parsed: ParsedOrgProject[] = []; + const resolved: unknown[] = []; const command = buildCommand< Record, - [ParsedOrgProject?], + [EchoTarget], TestContext >({ docs: { brief: "Takes a target" }, @@ -47,16 +61,15 @@ async function runWithTarget(args: string[]): Promise<{ { placeholder: "org/project", brief: "Target", - parse: parseOrgProjectPositional, + parse: echoTarget, optional: true, }, ], }, }, - // oxlint-disable-next-line require-yield -- test command only records the parsed target - async *func(_flags, target = AUTO_DETECT_TARGET) { - // Copying reads every property, which runs the lazy parse - parsed.push({ ...target }); + // oxlint-disable-next-line require-yield -- test command only records the resolved target + async *func(_flags, target) { + resolved.push(await target.resolved); }, }); const app = buildApplication( @@ -66,6 +79,7 @@ async function runWithTarget(args: string[]): Promise<{ let stderr = ""; const stdout = { write: () => true }; const context = { + cwd: "/work", process: { stdout, stderr: { @@ -79,23 +93,30 @@ async function runWithTarget(args: string[]): Promise<{ stdout, } as unknown as TestContext; await run(app, ["cmd", ...args], context); - return { parsed, stderr, exitCode: context.process.exitCode }; + return { resolved, stderr, exitCode: context.process.exitCode }; } -describe("parseOrgProjectPositional", () => { - test("hands the command a parsed target", async () => { - const { parsed } = await runWithTarget(["acme/web"]); - expect(parsed).toEqual([{ type: "explicit", org: "acme", project: "web" }]); +describe("orgProjectTarget", () => { + test("resolves the parsed target with the command context", async () => { + const { resolved } = await runWithTarget(["acme/web"]); + expect(resolved).toEqual([ + { + parsed: { type: "explicit", org: "acme", project: "web" }, + cwd: "/work", + }, + ]); }); - test("an omitted target means auto-detect", async () => { - const { parsed } = await runWithTarget([]); - expect(parsed).toEqual([{ type: "auto-detect" }]); + test("an omitted target resolves as auto-detect", async () => { + const { resolved } = await runWithTarget([]); + expect(resolved).toEqual([ + { parsed: { type: "auto-detect" }, cwd: "/work" }, + ]); }); test("invalid input fails inside the command with the CliError exit code", async () => { - const { parsed, stderr, exitCode } = await runWithTarget(["@latest"]); - expect(parsed).toEqual([]); + const { resolved, stderr, exitCode } = await runWithTarget(["@latest"]); + expect(resolved).toEqual([]); expect(exitCode).toBe(EXIT.VALIDATION); expect(stderr).toContain("is an issue selector"); // Not Stricli's argument-parse error, which would exit with -4. @@ -103,40 +124,58 @@ describe("parseOrgProjectPositional", () => { }); test("keeps completing org/project slugs", () => { - expect(getCompletionTarget(parseOrgProjectPositional)).toBe( - ORG_PROJECT_TARGET, - ); + expect(getCompletionTarget(echoTarget)).toBe(ORG_PROJECT_TARGET); }); }); -describe("lazy parsing", () => { +describe("lazy parsing and resolution", () => { + const context = { cwd: "/work" }; + test("parses on first property access", () => { - const target = parseOrgProjectPositional("@latest"); + const target = echoTarget.call(context, "@latest"); expect(() => target.type).toThrow(ValidationError); }); - test("reads like the parsed value", () => { - const target = parseOrgProjectPositional("acme/web"); + test("rejects resolved with the parse error", async () => { + const target = echoTarget.call(context, "@latest"); + await expect(target.resolved).rejects.toThrow(ValidationError); + }); + + test("reads like the parsed value, without the resolved getter", () => { + const target = echoTarget.call(context, "acme/web"); expect(target).toEqual({ type: "explicit", org: "acme", project: "web" }); expect({ ...target }).toEqual({ type: "explicit", org: "acme", project: "web", }); - expect("org" in target).toBe(true); + expect("resolved" in target).toBe(true); + }); + + test("resolves once, on first access of resolved", async () => { + echoResolver.mockClear(); + const target = echoTarget.call(context, "acme/web"); + expect(echoResolver).not.toHaveBeenCalled(); + const first = target.resolved; + expect(target.resolved).toBe(first); + await first; + expect(echoResolver).toHaveBeenCalledTimes(1); }); test("is not a thenable, so awaiting it does not parse", async () => { // Stricli awaits parser results; a parse here would fail in its scanner. - const target = parseOrgProjectPositional("@latest"); + const target = echoTarget.call(context, "@latest"); await expect(Promise.resolve(target)).resolves.toBe(target); }); }); describe("rawPositional", () => { + const context = { cwd: "/work" }; + test("returns what the user typed without parsing", () => { - expect(rawPositional(parseOrgProjectPositional("acme/"))).toBe("acme/"); - expect(rawPositional(parseOrgProjectPositional("@latest"))).toBe("@latest"); + expect(rawPositional(echoTarget.call(context, "acme/"))).toBe("acme/"); + expect(rawPositional(echoTarget.call(context, "@latest"))).toBe("@latest"); + expect(rawPositional(echoTarget.call(context, undefined))).toBeUndefined(); expect(rawPositional("plain")).toBe("plain"); expect(rawPositional(undefined)).toBeUndefined(); }); From e179eafc16c90b2131b4746f769c658473d691f9 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:55:04 +0000 Subject: [PATCH 5/6] ref(cli): Derive target command names and guard destructive targets Target resolvers now read the command name from `commandPrefix`, which Stricli's `forCommand` sets and the SDK invoker now sets too. So `orgOptionalTarget` and `projectBoundTarget` are plain parsers and the `COMMAND_NAME` constants they needed are gone. `buildDeleteCommand` marks its lazy positionals destructive, and the target resolver then refuses an auto-detected target with `requireExplicitTarget`. `project delete` no longer needs its own resolver for that. A new `sentry-cli/require-target-resolution` lint rule fails a command function that takes a bound target type but never reads `.resolved`, since skipping it would also skip the resolver and the destructive guard. Refs #1564 Co-Authored-By: Burak Yigit Kaya --- .oxlintrc.json | 1 + packages/cli/AGENTS.md | 24 ++++-- packages/cli/lint-rules/cli-oxlint-plugin.js | 61 ++++++++++++-- packages/cli/src/commands/feedback/list.ts | 3 +- packages/cli/src/commands/project/delete.ts | 25 +----- packages/cli/src/commands/replay/list.ts | 3 +- packages/cli/src/commands/trace/list.ts | 5 +- packages/cli/src/lib/mutate-command.ts | 4 + packages/cli/src/lib/positional-parse.ts | 47 +++++++++-- packages/cli/src/lib/sdk-invoke.ts | 6 +- packages/cli/src/lib/target-positional.ts | 83 +++++++++++++------ .../cli/test/commands/feedback/list.test.ts | 1 + .../cli/test/commands/project/delete.test.ts | 2 + .../test/commands/project/view.func.test.ts | 1 + .../cli/test/commands/replay/list.test.ts | 1 + packages/cli/test/commands/trace/list.test.ts | 1 + .../cli/test/lib/positional-parse.test.ts | 56 ++++++++++--- packages/cli/test/lib/sdk-invoke.test.ts | 9 ++ 18 files changed, 244 insertions(+), 89 deletions(-) diff --git a/.oxlintrc.json b/.oxlintrc.json index 233e8d2e0..4848b6473 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -69,6 +69,7 @@ "sentry-cli/no-inline-touch-cache": "error", "sentry-cli/no-args-join-in-release": "error", "sentry-cli/no-direct-target-resolution": "error", + "sentry-cli/require-target-resolution": "error", "sentry-cli/no-generic-is-record": "error", "sentry-cli/prefer-paginate-helper": "error", "sentry-cli/no-namespace-import": "error", diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index ac21ee7fb..31c239793 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -241,14 +241,20 @@ Reference: `span/list.ts`, `trace/view.ts`, `event/view.ts` **Target positionals:** A single `[/][]` positional uses a parser from `src/lib/target-positional.ts`, which binds the resolver: -`orgOptionalTarget(COMMAND_NAME)` (org, optional project), -`projectBoundTarget(COMMAND_NAME)` (one org and project), or -`orgProjectTarget(resolve)` for a command-specific resolver (see -`project view` and `project delete`). `func` takes the matching type -(`OrgOptionalTarget`, `ProjectBoundTarget`, `OrgProjectTarget`), reads the -parsed form as `target.type` / `target.org`, and gets the org and project with +`orgOptionalTarget` (org, optional project), `projectBoundTarget` (one org and +project), or `orgProjectTarget(resolve)` for a command-specific resolver (see +`project view`). `func` takes the matching type (`OrgOptionalTarget`, +`ProjectBoundTarget`, `OrgProjectTarget`), reads the parsed form as +`target.type` / `target.org`, and gets the org and project with `await target.resolved`. It never calls `parseOrgProjectArg` or a resolver -itself, and an omitted target arrives as auto-detect. +itself, and an omitted target arrives as auto-detect. The +`sentry-cli/require-target-resolution` lint rule fails a function that takes +one of these types and never reads `.resolved`. + +Resolvers need no command name: they read it from `this.commandPrefix`, which +Stricli's `forCommand` and the SDK invoker set (test mock contexts set it +too). In a `buildDeleteCommand` command, the target refuses to auto-detect, so +delete commands don't call `requireExplicitTarget` for it. Both steps are lazy (`lazyPositional` in `src/lib/positional-parse.ts`): the parse runs on the first property read and the resolver on the first @@ -265,8 +271,8 @@ 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. `LIST_TARGET_POSITIONAL` and `LIST_ORG_TARGET_POSITIONAL` already use them. A parser tagged with -`withCompletionTarget`, such as the `target-positional.ts` parsers, completes the same -way. +`withCompletionTarget`, such as the `target-positional.ts` parsers, completes +the same way. `pnpm run generate:completions` collects these into the gitignored `src/generated/completion-targets.json` that the `__complete` fast path reads. `project create` keeps its dedicated `name:platform` completer in diff --git a/packages/cli/lint-rules/cli-oxlint-plugin.js b/packages/cli/lint-rules/cli-oxlint-plugin.js index d1035dd58..69f58b882 100644 --- a/packages/cli/lint-rules/cli-oxlint-plugin.js +++ b/packages/cli/lint-rules/cli-oxlint-plugin.js @@ -25,20 +25,53 @@ function rule(message, visitors) { }; } -function containsIdentifier(node, identifier) { +function someNode(node, predicate) { if (!node || typeof node !== "object") return false; - if (node.type === "Identifier" && node.name === identifier) return true; + if (predicate(node)) return true; return Object.entries(node).some( ([key, value]) => key !== "parent" && (Array.isArray(value) - ? value.some((child) => containsIdentifier(child, identifier)) - : value && - typeof value === "object" && - containsIdentifier(value, identifier)), + ? value.some((child) => someNode(child, predicate)) + : value && typeof value === "object" && someNode(value, predicate)), ); } +function containsIdentifier(node, identifier) { + return someNode( + node, + (n) => n.type === "Identifier" && n.name === identifier, + ); +} + +function readsProperty(node, object, property) { + return someNode( + node, + (n) => + member(n, property) && + n.object.type === "Identifier" && + n.object.name === object, + ); +} + +/** Target positional types from `src/lib/target-positional.ts`. */ +const BOUND_TARGET_TYPES = new Set([ + "OrgOptionalTarget", + "OrgProjectTarget", + "ProjectBoundTarget", +]); + +function boundTargetParams(fn) { + return fn.params.filter((param) => { + const type = param.typeAnnotation?.typeAnnotation; + return ( + param.type === "Identifier" && + type?.type === "TSTypeReference" && + BOUND_TARGET_TYPES.has(type.typeName.name) + ); + }); +} + function silentBody(block, parameter) { if (block?.type !== "BlockStatement" || block.body.length > 1) return false; if (block.body.length === 0) return true; @@ -203,6 +236,22 @@ const rules = { } : {}, ), + "require-target-resolution": rule( + "Await `.resolved`: it runs the target's resolver and destructive-command guard.", + (_context, file, report) => { + if (!file.includes("/src/commands/")) return {}; + const check = (node) => { + for (const param of boundTargetParams(node)) { + if (!readsProperty(node.body, param.name, "resolved")) report(param); + } + }; + return { + FunctionDeclaration: check, + FunctionExpression: check, + ArrowFunctionExpression: check, + }; + }, + ), "no-generic-is-record": rule( "Use a shape-specific guard instead of isRecord().", (_context, file, report) => diff --git a/packages/cli/src/commands/feedback/list.ts b/packages/cli/src/commands/feedback/list.ts index 3081586bb..6502e2fba 100644 --- a/packages/cli/src/commands/feedback/list.ts +++ b/packages/cli/src/commands/feedback/list.ts @@ -57,7 +57,6 @@ import { const DEFAULT_PERIOD = "14d"; const DEFAULT_STATUS: FeedbackStatus = "unresolved"; const PAGINATION_KEY = "feedback-list"; -const COMMAND_NAME = "feedback list"; type ListFlags = { readonly status: FeedbackStatus; @@ -161,7 +160,7 @@ export const listCommand = buildListCommand("feedback", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: orgOptionalTarget(COMMAND_NAME), + parse: orgOptionalTarget, optional: true, }, ], diff --git a/packages/cli/src/commands/project/delete.ts b/packages/cli/src/commands/project/delete.ts index e06d73ed5..fda33c9d4 100644 --- a/packages/cli/src/commands/project/delete.ts +++ b/packages/cli/src/commands/project/delete.ts @@ -14,7 +14,8 @@ * Safety measures: * - Uses `buildDeleteCommand` — auto-injects `--yes`/`--force`/`--dry-run` * flags and enforces the non-interactive guard before `func()` runs - * - No auto-detect mode: `requireExplicitTarget` blocks accidental deletion + * - No auto-detect mode: `buildDeleteCommand` marks the target destructive, so + * `target.resolved` rejects an omitted target * - Type-out confirmation via `confirmByTyping` (unless --yes/--force) */ @@ -36,33 +37,15 @@ import { buildDeleteCommand, confirmByTyping, isConfirmationBypassed, - requireExplicitTarget, } from "../../lib/mutate-command.js"; -import { resolveProjectBoundTarget } from "../../lib/resolve-target.js"; import { buildProjectUrl } from "../../lib/sentry-web-urls.js"; import { - orgProjectTarget, type ProjectBoundTarget, + projectBoundTarget, } from "../../lib/target-positional.js"; const log = logger.withTag("project.delete"); -/** Command name used in error messages and resolution hints */ -const COMMAND_NAME = "project delete"; - -/** - * The project to delete. Blocks auto-detect for safety: destructive commands - * require an explicit target. - */ -const deleteTarget = orgProjectTarget(async (parsed, { cwd }) => { - requireExplicitTarget( - parsed, - "Project target", - `sentry ${COMMAND_NAME} /`, - ); - return await resolveProjectBoundTarget(parsed, cwd, COMMAND_NAME); -}); - /** * Build an actionable 403 error by checking the user's org role. * @@ -184,7 +167,7 @@ export const deleteCommand = buildDeleteCommand({ { placeholder: "org/project", brief: "/ or (search across orgs)", - parse: deleteTarget, + parse: projectBoundTarget, }, ], }, diff --git a/packages/cli/src/commands/replay/list.ts b/packages/cli/src/commands/replay/list.ts index c88be8c96..073e39fbd 100644 --- a/packages/cli/src/commands/replay/list.ts +++ b/packages/cli/src/commands/replay/list.ts @@ -95,7 +95,6 @@ const SORT_MAP: Record = { const DEFAULT_PERIOD = LIST_PERIOD_FLAG.default; const DEFAULT_SORT: ReplaySortValue = SORT_MAP.date; const PAGINATION_KEY = "replay-list"; -const COMMAND_NAME = "replay list"; function parseLimit(value: string): number { return validateLimit(value, LIST_MIN_LIMIT, LIST_MAX_LIMIT); @@ -283,7 +282,7 @@ export const listCommand = buildListCommand("replay", { { placeholder: "org/project", brief: "/, /, or (search)", - parse: orgOptionalTarget(COMMAND_NAME), + parse: orgOptionalTarget, optional: true, }, ], diff --git a/packages/cli/src/commands/trace/list.ts b/packages/cli/src/commands/trace/list.ts index e76a79a6a..1cefeb846 100644 --- a/packages/cli/src/commands/trace/list.ts +++ b/packages/cli/src/commands/trace/list.ts @@ -83,9 +83,6 @@ type TraceListResult = { /** Accepted values for the --sort flag */ const VALID_SORT_VALUES: SortValue[] = ["date", "duration"]; -/** Command name used in resolver error messages */ -const COMMAND_NAME = "trace list"; - /** Command key for pagination cursor storage */ export const PAGINATION_KEY = "trace-list"; @@ -225,7 +222,7 @@ export const listCommand = buildListCommand("trace", { { placeholder: "org/project", brief: "/ or (search)", - parse: projectBoundTarget(COMMAND_NAME), + parse: projectBoundTarget, optional: true, }, ], diff --git a/packages/cli/src/lib/mutate-command.ts b/packages/cli/src/lib/mutate-command.ts index f79a24ba6..53912b838 100644 --- a/packages/cli/src/lib/mutate-command.ts +++ b/packages/cli/src/lib/mutate-command.ts @@ -23,6 +23,7 @@ import { buildCommand, type CommandDocumentation } from "./command.js"; import { CliError, ContextError } from "./errors.js"; import type { CommandReturn } from "./formatters/output.js"; import { logger } from "./logger.js"; +import { markDestructive } from "./positional-parse.js"; // --------------------------------------------------------------------------- // Level A: shared flag / alias definitions @@ -399,6 +400,9 @@ export function buildDeleteCommand< ); } + // Lazy target positionals refuse to auto-detect when they resolve + markDestructive(args); + return originalFunc.call(this, flags, ...(args as unknown as ARGS)); } as typeof originalFunc; diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts index d8b66125d..9edb7bf32 100644 --- a/packages/cli/src/lib/positional-parse.ts +++ b/packages/cli/src/lib/positional-parse.ts @@ -28,6 +28,8 @@ * * Code that needs the raw input (plural alias interception, `help` recovery, * telemetry) reads it with {@link rawPositional}, which never parses. + * `buildDeleteCommand` calls {@link markDestructive} on its positionals, so + * resolvers see `destructive: true` and can refuse to auto-detect. */ import { @@ -41,8 +43,16 @@ export type LazyPositional = T & { readonly resolved: Promise; }; -/** Raw input behind each stand-in; `undefined` for an omitted positional. */ -const rawInputs = new WeakMap(); +/** What the running command tells a positional's resolver. */ +export type ResolveOptions = { + /** The command is destructive (see {@link markDestructive}). */ + readonly destructive: boolean; +}; + +/** Per stand-in state: the raw input (`undefined` if omitted) and options. */ +type StandInState = { raw: string | undefined; destructive: boolean }; + +const standIns = new WeakMap(); /** Parsers made by {@link lazyPositional}. */ const lazyParsers = new WeakSet(); @@ -60,9 +70,10 @@ const RESOLVED_KEY = "resolved"; function lazyValue( raw: string | undefined, parse: (raw: string | undefined) => T, - resolve: (value: T, context: C) => Promise, + resolve: (value: T, context: C, options: ResolveOptions) => Promise, context: C, ): LazyPositional { + const state: StandInState = { raw, destructive: false }; let parsed: { value: T } | undefined; let resolved: Promise | undefined; const value = (): T => { @@ -75,7 +86,10 @@ function lazyValue( return; } if (key === RESOLVED_KEY) { - resolved ??= (async () => await resolve(value(), context))(); + resolved ??= (async () => + await resolve(value(), context, { + destructive: state.destructive, + }))(); return resolved; } return Reflect.get(value(), key); @@ -89,7 +103,7 @@ function lazyValue( return descriptor && { ...descriptor, configurable: true }; }, }); - rawInputs.set(standIn, { raw }); + standIns.set(standIn, state); return standIn; } @@ -100,12 +114,12 @@ function lazyValue( * @param parse - Parses the raw input (`undefined` when omitted); may throw a * `CliError` * @param resolve - Resolves the parsed value; gets the command context that - * Stricli passes to parsers as `this` + * Stricli passes to parsers as `this`, and {@link ResolveOptions} */ export function lazyPositional( kind: CompletionTarget, parse: (raw: string | undefined) => T, - resolve: (value: T, context: C) => Promise, + resolve: (value: T, context: C, options: ResolveOptions) => Promise, ): (this: C, input?: string) => LazyPositional { const parser = withCompletionTarget(kind, function (this: C, input?: string) { return lazyValue(input, parse, resolve, this); @@ -165,6 +179,21 @@ export function applyLazyPositionals( }); } +/** + * Tell the resolvers of these positionals that the command is destructive. + * Must run before `func` reads `resolved`. Other values are ignored. + */ +export function markDestructive(args: readonly unknown[]): void { + for (const arg of args) { + if (typeof arg === "object" && arg !== null) { + const state = standIns.get(arg); + if (state) { + state.destructive = true; + } + } + } +} + /** * What the user typed for a positional value. Never triggers a parse. * @@ -173,8 +202,8 @@ export function applyLazyPositionals( */ export function rawPositional(value: unknown): unknown { if (typeof value === "object" && value !== null) { - const entry = rawInputs.get(value); - return entry ? entry.raw : value; + const state = standIns.get(value); + return state ? state.raw : value; } return value; } diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index bd44c4356..ec0da0339 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -564,6 +564,8 @@ export function buildInvoker(options?: SentryOptions) { positionalArgs: string[], meta?: { streaming?: boolean }, ): Promise | AsyncIterable { + // What Stricli's forCommand sets; commands read it for usage hints + const commandPrefix = ["sentry", ...commandPath]; if (meta?.streaming) { return executeWithStream(options, async (ctx, span) => { const { handler, flagDefs } = await resolveCommand(commandPath); @@ -573,7 +575,7 @@ export function buildInvoker(options?: SentryOptions) { } const resolvedFlags = applyFlagDefaults(flags, flagDefs); await handler.call( - ctx.context, + { ...ctx.context, commandPrefix }, { ...resolvedFlags, json: true }, ...positionalArgs, ); @@ -588,7 +590,7 @@ export function buildInvoker(options?: SentryOptions) { } const resolvedFlags = applyFlagDefaults(flags, flagDefs); await handler.call( - ctx.context, + { ...ctx.context, commandPrefix }, { ...resolvedFlags, json: true }, ...positionalArgs, ); diff --git a/packages/cli/src/lib/target-positional.ts b/packages/cli/src/lib/target-positional.ts index db878c9a1..7f931ae66 100644 --- a/packages/cli/src/lib/target-positional.ts +++ b/packages/cli/src/lib/target-positional.ts @@ -8,10 +8,16 @@ * `target.resolved` for the org and project, without choosing or calling a * parser or resolver itself. An omitted target arrives as auto-detect. See * `positional-parse.ts` for when parsing and resolution run. + * + * Resolution gets the command path from `commandPrefix`, which Stricli's + * `forCommand` (and the SDK invoker) set on the context, so usage hints need + * no hand-written command name. Destructive commands (`buildDeleteCommand`) + * refuse an auto-detected target. */ import { type ParsedOrgProject, parseOrgProjectArg } from "./arg-parsing.js"; import { ORG_PROJECT_TARGET } from "./completion-target.js"; +import { requireExplicitTarget } from "./mutate-command.js"; import { type LazyPositional, lazyPositional } from "./positional-parse.js"; import { type ResolvedOrgOptionalTarget, @@ -24,49 +30,78 @@ import { export type OrgProjectTarget = LazyPositional; /** What a target resolver gets from the command context. */ -export type TargetContext = { readonly cwd: string }; +export type TargetContext = { + readonly cwd: string; + /** Command path with the program name, e.g. `["sentry", "trace", "list"]` */ + readonly commandPrefix?: readonly string[]; +}; + +/** What a target resolver gets: the context plus the running command's name. */ +export type TargetResolveContext = TargetContext & { + /** Command path for usage hints, e.g. `"trace list"` */ + readonly commandName: string; +}; + +/** + * The running command's path without the program name. + * + * @throws {Error} When the context has no `commandPrefix`; every caller that + * runs a command (Stricli, the SDK invoker) sets it + */ +function commandNameOf(context: TargetContext): string { + const prefix = context.commandPrefix; + if (!prefix || prefix.length < 2) { + throw new Error( + "Target resolution needs context.commandPrefix (set by Stricli's forCommand)", + ); + } + return prefix.slice(1).join(" "); +} /** * Positional parser for an `[/][]` target with its own - * resolver. Completes like `parseOrgProjectTarget`. + * resolver. Completes like `parseOrgProjectTarget`. In a destructive command + * an auto-detected target fails `requireExplicitTarget` before `resolve` + * runs. * * @param resolve - Turns the parsed target into what `target.resolved` * yields */ export function orgProjectTarget( - resolve: (parsed: ParsedOrgProject, context: TargetContext) => Promise, + resolve: ( + parsed: ParsedOrgProject, + context: TargetResolveContext, + ) => Promise, ): (this: TargetContext, input?: string) => OrgProjectTarget { return lazyPositional( ORG_PROJECT_TARGET, (raw) => parseOrgProjectArg(raw), - resolve, + async (parsed, context: TargetContext, { destructive }) => { + const commandName = commandNameOf(context); + if (destructive) { + requireExplicitTarget( + parsed, + "Project target", + `sentry ${commandName} /`, + ); + } + return await resolve(parsed, { ...context, commandName }); + }, ); } /** A target resolved with {@link resolveOrgOptionalTarget}. */ export type OrgOptionalTarget = OrgProjectTarget; -/** - * Target for commands that need an org and take an optional project. - * - * @param commandName - Command path for usage hints, e.g. `"feedback list"` - */ -export function orgOptionalTarget(commandName: string) { - return orgProjectTarget((parsed, { cwd }) => - resolveOrgOptionalTarget(parsed, cwd, commandName), - ); -} +/** Target for commands that need an org and take an optional project. */ +export const orgOptionalTarget = orgProjectTarget((parsed, context) => + resolveOrgOptionalTarget(parsed, context.cwd, context.commandName), +); /** A target resolved with {@link resolveProjectBoundTarget}. */ export type ProjectBoundTarget = OrgProjectTarget; -/** - * Target for commands that need a single org and project. - * - * @param commandName - Command path for usage hints, e.g. `"trace list"` - */ -export function projectBoundTarget(commandName: string) { - return orgProjectTarget((parsed, { cwd }) => - resolveProjectBoundTarget(parsed, cwd, commandName), - ); -} +/** Target for commands that need a single org and project. */ +export const projectBoundTarget = orgProjectTarget((parsed, context) => + resolveProjectBoundTarget(parsed, context.cwd, context.commandName), +); diff --git a/packages/cli/test/commands/feedback/list.test.ts b/packages/cli/test/commands/feedback/list.test.ts index 8ddce9a87..2b17f4eb0 100644 --- a/packages/cli/test/commands/feedback/list.test.ts +++ b/packages/cli/test/commands/feedback/list.test.ts @@ -81,6 +81,7 @@ function createMockContext() { stdout: { write: stdoutWrite }, stderr: { write: vi.fn(() => true) }, cwd: "/tmp", + commandPrefix: ["sentry", "feedback", "list"], }, stdoutWrite, }; diff --git a/packages/cli/test/commands/project/delete.test.ts b/packages/cli/test/commands/project/delete.test.ts index dcd839625..151469704 100644 --- a/packages/cli/test/commands/project/delete.test.ts +++ b/packages/cli/test/commands/project/delete.test.ts @@ -133,6 +133,7 @@ function createMockContext() { stdout: { write: stdoutWrite }, stderr: { write: stderrWrite }, cwd: "/tmp", + commandPrefix: ["sentry", "project", "delete"], }, stdoutWrite, stderrWrite, @@ -425,6 +426,7 @@ describe("project delete — interactive confirmation", () => { stdout: { write: stdoutWrite }, stderr: { write: vi.fn(() => true) }, cwd: "/tmp", + commandPrefix: ["sentry", "project", "delete"], }, stdoutWrite, }; diff --git a/packages/cli/test/commands/project/view.func.test.ts b/packages/cli/test/commands/project/view.func.test.ts index 5d385b588..ed8b98480 100644 --- a/packages/cli/test/commands/project/view.func.test.ts +++ b/packages/cli/test/commands/project/view.func.test.ts @@ -78,6 +78,7 @@ function createMockContext() { stdout: { write: stdoutWrite }, stderr: { write: vi.fn(() => true) }, cwd: "/tmp", + commandPrefix: ["sentry", "project", "view"], }, stdoutWrite, }; diff --git a/packages/cli/test/commands/replay/list.test.ts b/packages/cli/test/commands/replay/list.test.ts index 3fc175e83..e4f05bad6 100644 --- a/packages/cli/test/commands/replay/list.test.ts +++ b/packages/cli/test/commands/replay/list.test.ts @@ -99,6 +99,7 @@ describe("listCommand.func", () => { stdout: { write: stdoutWrite }, stderr: { write: vi.fn(() => true) }, cwd: "/tmp", + commandPrefix: ["sentry", "replay", "list"], }, stdoutWrite, }; diff --git a/packages/cli/test/commands/trace/list.test.ts b/packages/cli/test/commands/trace/list.test.ts index fb89ef87e..ed16064e9 100644 --- a/packages/cli/test/commands/trace/list.test.ts +++ b/packages/cli/test/commands/trace/list.test.ts @@ -294,6 +294,7 @@ describe("listCommand.func", () => { stdout: { write: stdoutWrite }, stderr: { write: vi.fn(() => true) }, cwd: "/tmp", + commandPrefix: ["sentry", "trace", "list"], }, stdoutWrite, }; diff --git a/packages/cli/test/lib/positional-parse.test.ts b/packages/cli/test/lib/positional-parse.test.ts index 4bc745b87..8267674c2 100644 --- a/packages/cli/test/lib/positional-parse.test.ts +++ b/packages/cli/test/lib/positional-parse.test.ts @@ -14,8 +14,16 @@ import { getCompletionTarget, ORG_PROJECT_TARGET, } from "../../src/lib/completion-target.js"; -import { EXIT, getExitCode, ValidationError } from "../../src/lib/errors.js"; -import { rawPositional } from "../../src/lib/positional-parse.js"; +import { + ContextError, + EXIT, + getExitCode, + ValidationError, +} from "../../src/lib/errors.js"; +import { + markDestructive, + rawPositional, +} from "../../src/lib/positional-parse.js"; import { buildRouteMap } from "../../src/lib/route-map.js"; import { type OrgProjectTarget, @@ -28,12 +36,12 @@ type TestContext = CommandContext & { stdout: { write: (s: string) => boolean }; }; -/** Resolver that echoes the parsed target and the context's cwd. */ +/** Resolver that echoes the parsed target, cwd, and command name. */ const echoResolver = vi.fn( - async (parsed: ParsedOrgProject, { cwd }: { cwd: string }) => ({ - parsed: { ...parsed }, - cwd, - }), + async ( + parsed: ParsedOrgProject, + { cwd, commandName }: { cwd: string; commandName: string }, + ) => ({ parsed: { ...parsed }, cwd, commandName }), ); const echoTarget = orgProjectTarget(echoResolver); @@ -80,6 +88,10 @@ async function runWithTarget(args: string[]): Promise<{ const stdout = { write: () => true }; const context = { cwd: "/work", + // Like buildContext: Stricli passes the command path to forCommand + forCommand({ prefix }: { prefix: readonly string[] }) { + return { ...this, commandPrefix: prefix }; + }, process: { stdout, stderr: { @@ -97,12 +109,13 @@ async function runWithTarget(args: string[]): Promise<{ } describe("orgProjectTarget", () => { - test("resolves the parsed target with the command context", async () => { + test("resolves the parsed target with the command context and name", async () => { const { resolved } = await runWithTarget(["acme/web"]); expect(resolved).toEqual([ { parsed: { type: "explicit", org: "acme", project: "web" }, cwd: "/work", + commandName: "cmd", }, ]); }); @@ -110,7 +123,7 @@ describe("orgProjectTarget", () => { test("an omitted target resolves as auto-detect", async () => { const { resolved } = await runWithTarget([]); expect(resolved).toEqual([ - { parsed: { type: "auto-detect" }, cwd: "/work" }, + { parsed: { type: "auto-detect" }, cwd: "/work", commandName: "cmd" }, ]); }); @@ -129,7 +142,7 @@ describe("orgProjectTarget", () => { }); describe("lazy parsing and resolution", () => { - const context = { cwd: "/work" }; + const context = { cwd: "/work", commandPrefix: ["sentry", "thing", "list"] }; test("parses on first property access", () => { const target = echoTarget.call(context, "@latest"); @@ -162,6 +175,29 @@ describe("lazy parsing and resolution", () => { expect(echoResolver).toHaveBeenCalledTimes(1); }); + test("a destructive command refuses an auto-detected target", async () => { + echoResolver.mockClear(); + const omitted = echoTarget.call(context, undefined); + markDestructive([omitted]); + const error = await omitted.resolved.catch((e: unknown) => e); + expect(error).toBeInstanceOf(ContextError); + expect((error as Error).message).toContain( + "sentry thing list /", + ); + expect(echoResolver).not.toHaveBeenCalled(); + + const explicit = echoTarget.call(context, "acme/web"); + markDestructive([explicit]); + await expect(explicit.resolved).resolves.toMatchObject({ + commandName: "thing list", + }); + }); + + test("needs the command path on the context", async () => { + const target = echoTarget.call({ cwd: "/work" }, "acme/web"); + await expect(target.resolved).rejects.toThrow("commandPrefix"); + }); + test("is not a thenable, so awaiting it does not parse", async () => { // Stricli awaits parser results; a parse here would fail in its scanner. const target = echoTarget.call(context, "@latest"); diff --git a/packages/cli/test/lib/sdk-invoke.test.ts b/packages/cli/test/lib/sdk-invoke.test.ts index a843c55fd..39c54d1bc 100644 --- a/packages/cli/test/lib/sdk-invoke.test.ts +++ b/packages/cli/test/lib/sdk-invoke.test.ts @@ -270,4 +270,13 @@ describe("buildInvoker: lazily parsed positionals", () => { invoke(["project", "view"], {}, ["@latest"]) as Promise, ).rejects.toThrow("is an issue selector"); }); + + test("resolution sees the command path and the destructive guard", async () => { + // The usage hint names the command from the SDK's command path, and + // project delete refuses to auto-detect an omitted target. + const invoke = buildInvoker({ token: "sntrys_test" }); + await expect( + invoke(["project", "delete"], { yes: true }, []) as Promise, + ).rejects.toThrow("sentry project delete /"); + }); }); From 94d324db403088008bf4cd3c4cd2e912f0ce6211 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:45:01 +0000 Subject: [PATCH 6/6] ref(cli): Replace the target Proxy with a LazyPositional class `LazyPositional` now exposes `parsed`, `resolved`, and `raw` as plain class members instead of a Proxy that mimics the parsed value. Commands read `target.parsed.type`. The thenable guard, own-key and descriptor traps, and the raw-input WeakMap are gone; `rawPositional` is an `instanceof` check. The typed SDK invoker also sets `directInvoke` on its context, which turns off `help` positional recovery. A failing SDK call whose target is `help` throws its error again, as on main, instead of returning help text. Refs #1564 Co-Authored-By: Burak Yigit Kaya --- packages/cli/AGENTS.md | 11 +- packages/cli/src/commands/project/view.ts | 2 +- packages/cli/src/context.ts | 6 + packages/cli/src/lib/command.ts | 8 +- packages/cli/src/lib/positional-parse.ts | 138 ++++++++---------- packages/cli/src/lib/sdk-invoke.ts | 11 +- packages/cli/src/lib/target-positional.ts | 4 +- packages/cli/test/lib/help-positional.test.ts | 30 +++- .../cli/test/lib/positional-parse.test.ts | 12 +- 9 files changed, 115 insertions(+), 107 deletions(-) diff --git a/packages/cli/AGENTS.md b/packages/cli/AGENTS.md index 31c239793..6f9b892d3 100644 --- a/packages/cli/AGENTS.md +++ b/packages/cli/AGENTS.md @@ -245,8 +245,8 @@ parser from `src/lib/target-positional.ts`, which binds the resolver: project), or `orgProjectTarget(resolve)` for a command-specific resolver (see `project view`). `func` takes the matching type (`OrgOptionalTarget`, `ProjectBoundTarget`, `OrgProjectTarget`), reads the parsed form as -`target.type` / `target.org`, and gets the org and project with -`await target.resolved`. It never calls `parseOrgProjectArg` or a resolver +`target.parsed` (e.g. `target.parsed.type`), and gets the org and project +with `await target.resolved`. It never calls `parseOrgProjectArg` or a resolver itself, and an omitted target arrives as auto-detect. The `sentry-cli/require-target-resolution` lint rule fails a function that takes one of these types and never reads `.resolved`. @@ -257,10 +257,11 @@ too). In a `buildDeleteCommand` command, the target refuses to auto-detect, so delete commands don't call `requireExplicitTarget` for it. Both steps are lazy (`lazyPositional` in `src/lib/positional-parse.ts`): the -parse runs on the first property read and the resolver on the first +parse runs on the first `parsed` read and the resolver on the first `resolved` read, both inside `func` after the auth guard. Invalid input still -throws the parser's `CliError` with its exit code. Use `rawPositional(value)` -for what the user typed; it never parses. `buildCommand` runs these parsers on +throws the parser's `CliError` with its exit code. `target.raw` is what the +user typed; shared code that handles any positional uses +`rawPositional(value)`. `buildCommand` runs these parsers on positionals that arrive as strings or are omitted, so the SDK invoke path and tests that call `command.loader()` keep passing raw strings. Positionals whose meaning depends on how many arguments were given, such as diff --git a/packages/cli/src/commands/project/view.ts b/packages/cli/src/commands/project/view.ts index f3f6c1b64..7d509a1ef 100644 --- a/packages/cli/src/commands/project/view.ts +++ b/packages/cli/src/commands/project/view.ts @@ -386,7 +386,7 @@ export const viewCommand = buildCommand({ let dsns: (string | null)[]; let targets: ResolvedTarget[]; - if (target.type === ProjectSpecificationType.AutoDetect) { + if (target.parsed.type === ProjectSpecificationType.AutoDetect) { const fetched = await fetchAllProjectDetails(resolvedTargets); projects = fetched.projects; dsns = fetched.dsns; diff --git a/packages/cli/src/context.ts b/packages/cli/src/context.ts index 59aaad3ff..fe0e4f85c 100644 --- a/packages/cli/src/context.ts +++ b/packages/cli/src/context.ts @@ -32,6 +32,12 @@ export interface SentryContext extends CommandContext { * help when a user passes `help` as a positional argument. */ readonly commandPrefix?: readonly string[]; + /** + * Set by the typed SDK invoker, which calls a command's func directly. + * Turns off CLI-only `help` recovery, so a failing call with a `help` + * positional throws its error instead of returning help text. + */ + readonly directInvoke?: boolean; } /** diff --git a/packages/cli/src/lib/command.ts b/packages/cli/src/lib/command.ts index 112cf9259..9e6835651 100644 --- a/packages/cli/src/lib/command.ts +++ b/packages/cli/src/lib/command.ts @@ -667,7 +667,11 @@ export function buildCommand< async function maybeRecoverWithHelp( err: unknown, stdout: Writer, - ctx: { commandPrefix?: readonly string[]; stderr: Writer }, + ctx: { + commandPrefix?: readonly string[]; + directInvoke?: boolean; + stderr: Writer; + }, args: unknown[], ): Promise { if (!(err instanceof CliError) || err instanceof OutputError) { @@ -682,7 +686,7 @@ export function buildCommand< ) { return false; } - if (!ctx.commandPrefix) { + if (!ctx.commandPrefix || ctx.directInvoke) { return false; } const pathSegments = ctx.commandPrefix.slice(1); // strip "sentry" prefix diff --git a/packages/cli/src/lib/positional-parse.ts b/packages/cli/src/lib/positional-parse.ts index 9edb7bf32..b91c5fe52 100644 --- a/packages/cli/src/lib/positional-parse.ts +++ b/packages/cli/src/lib/positional-parse.ts @@ -1,16 +1,16 @@ /** * Positionals whose value parses and resolves lazily. * - * A parser made with {@link lazyPositional} gives the command a stand-in - * typed as `T & { resolved: Promise }`: + * A parser made with {@link lazyPositional} gives the command a + * {@link LazyPositional}: * - * - Reading a property of `T` (e.g. `target.type`) runs the parse on first - * access, inside `func`. - * - Reading `resolved` starts the positional's own resolver on first access - * and returns its promise, so `func` awaits `target.resolved` instead of - * picking and calling a resolver itself. A parse error rejects it. + * - `target.parsed` runs the parse on first access, inside `func`. + * - `target.resolved` starts the positional's own resolver on first access + * and returns its promise, so `func` awaits it instead of picking and + * calling a resolver itself. A parse error rejects it. + * - `target.raw` is what the user typed (`undefined` if omitted). * - * Until `func` reads it, the stand-in does nothing, so nothing can fail and + * Until `func` reads it, the value does nothing, so nothing can fail and * nothing runs early: * * - Stricli's argument scanner never sees an error, so invalid input still @@ -24,10 +24,11 @@ * `buildCommand` also runs these parsers on positionals that reach the * command as plain strings (the SDK invoke path and tests calling the loaded * func) and on omitted optional ones (parsed from `undefined`), so `func` - * always gets a stand-in and every caller keeps passing what the user typed. + * always gets a {@link LazyPositional} and every caller keeps passing what + * the user typed. * - * Code that needs the raw input (plural alias interception, `help` recovery, - * telemetry) reads it with {@link rawPositional}, which never parses. + * Shared code that handles any positional (plural alias interception, `help` + * recovery, telemetry) reads the raw input with {@link rawPositional}. * `buildDeleteCommand` calls {@link markDestructive} on its positionals, so * resolvers see `destructive: true` and can refuse to auto-detect. */ @@ -37,74 +38,55 @@ import { withCompletionTarget, } from "./completion-target.js"; -/** A lazily parsed positional value with a lazily started resolution. */ -export type LazyPositional = T & { - /** Resolves the value on first access; rejects on a parse or resolve error. */ - readonly resolved: Promise; -}; - /** What the running command tells a positional's resolver. */ export type ResolveOptions = { /** The command is destructive (see {@link markDestructive}). */ readonly destructive: boolean; }; -/** Per stand-in state: the raw input (`undefined` if omitted) and options. */ -type StandInState = { raw: string | undefined; destructive: boolean }; - -const standIns = new WeakMap(); +/** Values whose command is destructive (see {@link markDestructive}). */ +const destructiveValues = new WeakSet(); /** Parsers made by {@link lazyPositional}. */ const lazyParsers = new WeakSet(); -const RESOLVED_KEY = "resolved"; - /** - * A stand-in for `parse(raw)` that parses on first property access. - * - * `then` reads as `undefined` without parsing: Stricli awaits parser - * results, and a promise checks whether a resolved value is thenable. - * `resolved` is not an own key, so spreading or comparing the stand-in sees - * only the parsed value. + * A positional value that parses on first `parsed` read and resolves on + * first `resolved` read. Each step runs once. */ -function lazyValue( - raw: string | undefined, - parse: (raw: string | undefined) => T, - resolve: (value: T, context: C, options: ResolveOptions) => Promise, - context: C, -): LazyPositional { - const state: StandInState = { raw, destructive: false }; - let parsed: { value: T } | undefined; - let resolved: Promise | undefined; - const value = (): T => { - parsed ??= { value: parse(raw) }; - return parsed.value; - }; - const standIn = new Proxy({} as LazyPositional, { - get: (_target, key) => { - if (key === "then") { - return; - } - if (key === RESOLVED_KEY) { - resolved ??= (async () => - await resolve(value(), context, { - destructive: state.destructive, - }))(); - return resolved; - } - return Reflect.get(value(), key); - }, - set: (_target, key, newValue) => Reflect.set(value(), key, newValue), - has: (_target, key) => key === RESOLVED_KEY || Reflect.has(value(), key), - ownKeys: () => Reflect.ownKeys(value()), - getOwnPropertyDescriptor: (_target, key) => { - const descriptor = Reflect.getOwnPropertyDescriptor(value(), key); - // The empty proxy target lacks the key, so it must report configurable - return descriptor && { ...descriptor, configurable: true }; - }, - }); - standIns.set(standIn, state); - return standIn; +export class LazyPositional { + #parsed: { value: T } | undefined; + #resolved: Promise | undefined; + readonly #parse: (raw: string | undefined) => T; + readonly #resolve: (value: T, options: ResolveOptions) => Promise; + + /** What the user typed; `undefined` for an omitted positional. */ + readonly raw: string | undefined; + + constructor( + raw: string | undefined, + parse: (raw: string | undefined) => T, + resolve: (value: T, options: ResolveOptions) => Promise, + ) { + this.raw = raw; + this.#parse = parse; + this.#resolve = resolve; + } + + /** The parsed value. Throws the parser's error. */ + get parsed(): T { + this.#parsed ??= { value: this.#parse(this.raw) }; + return this.#parsed.value; + } + + /** The resolved value; rejects on a parse or resolve error. */ + get resolved(): Promise { + this.#resolved ??= (async () => + await this.#resolve(this.parsed, { + destructive: destructiveValues.has(this), + }))(); + return this.#resolved; + } } /** @@ -116,13 +98,15 @@ function lazyValue( * @param resolve - Resolves the parsed value; gets the command context that * Stricli passes to parsers as `this`, and {@link ResolveOptions} */ -export function lazyPositional( +export function lazyPositional( kind: CompletionTarget, parse: (raw: string | undefined) => T, resolve: (value: T, context: C, options: ResolveOptions) => Promise, ): (this: C, input?: string) => LazyPositional { const parser = withCompletionTarget(kind, function (this: C, input?: string) { - return lazyValue(input, parse, resolve, this); + return new LazyPositional(input, parse, (value: T, options) => + resolve(value, this, options), + ); }); lazyParsers.add(parser); return parser; @@ -185,11 +169,8 @@ export function applyLazyPositionals( */ export function markDestructive(args: readonly unknown[]): void { for (const arg of args) { - if (typeof arg === "object" && arg !== null) { - const state = standIns.get(arg); - if (state) { - state.destructive = true; - } + if (arg instanceof LazyPositional) { + destructiveValues.add(arg); } } } @@ -197,13 +178,8 @@ export function markDestructive(args: readonly unknown[]): void { /** * What the user typed for a positional value. Never triggers a parse. * - * @returns The raw input behind a stand-in (`undefined` if omitted), - * otherwise `value` + * @returns `value.raw` for a {@link LazyPositional}, otherwise `value` */ export function rawPositional(value: unknown): unknown { - if (typeof value === "object" && value !== null) { - const state = standIns.get(value); - return state ? state.raw : value; - } - return value; + return value instanceof LazyPositional ? value.raw : value; } diff --git a/packages/cli/src/lib/sdk-invoke.ts b/packages/cli/src/lib/sdk-invoke.ts index ec0da0339..898b6ec01 100644 --- a/packages/cli/src/lib/sdk-invoke.ts +++ b/packages/cli/src/lib/sdk-invoke.ts @@ -564,8 +564,11 @@ export function buildInvoker(options?: SentryOptions) { positionalArgs: string[], meta?: { streaming?: boolean }, ): Promise | AsyncIterable { - // What Stricli's forCommand sets; commands read it for usage hints - const commandPrefix = ["sentry", ...commandPath]; + // Usage hints read the command path Stricli's forCommand would set + const direct = { + commandPrefix: ["sentry", ...commandPath], + directInvoke: true, + }; if (meta?.streaming) { return executeWithStream(options, async (ctx, span) => { const { handler, flagDefs } = await resolveCommand(commandPath); @@ -575,7 +578,7 @@ export function buildInvoker(options?: SentryOptions) { } const resolvedFlags = applyFlagDefaults(flags, flagDefs); await handler.call( - { ...ctx.context, commandPrefix }, + { ...ctx.context, ...direct }, { ...resolvedFlags, json: true }, ...positionalArgs, ); @@ -590,7 +593,7 @@ export function buildInvoker(options?: SentryOptions) { } const resolvedFlags = applyFlagDefaults(flags, flagDefs); await handler.call( - { ...ctx.context, commandPrefix }, + { ...ctx.context, ...direct }, { ...resolvedFlags, json: true }, ...positionalArgs, ); diff --git a/packages/cli/src/lib/target-positional.ts b/packages/cli/src/lib/target-positional.ts index 7f931ae66..c73f85c0a 100644 --- a/packages/cli/src/lib/target-positional.ts +++ b/packages/cli/src/lib/target-positional.ts @@ -2,9 +2,9 @@ * `[/][]` target positionals. * * Each parser here hands `func` an {@link OrgProjectTarget}: the - * {@link ParsedOrgProject} (parsed on first property access) plus a + * {@link ParsedOrgProject} as `target.parsed` (parsed on first read) plus a * `resolved` promise bound to the resolver the positional declares. A - * command reads `target.type` when it needs the parsed form and awaits + * command reads `target.parsed.type` when it needs the parsed form and awaits * `target.resolved` for the org and project, without choosing or calling a * parser or resolver itself. An omitted target arrives as auto-detect. See * `positional-parse.ts` for when parsing and resolution run. diff --git a/packages/cli/test/lib/help-positional.test.ts b/packages/cli/test/lib/help-positional.test.ts index ede51691c..43ff47584 100644 --- a/packages/cli/test/lib/help-positional.test.ts +++ b/packages/cli/test/lib/help-positional.test.ts @@ -52,10 +52,13 @@ type CapturedOutput = { * We must provide it so `commandPrefix` is set on the context, enabling * the help recovery logic in `buildCommand`. */ -function buildMockContext(captured: { - stdout: string; - stderr: string; -}): SentryContext & { +function buildMockContext( + captured: { + stdout: string; + stderr: string; + }, + extra: Partial = {}, +): SentryContext & { forCommand: (opts: { prefix: readonly string[] }) => SentryContext; } { const stdoutWriter = { @@ -89,6 +92,7 @@ function buildMockContext(captured: { forCommand: ({ prefix }: { prefix: readonly string[] }): SentryContext => ({ ...baseContext, commandPrefix: prefix, + ...extra, }), }; } @@ -100,9 +104,12 @@ function buildMockContext(captured: { * will be recovered by the buildCommand wrapper, which shows help output * instead of the error. */ -async function runCommand(args: string[]): Promise { +async function runCommand( + args: string[], + extra?: Partial, +): Promise { const captured = { stdout: "", stderr: "" }; - const mockContext = buildMockContext(captured); + const mockContext = buildMockContext(captured, extra); try { await run(app, args, mockContext); @@ -114,6 +121,17 @@ async function runCommand(args: string[]): Promise { } describe("help recovery on ResolutionError", () => { + test("is off for direct (typed SDK) invocations", async () => { + // The typed SDK sets directInvoke; its callers get the error, not help + const args = ["issue", "list", "help"]; + const recovered = await runCommand(args); + const direct = await runCommand(args, { directInvoke: true }); + + expect(recovered.stdout).toContain("sentry issue list"); + expect(direct.stdout).not.toContain("sentry issue list"); + expect(direct.stderr).not.toContain("Tip"); + }); + test("sentry issue list help → shows help for issue list", async () => { // "help" is treated as a project slug, fails resolution → recovery shows help const { stdout, stderr } = await runCommand(["issue", "list", "help"]); diff --git a/packages/cli/test/lib/positional-parse.test.ts b/packages/cli/test/lib/positional-parse.test.ts index 8267674c2..0103e0536 100644 --- a/packages/cli/test/lib/positional-parse.test.ts +++ b/packages/cli/test/lib/positional-parse.test.ts @@ -144,9 +144,9 @@ describe("orgProjectTarget", () => { describe("lazy parsing and resolution", () => { const context = { cwd: "/work", commandPrefix: ["sentry", "thing", "list"] }; - test("parses on first property access", () => { + test("parses on first read of parsed", () => { const target = echoTarget.call(context, "@latest"); - expect(() => target.type).toThrow(ValidationError); + expect(() => target.parsed).toThrow(ValidationError); }); test("rejects resolved with the parse error", async () => { @@ -154,15 +154,15 @@ describe("lazy parsing and resolution", () => { await expect(target.resolved).rejects.toThrow(ValidationError); }); - test("reads like the parsed value, without the resolved getter", () => { + test("keeps the raw input and the parsed value", () => { const target = echoTarget.call(context, "acme/web"); - expect(target).toEqual({ type: "explicit", org: "acme", project: "web" }); - expect({ ...target }).toEqual({ + expect(target.raw).toBe("acme/web"); + expect(target.parsed).toEqual({ type: "explicit", org: "acme", project: "web", }); - expect("resolved" in target).toBe(true); + expect(target.parsed).toBe(target.parsed); }); test("resolves once, on first access of resolved", async () => {