diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index f5ff4605..4b104db7 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -679,7 +679,7 @@ List recent traces in a project - `--json - Output as JSON` - `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` -#### `sentry trace view ` +#### `sentry trace view ` View details of a specific trace @@ -690,14 +690,14 @@ View details of a specific trace - `--json - Output as JSON` - `--fields - Comma-separated fields to include in JSON output (dot.notation supported)` -#### `sentry trace logs ` +#### `sentry trace logs ` View logs associated with a trace **Flags:** - `-w, --web - Open trace in browser` - `-t, --period - Time period to search (e.g., "14d", "7d", "24h"). Default: 14d - (default: "14d")` -- `-n, --limit - Number of log entries (1-1000) - (default: "100")` +- `-n, --limit - Number of log entries (<=1000) - (default: "100")` - `-q, --query - Additional filter query (Sentry search syntax)` - `-f, --fresh - Bypass cache, re-detect projects, and fetch fresh data` - `--json - Output as JSON` diff --git a/src/commands/span/list.ts b/src/commands/span/list.ts index eec9680e..96bf97d2 100644 --- a/src/commands/span/list.ts +++ b/src/commands/span/list.ts @@ -7,11 +7,7 @@ import type { SentryContext } from "../../context.js"; import type { SpanSortValue } from "../../lib/api/traces.js"; import { listSpans } from "../../lib/api-client.js"; -import { - parseOrgProjectArg, - parseSlashSeparatedArg, - validateLimit, -} from "../../lib/arg-parsing.js"; +import { validateLimit } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; import { buildPaginationContextKey, @@ -19,7 +15,6 @@ import { resolveOrgCursor, setPaginationCursor, } from "../../lib/db/pagination.js"; -import { ContextError, ValidationError } from "../../lib/errors.js"; import { type FlatSpan, formatSpanTable, @@ -36,10 +31,10 @@ import { LIST_CURSOR_FLAG, } from "../../lib/list-command.js"; import { - resolveOrgAndProject, - resolveProjectBySlug, -} from "../../lib/resolve-target.js"; -import { validateTraceId } from "../../lib/trace-id.js"; + parseTraceTarget, + resolveTraceOrgProject, + warnIfNormalized, +} from "../../lib/trace-target.js"; type ListFlags = { readonly limit: number; @@ -74,49 +69,6 @@ export const PAGINATION_KEY = "span-list"; /** Usage hint for ContextError messages */ const USAGE_HINT = "sentry span list [//]"; -/** - * Parse positional arguments for span list. - * Handles: `` or `//` - * - * Uses the standard `parseSlashSeparatedArg` pattern: the last `/`-separated - * segment is the trace ID, and everything before it is the org/project target. - * - * @param args - Positional arguments from CLI - * @returns Parsed trace ID and optional target arg - * @throws {ContextError} If no arguments provided - * @throws {ValidationError} If the trace ID format is invalid - */ -export function parsePositionalArgs(args: string[]): { - traceId: string; - targetArg: string | undefined; -} { - if (args.length === 0) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - const first = args[0]; - if (first === undefined) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - if (args.length === 1) { - const { id, targetArg } = parseSlashSeparatedArg( - first, - "Trace ID", - USAGE_HINT - ); - return { traceId: validateTraceId(id), targetArg }; - } - - const second = args[1]; - if (second === undefined) { - return { traceId: validateTraceId(first), targetArg: undefined }; - } - - // Two or more args — first is target, second is trace ID - return { traceId: validateTraceId(second), targetArg: first }; -} - /** * Parse --limit flag, delegating range validation to shared utility. */ @@ -281,46 +233,15 @@ export const listCommand = buildCommand({ applyFreshFlag(flags); const { cwd, setContext } = this; - // Parse positional args - const { traceId, targetArg } = parsePositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - - // Resolve target - let target: { org: string; project: string } | null = null; - - switch (parsed.type) { - case "explicit": - target = { org: parsed.org, project: parsed.project }; - break; - - case "project-search": - target = await resolveProjectBySlug( - parsed.projectSlug, - USAGE_HINT, - `sentry span list /${parsed.projectSlug}/${traceId}` - ); - break; - - case "org-all": - throw new ContextError("Specific project", USAGE_HINT); - - case "auto-detect": - target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT }); - break; - - default: { - const _exhaustiveCheck: never = parsed; - throw new ValidationError( - `Invalid target specification: ${_exhaustiveCheck}` - ); - } - } - - if (!target) { - throw new ContextError("Organization and project", USAGE_HINT); - } - - setContext([target.org], [target.project]); + // Parse and resolve org/project/trace-id + const parsed = parseTraceTarget(args, USAGE_HINT); + warnIfNormalized(parsed, "span.list"); + const { traceId, org, project } = await resolveTraceOrgProject( + parsed, + cwd, + USAGE_HINT + ); + setContext([org], [project]); // Build server-side query const queryParts = [`trace:${traceId}`]; @@ -332,22 +253,18 @@ export const listCommand = buildCommand({ // Build context key and resolve cursor for pagination const contextKey = buildPaginationContextKey( "span", - `${target.org}/${target.project}/${traceId}`, + `${org}/${project}/${traceId}`, { sort: flags.sort, q: flags.query } ); const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey); // Fetch spans from EAP endpoint - const { data: spanItems, nextCursor } = await listSpans( - target.org, - target.project, - { - query: apiQuery, - sort: flags.sort, - limit: flags.limit, - cursor, - } - ); + const { data: spanItems, nextCursor } = await listSpans(org, project, { + query: apiQuery, + sort: flags.sort, + limit: flags.limit, + cursor, + }); // Store or clear pagination cursor if (nextCursor) { @@ -362,11 +279,11 @@ export const listCommand = buildCommand({ // Build hint footer let hint: string | undefined; if (flatSpans.length === 0 && hasMore) { - hint = `Try the next page: ${nextPageHint(target.org, target.project, traceId, flags)}`; + hint = `Try the next page: ${nextPageHint(org, project, traceId, flags)}`; } else if (flatSpans.length > 0) { const countText = `Showing ${flatSpans.length} span${flatSpans.length === 1 ? "" : "s"}.`; hint = hasMore - ? `${countText} Next page: ${nextPageHint(target.org, target.project, traceId, flags)}` + ? `${countText} Next page: ${nextPageHint(org, project, traceId, flags)}` : `${countText} Use 'sentry span view ${traceId} ' to view span details.`; } diff --git a/src/commands/span/view.ts b/src/commands/span/view.ts index 0d8aa34a..98025639 100644 --- a/src/commands/span/view.ts +++ b/src/commands/span/view.ts @@ -6,11 +6,7 @@ import type { SentryContext } from "../../context.js"; import { getDetailedTrace } from "../../lib/api-client.js"; -import { - parseOrgProjectArg, - parseSlashSeparatedArg, - spansFlag, -} from "../../lib/arg-parsing.js"; +import { spansFlag } from "../../lib/arg-parsing.js"; import { buildCommand } from "../../lib/command.js"; import { ContextError, ValidationError } from "../../lib/errors.js"; import { @@ -30,10 +26,10 @@ import { } from "../../lib/list-command.js"; import { logger } from "../../lib/logger.js"; import { - resolveOrgAndProject, - resolveProjectBySlug, -} from "../../lib/resolve-target.js"; -import { validateTraceId } from "../../lib/trace-id.js"; + parseSlashSeparatedTraceTarget, + resolveTraceOrgProject, + warnIfNormalized, +} from "../../lib/trace-target.js"; const log = logger.withTag("span.view"); @@ -51,23 +47,18 @@ const USAGE_HINT = /** * Parse positional arguments for span view. * - * Uses the same `[//]` pattern as other commands. - * The first positional is the trace ID (optionally slash-prefixed with - * org/project), and the remaining positionals are span IDs. - * - * Formats: - * - ` [...]` — auto-detect org/project - * - `// [...]` — explicit target + * The first positional is the trace ID (optionally with org/project prefix), + * parsed via the shared `parseSlashSeparatedTraceTarget`. The remaining + * positionals are span IDs. * * @param args - Positional arguments from CLI - * @returns Parsed trace ID, span IDs, and optional target arg + * @returns Parsed trace target and span IDs * @throws {ContextError} If insufficient arguments * @throws {ValidationError} If any ID has an invalid format */ export function parsePositionalArgs(args: string[]): { - traceId: string; + traceTarget: ReturnType; spanIds: string[]; - targetArg: string | undefined; } { if (args.length === 0) { throw new ContextError("Trace ID and span ID", USAGE_HINT); @@ -78,13 +69,8 @@ export function parsePositionalArgs(args: string[]): { throw new ContextError("Trace ID and span ID", USAGE_HINT); } - // First arg is trace ID (possibly with org/project prefix) - const { id, targetArg } = parseSlashSeparatedArg( - first, - "Trace ID", - USAGE_HINT - ); - const traceId = validateTraceId(id); + // First arg is trace target (possibly with org/project prefix) + const traceTarget = parseSlashSeparatedTraceTarget(first, USAGE_HINT); // Remaining args are span IDs const rawSpanIds = args.slice(1); @@ -95,7 +81,7 @@ export function parsePositionalArgs(args: string[]): { } const spanIds = rawSpanIds.map((v) => validateSpanId(v)); - return { traceId, spanIds, targetArg }; + return { traceTarget, spanIds }; } /** @@ -117,43 +103,6 @@ function warnMissingIds(spanIds: string[], foundIds: Set): void { } } -/** Resolved target type for span commands. */ -type ResolvedSpanTarget = { org: string; project: string }; - -/** - * Resolve org/project from the parsed target argument. - */ -async function resolveTarget( - parsed: ReturnType, - traceId: string, - cwd: string -): Promise { - switch (parsed.type) { - case "explicit": - return { org: parsed.org, project: parsed.project }; - - case "project-search": - return await resolveProjectBySlug( - parsed.projectSlug, - USAGE_HINT, - `sentry span view /${parsed.projectSlug}/${traceId} ` - ); - - case "org-all": - throw new ContextError("Specific project", USAGE_HINT); - - case "auto-detect": - return await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT }); - - default: { - const _exhaustiveCheck: never = parsed; - throw new ValidationError( - `Invalid target specification: ${_exhaustiveCheck}` - ); - } - } -} - // --------------------------------------------------------------------------- // Output config types and formatters // --------------------------------------------------------------------------- @@ -285,21 +234,21 @@ export const viewCommand = buildCommand({ applyFreshFlag(flags); const { cwd, setContext } = this; - // Parse positional args: first is trace ID (with optional target), rest are span IDs - const { traceId, spanIds, targetArg } = parsePositionalArgs(args); - const parsed = parseOrgProjectArg(targetArg); - - const target = await resolveTarget(parsed, traceId, cwd); + // Parse positional args: first is trace target, rest are span IDs + const { traceTarget, spanIds } = parsePositionalArgs(args); + warnIfNormalized(traceTarget, "span.view"); - if (!target) { - throw new ContextError("Organization and project", USAGE_HINT); - } - - setContext([target.org], [target.project]); + // Resolve org/project + const { traceId, org, project } = await resolveTraceOrgProject( + traceTarget, + cwd, + USAGE_HINT + ); + setContext([org], [project]); // Fetch trace data (single fetch for all span lookups) const timestamp = Math.floor(Date.now() / 1000); - const spans = await getDetailedTrace(target.org, traceId, timestamp); + const spans = await getDetailedTrace(org, traceId, timestamp); if (spans.length === 0) { throw new ValidationError( diff --git a/src/commands/trace/logs.ts b/src/commands/trace/logs.ts index 1239080c..c78a1737 100644 --- a/src/commands/trace/logs.ts +++ b/src/commands/trace/logs.ts @@ -9,7 +9,6 @@ import { listTraceLogs } from "../../lib/api-client.js"; import { validateLimit } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; -import { ContextError } from "../../lib/errors.js"; import { filterFields } from "../../lib/formatters/json.js"; import { formatLogTable } from "../../lib/formatters/log.js"; import { CommandOutput, formatFooter } from "../../lib/formatters/output.js"; @@ -18,9 +17,12 @@ import { FRESH_ALIASES, FRESH_FLAG, } from "../../lib/list-command.js"; -import { resolveOrg } from "../../lib/resolve-target.js"; import { buildTraceUrl } from "../../lib/sentry-urls.js"; -import { validateTraceId } from "../../lib/trace-id.js"; +import { + parseTraceTarget, + resolveTraceOrg, + warnIfNormalized, +} from "../../lib/trace-target.js"; type LogsFlags = { readonly json: boolean; @@ -65,9 +67,6 @@ function formatTraceLogsHuman(data: TraceLogsData): string { /** Maximum allowed value for --limit flag */ const MAX_LIMIT = 1000; -/** Minimum allowed value for --limit flag */ -const MIN_LIMIT = 1; - /** Default number of log entries to show */ const DEFAULT_LIMIT = 100; @@ -79,71 +78,13 @@ const DEFAULT_LIMIT = 100; const DEFAULT_PERIOD = "14d"; /** Usage hint shown in error messages */ -const USAGE_HINT = "sentry trace logs [] "; +const USAGE_HINT = "sentry trace logs [/]"; /** * Parse --limit flag, delegating range validation to shared utility. */ function parseLimit(value: string): number { - return validateLimit(value, MIN_LIMIT, MAX_LIMIT); -} - -/** - * Parse positional arguments for trace logs. - * - * Accepted forms: - * - `` → auto-detect org - * - ` ` → explicit org (space-separated) - * - `/` → explicit org (slash-separated, one arg) - * - * @param args - Positional arguments from CLI - * @returns Parsed trace ID and optional explicit org slug - * @throws {ContextError} If no arguments are provided - * @throws {ValidationError} If trace ID format is invalid - */ -export function parsePositionalArgs(args: string[]): { - traceId: string; - orgArg: string | undefined; -} { - if (args.length === 0) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - if (args.length === 1) { - const first = args[0]; - if (first === undefined) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - // Check for "org/traceId" slash-separated form - const slashIdx = first.indexOf("/"); - if (slashIdx !== -1) { - const orgArg = first.slice(0, slashIdx); - const traceId = first.slice(slashIdx + 1); - - if (!orgArg) { - throw new ContextError("Organization", USAGE_HINT); - } - if (!traceId) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - return { traceId: validateTraceId(traceId), orgArg }; - } - - // Plain trace ID — org will be auto-detected - return { traceId: validateTraceId(first), orgArg: undefined }; - } - - // Two or more args — first is org, second is trace ID - const orgArg = args[0]; - const traceId = args[1]; - - if (orgArg === undefined || traceId === undefined) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - return { traceId: validateTraceId(traceId), orgArg }; + return validateLimit(value, 1, MAX_LIMIT); } export const logsCommand = buildCommand({ @@ -155,12 +96,11 @@ export const logsCommand = buildCommand({ "automatically queries all projects — no project flag needed.\n\n" + "Target specification:\n" + " sentry trace logs # auto-detect org\n" + - " sentry trace logs # explicit org\n" + - " sentry trace logs / # slash-separated\n\n" + + " sentry trace logs / # explicit org\n\n" + "The trace ID is the 32-character hexadecimal identifier.\n\n" + "Examples:\n" + " sentry trace logs abc123def456abc123def456abc123de\n" + - " sentry trace logs myorg abc123def456abc123def456abc123de\n" + + " sentry trace logs myorg/abc123def456abc123def456abc123de\n" + " sentry trace logs --period 7d abc123def456abc123def456abc123de\n" + " sentry trace logs --json abc123def456abc123def456abc123de", }, @@ -177,8 +117,8 @@ export const logsCommand = buildCommand({ positional: { kind: "array", parameter: { - placeholder: "args", - brief: "[] - Optional org and required trace ID", + placeholder: "org/trace-id", + brief: "[/] - Optional org and required trace ID", parse: String, }, }, @@ -197,7 +137,7 @@ export const logsCommand = buildCommand({ limit: { kind: "parsed", parse: parseLimit, - brief: `Number of log entries (${MIN_LIMIT}-${MAX_LIMIT})`, + brief: `Number of log entries (<=${MAX_LIMIT})`, default: String(DEFAULT_LIMIT), }, query: { @@ -220,18 +160,10 @@ export const logsCommand = buildCommand({ applyFreshFlag(flags); const { cwd, setContext } = this; - const { traceId, orgArg } = parsePositionalArgs(args); - - // Resolve org — trace-logs is org-scoped, no project needed - const resolved = await resolveOrg({ org: orgArg, cwd }); - if (!resolved) { - throw new ContextError("Organization", USAGE_HINT, [ - "Set a default org with 'sentry org list', or specify one explicitly", - `Example: sentry trace logs myorg ${traceId}`, - ]); - } - - const { org } = resolved; + // Parse and resolve org/trace-id + const parsed = parseTraceTarget(args, USAGE_HINT); + warnIfNormalized(parsed, "trace.logs"); + const { traceId, org } = await resolveTraceOrg(parsed, cwd, USAGE_HINT); setContext([org], []); if (flags.web) { diff --git a/src/commands/trace/view.ts b/src/commands/trace/view.ts index a8ae90ee..39bc57fc 100644 --- a/src/commands/trace/view.ts +++ b/src/commands/trace/view.ts @@ -9,13 +9,11 @@ import { getDetailedTrace } from "../../lib/api-client.js"; import { detectSwappedViewArgs, looksLikeIssueShortId, - parseOrgProjectArg, - parseSlashSeparatedArg, spansFlag, } from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { buildCommand } from "../../lib/command.js"; -import { ContextError, ValidationError } from "../../lib/errors.js"; +import { ValidationError } from "../../lib/errors.js"; import { computeTraceSummary, formatSimpleSpanTree, @@ -28,12 +26,12 @@ import { FRESH_FLAG, } from "../../lib/list-command.js"; import { logger } from "../../lib/logger.js"; -import { - resolveOrgAndProject, - resolveProjectBySlug, -} from "../../lib/resolve-target.js"; import { buildTraceUrl } from "../../lib/sentry-urls.js"; -import { validateTraceId } from "../../lib/trace-id.js"; +import { + parseTraceTarget, + resolveTraceOrgProject, + warnIfNormalized, +} from "../../lib/trace-target.js"; type ViewFlags = { readonly json: boolean; @@ -44,84 +42,52 @@ type ViewFlags = { }; /** Usage hint for ContextError messages */ -const USAGE_HINT = "sentry trace view / "; +const USAGE_HINT = "sentry trace view [//]"; /** - * Parse positional arguments for trace view. - * Handles: `` or ` ` + * Detect UX issues in raw positional args before trace-target parsing. * - * Validates the trace ID format (32-character hex) and silently strips - * dashes from UUID-format inputs. + * - **Swapped args**: user typed ` /` instead of + * `/ `. If detected, swaps them silently and warns. + * - **Issue short ID**: first arg looks like `CAM-82X` — suggests `sentry issue view`. * - * @param args - Positional arguments from CLI - * @returns Parsed trace ID and optional target arg - * @throws {ContextError} If no arguments provided - * @throws {ValidationError} If the trace ID format is invalid + * Returns corrected args and optional warnings to emit. + * + * @internal Exported for testing */ -export function parsePositionalArgs(args: string[]): { - traceId: string; - targetArg: string | undefined; - /** Warning message if arguments appear to be in the wrong order */ +export function preProcessArgs(args: string[]): { + correctedArgs: string[]; warning?: string; - /** Suggestion when first arg looks like an issue short ID */ suggestion?: string; } { - if (args.length === 0) { - throw new ContextError("Trace ID", USAGE_HINT); + if (args.length < 2) { + return { correctedArgs: args }; } const first = args[0]; - if (first === undefined) { - throw new ContextError("Trace ID", USAGE_HINT); - } - - if (args.length === 1) { - const { id, targetArg } = parseSlashSeparatedArg( - first, - "Trace ID", - USAGE_HINT - ); - return { traceId: validateTraceId(id), targetArg }; - } - const second = args[1]; - if (second === undefined) { - return { traceId: validateTraceId(first), targetArg: undefined }; + if (!(first && second)) { + return { correctedArgs: args }; } // Detect swapped args: user put ID first and target second const swapWarning = detectSwappedViewArgs(first, second); if (swapWarning) { + // Swap them: put target first, trace ID second return { - traceId: validateTraceId(first), - targetArg: second, + correctedArgs: [second, first, ...args.slice(2)], warning: swapWarning, }; } - // Detect issue short ID passed as first arg (e.g., "CAM-82X some-trace-id") + // Detect issue short ID passed as first arg const suggestion = looksLikeIssueShortId(first) ? `Did you mean: sentry issue view ${first}` : undefined; - // Two or more args - first is target, second is trace ID - return { - traceId: validateTraceId(second), - targetArg: first, - suggestion, - }; + return { correctedArgs: args, suggestion }; } -/** - * Resolved target type for trace commands. - * @internal Exported for testing - */ -export type ResolvedTraceTarget = { - org: string; - project: string; - detectedFrom?: string; -}; - /** * Return type for trace view — includes all data both renderers need. * @internal Exported for testing @@ -157,9 +123,9 @@ export const viewCommand = buildCommand({ fullDescription: "View detailed information about a distributed trace by its ID.\n\n" + "Target specification:\n" + - " sentry trace view # auto-detect from DSN or config\n" + - " sentry trace view / # explicit org and project\n" + - " sentry trace view # find project across all orgs\n\n" + + " sentry trace view # auto-detect from DSN or config\n" + + " sentry trace view // # explicit org and project\n" + + " sentry trace view # find project across all orgs\n\n" + "The trace ID is the 32-character hexadecimal identifier.", }, output: { @@ -170,9 +136,9 @@ export const viewCommand = buildCommand({ positional: { kind: "array", parameter: { - placeholder: "args", + placeholder: "org/project/trace-id", brief: - "[/] - Target (optional) and trace ID (required)", + "[//] - Target (optional) and trace ID (required)", parse: String, }, }, @@ -192,67 +158,36 @@ export const viewCommand = buildCommand({ const { cwd, setContext } = this; const log = logger.withTag("trace.view"); - // Parse positional args - const { traceId, targetArg, warning, suggestion } = - parsePositionalArgs(args); + // Pre-process: detect swapped args and issue short IDs + const { correctedArgs, warning, suggestion } = preProcessArgs(args); if (warning) { log.warn(warning); } if (suggestion) { log.warn(suggestion); } - const parsed = parseOrgProjectArg(targetArg); - - let target: ResolvedTraceTarget | null = null; - - switch (parsed.type) { - case "explicit": - target = { - org: parsed.org, - project: parsed.project, - }; - break; - - case "project-search": - target = await resolveProjectBySlug( - parsed.projectSlug, - USAGE_HINT, - `sentry trace view /${parsed.projectSlug} ${traceId}` - ); - break; - - case "org-all": - throw new ContextError("Specific project", USAGE_HINT); - - case "auto-detect": - target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT }); - break; - - default: { - // Exhaustive check - should never reach here - const _exhaustiveCheck: never = parsed; - throw new ValidationError( - `Invalid target specification: ${_exhaustiveCheck}` - ); - } - } - if (!target) { - throw new ContextError("Organization and project", USAGE_HINT); - } + // Parse and resolve org/project/trace-id + const parsed = parseTraceTarget(correctedArgs, USAGE_HINT); + warnIfNormalized(parsed, "trace.view"); + const { traceId, org, project } = await resolveTraceOrgProject( + parsed, + cwd, + USAGE_HINT + ); // Set telemetry context - setContext([target.org], [target.project]); + setContext([org], [project]); if (flags.web) { - await openInBrowser(buildTraceUrl(target.org, traceId), "trace"); + await openInBrowser(buildTraceUrl(org, traceId), "trace"); return; } // The trace API requires a timestamp to help locate the trace data. // Use current time - the API will search around this timestamp. const timestamp = Math.floor(Date.now() / 1000); - const spans = await getDetailedTrace(target.org, traceId, timestamp); + const spans = await getDetailedTrace(org, traceId, timestamp); if (spans.length === 0) { throw new ValidationError( diff --git a/src/lib/trace-target.ts b/src/lib/trace-target.ts new file mode 100644 index 00000000..7a77c804 --- /dev/null +++ b/src/lib/trace-target.ts @@ -0,0 +1,399 @@ +/** + * Shared Trace-Target Parsing & Resolution + * + * Provides a unified abstraction for commands that accept trace IDs + * with optional org/project context. Trace IDs are globally unique, + * so these formats are all supported: + * + * - `` — auto-detect org/project from DSN/config + * - `/` — org-scoped (for org-only APIs like trace-logs) + * - `//` — fully explicit + * + * Also handles two-arg forms: + * - `/ ` — target as first arg, trace ID as second + * - ` ` — org as first arg, trace ID as second + * + * Used by: span list, span view, trace view, trace logs. + */ + +import { normalizeSlug, parseOrgProjectArg } from "./arg-parsing.js"; +import { ContextError, ValidationError } from "./errors.js"; +import { logger } from "./logger.js"; +import { + resolveOrg, + resolveOrgAndProject, + resolveProjectBySlug, +} from "./resolve-target.js"; +import { validateTraceId } from "./trace-id.js"; + +/** Match `[]` in usageHint — captures bracket content + trailing placeholder */ +const USAGE_TARGET_RE = /\[.*\]<[^>]+>/; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +/** + * Parsed result from trace-related positional arguments. + * Discriminated union based on the `type` field. + * + * Unlike `ParsedOrgProject`, the `traceId` is always present + * because it's the mandatory identifier for all trace commands. + */ +export type ParsedTraceTarget = + | { + type: "explicit"; + traceId: string; + org: string; + project: string; + /** True if any slug was normalized (underscores → dashes) */ + normalized?: boolean; + } + | { + type: "org-scoped"; + traceId: string; + org: string; + /** True if org slug was normalized */ + normalized?: boolean; + } + | { + type: "project-search"; + traceId: string; + projectSlug: string; + /** True if slug was normalized */ + normalized?: boolean; + } + | { + type: "auto-detect"; + traceId: string; + }; + +/** Resolved trace target with both org and project. */ +export type ResolvedTraceOrgProject = { + traceId: string; + org: string; + project: string; +}; + +/** Resolved trace target with org only. */ +export type ResolvedTraceOrg = { + traceId: string; + org: string; +}; + +// --------------------------------------------------------------------------- +// Parsing +// --------------------------------------------------------------------------- + +/** + * Parse trace-related positional arguments into a {@link ParsedTraceTarget}. + * + * **Single argument (slash-separated):** + * - `` → auto-detect + * - `/` → org-scoped + * - `//` → explicit + * + * **Two arguments (space-separated):** + * - `/ ` → explicit + * - ` ` → project-search (bare slug) + * + * Extra positional arguments beyond the first two are ignored with a + * warning, matching the established pattern across CLI commands. + * + * @param args - Positional arguments from CLI + * @param usageHint - Usage example for error messages + * @returns Parsed trace target with type discrimination + * @throws {ContextError} If no arguments are provided + * @throws {ValidationError} If the trace ID format is invalid + */ +export function parseTraceTarget( + args: string[], + usageHint: string +): ParsedTraceTarget { + if (args.length === 0) { + throw new ContextError("Trace ID", usageHint); + } + + const first = args[0]; + if (first === undefined) { + throw new ContextError("Trace ID", usageHint); + } + + // Warn about extra positional args that will be ignored + if (args.length > 2) { + const log = logger.withTag("trace-target"); + log.warn( + `Extra arguments ignored: ${args.slice(2).join(" ")}. Expected: ${usageHint}` + ); + } + + if (args.length === 1) { + return parseSlashSeparatedTraceTarget(first, usageHint); + } + + // Two+ args: first is target context, second is trace ID + const second = args[1]; + if (second === undefined) { + return parseSlashSeparatedTraceTarget(first, usageHint); + } + + const traceId = validateTraceId(second); + return targetArgToTraceTarget(first, traceId); +} + +/** + * Parse a single slash-separated argument into a trace target. + * + * - No slashes → auto-detect (bare trace ID) + * - One slash → `/` (org-scoped) + * - Two+ slashes → split on last `/` → `//` (explicit) + * + * @internal Exported for testing + */ +export function parseSlashSeparatedTraceTarget( + input: string, + usageHint: string +): ParsedTraceTarget { + const lastSlash = input.lastIndexOf("/"); + + if (lastSlash === -1) { + // No slashes — bare trace ID + return { type: "auto-detect", traceId: validateTraceId(input) }; + } + + const prefix = input.slice(0, lastSlash); + const rawTraceId = input.slice(lastSlash + 1); + + if (!rawTraceId) { + throw new ContextError("Trace ID", usageHint); + } + + const traceId = validateTraceId(rawTraceId); + + if (!prefix) { + // "/" — leading slash, treat as auto-detect + return { type: "auto-detect", traceId }; + } + + const innerSlash = prefix.indexOf("/"); + + if (innerSlash === -1) { + // "org/" — org-scoped + const ns = normalizeSlug(prefix); + return { + type: "org-scoped", + traceId, + org: ns.slug, + ...(ns.normalized && { normalized: true }), + }; + } + + // "org/project/" — explicit + const rawOrg = prefix.slice(0, innerSlash); + const rawProject = prefix.slice(innerSlash + 1); + + if (!(rawOrg && rawProject)) { + throw new ContextError("Trace ID", usageHint); + } + + const no = normalizeSlug(rawOrg); + const np = normalizeSlug(rawProject); + const normalized = no.normalized || np.normalized; + + return { + type: "explicit", + traceId, + org: no.slug, + project: np.slug, + ...(normalized && { normalized: true }), + }; +} + +/** + * Convert a target argument + validated trace ID into a trace target. + * Delegates org/project parsing to {@link parseOrgProjectArg}. + * + * Note: `parseOrgProjectArg` already emits slug normalization warnings + * internally, so `normalized` is NOT propagated here to avoid double + * warnings when callers also check via {@link warnIfNormalized}. + * + * @internal Exported for testing + */ +export function targetArgToTraceTarget( + targetArg: string, + traceId: string +): ParsedTraceTarget { + const parsed = parseOrgProjectArg(targetArg); + + switch (parsed.type) { + case "explicit": + return { + type: "explicit", + traceId, + org: parsed.org, + project: parsed.project, + }; + + case "org-all": + // "org/" → org-scoped (valid for trace commands, not "all projects") + return { + type: "org-scoped", + traceId, + org: parsed.org, + }; + + case "project-search": + return { + type: "project-search", + traceId, + projectSlug: parsed.projectSlug, + }; + + case "auto-detect": + return { type: "auto-detect", traceId }; + + default: { + const _exhaustive: never = parsed; + throw new ValidationError(`Unexpected target: ${_exhaustive}`); + } + } +} + +/** + * Emit a slug normalization warning if applicable. + * Shared by all trace commands to avoid duplicating the check. + */ +export function warnIfNormalized(parsed: ParsedTraceTarget, tag: string): void { + if ("normalized" in parsed && parsed.normalized) { + const log = logger.withTag(tag); + log.warn("Normalized slug (Sentry slugs use dashes, not underscores)"); + } +} + +// --------------------------------------------------------------------------- +// Resolution — org + project +// --------------------------------------------------------------------------- + +/** + * Resolve a parsed trace target to org + project. + * + * For commands like `span list` and `trace view` that require both + * org and project for API calls. + * + * @throws {ContextError} If org-scoped without project + * @throws {ContextError} If auto-detection fails + */ +export async function resolveTraceOrgProject( + parsed: ParsedTraceTarget, + cwd: string, + usageHint: string +): Promise { + switch (parsed.type) { + case "explicit": + return { + traceId: parsed.traceId, + org: parsed.org, + project: parsed.project, + }; + + case "project-search": + return resolveProjectSearchTarget(parsed, usageHint); + + case "org-scoped": + throw new ContextError("Specific project", usageHint, [ + `Use: ${usageHint.replace(USAGE_TARGET_RE, `${parsed.org}//${parsed.traceId}`)}`, + `List projects: sentry project list ${parsed.org}/`, + ]); + + case "auto-detect": { + const resolved = await resolveOrgAndProject({ + cwd, + usageHint, + }); + if (!resolved) { + throw new ContextError("Organization and project", usageHint); + } + return { + traceId: parsed.traceId, + org: resolved.org, + project: resolved.project, + }; + } + + default: { + const _exhaustive: never = parsed; + throw new ValidationError(`Unexpected target type: ${_exhaustive}`); + } + } +} + +/** Resolve a project-search target by searching across orgs. */ +async function resolveProjectSearchTarget( + parsed: Extract, + usageHint: string +): Promise { + const target = await resolveProjectBySlug( + parsed.projectSlug, + usageHint, + usageHint.replace( + USAGE_TARGET_RE, + `/${parsed.projectSlug}/${parsed.traceId}` + ) + ); + return { + traceId: parsed.traceId, + org: target.org, + project: target.project, + }; +} + +// --------------------------------------------------------------------------- +// Resolution — org only +// --------------------------------------------------------------------------- + +/** + * Resolve a parsed trace target to org only. + * + * For commands like `trace logs` where the API is org-scoped. + * When both org and project are provided, only org is used. + * + * @throws {ContextError} If auto-detection fails + */ +export async function resolveTraceOrg( + parsed: ParsedTraceTarget, + cwd: string, + usageHint: string +): Promise { + switch (parsed.type) { + case "explicit": + return { traceId: parsed.traceId, org: parsed.org }; + + case "org-scoped": + return { traceId: parsed.traceId, org: parsed.org }; + + case "project-search": { + // Bare slug in org-only context → treat as org slug + const resolved = await resolveOrg({ org: parsed.projectSlug, cwd }); + if (!resolved) { + throw new ContextError("Organization", usageHint, [ + `Could not resolve "${parsed.projectSlug}" as an organization.`, + `Specify the org explicitly: /${parsed.traceId}`, + ]); + } + return { traceId: parsed.traceId, org: resolved.org }; + } + + case "auto-detect": { + const resolved = await resolveOrg({ cwd }); + if (!resolved) { + throw new ContextError("Organization", usageHint); + } + return { traceId: parsed.traceId, org: resolved.org }; + } + + default: { + const _exhaustive: never = parsed; + throw new ValidationError(`Unexpected target type: ${_exhaustive}`); + } + } +} diff --git a/test/commands/span/list.test.ts b/test/commands/span/list.test.ts index c2875fa3..49d6d91e 100644 --- a/test/commands/span/list.test.ts +++ b/test/commands/span/list.test.ts @@ -14,88 +14,15 @@ import { spyOn, test, } from "bun:test"; -import { - listCommand, - parsePositionalArgs, - parseSort, -} from "../../../src/commands/span/list.js"; +import { listCommand, parseSort } from "../../../src/commands/span/list.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; -import { ContextError, ValidationError } from "../../../src/lib/errors.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as resolveTarget from "../../../src/lib/resolve-target.js"; const VALID_TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; -describe("parsePositionalArgs", () => { - describe("single argument (trace ID only)", () => { - test("parses plain trace ID", () => { - const result = parsePositionalArgs([VALID_TRACE_ID]); - expect(result.traceId).toBe(VALID_TRACE_ID); - expect(result.targetArg).toBeUndefined(); - }); - - test("normalizes uppercase trace ID", () => { - const result = parsePositionalArgs(["AAAA1111BBBB2222CCCC3333DDDD4444"]); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("strips dashes from UUID-format input", () => { - const result = parsePositionalArgs([ - "aaaa1111-bbbb-2222-cccc-3333dddd4444", - ]); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - }); - - describe("slash-separated argument (org/project/trace-id)", () => { - test("parses org/project/trace-id format", () => { - const result = parsePositionalArgs([ - `my-org/my-project/${VALID_TRACE_ID}`, - ]); - expect(result.traceId).toBe(VALID_TRACE_ID); - expect(result.targetArg).toBe("my-org/my-project"); - }); - - test("single slash (org/project without ID) throws ContextError", () => { - // "my-project/trace-id" has exactly one slash → parseSlashSeparatedArg - // treats it as "org/project" without an ID, which throws - expect(() => - parsePositionalArgs([`my-project/${VALID_TRACE_ID}`]) - ).toThrow(ContextError); - }); - }); - - describe("two arguments (target + trace-id)", () => { - test("parses target and trace ID", () => { - const result = parsePositionalArgs(["my-org/frontend", VALID_TRACE_ID]); - expect(result.targetArg).toBe("my-org/frontend"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("parses project-only target", () => { - const result = parsePositionalArgs(["frontend", VALID_TRACE_ID]); - expect(result.targetArg).toBe("frontend"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - }); - - describe("error cases", () => { - test("throws ContextError for empty args", () => { - expect(() => parsePositionalArgs([])).toThrow(ContextError); - }); - - test("throws ValidationError for invalid trace ID", () => { - expect(() => parsePositionalArgs(["not-a-trace-id"])).toThrow( - ValidationError - ); - }); - - test("throws ValidationError for short hex", () => { - expect(() => parsePositionalArgs(["aabbccdd"])).toThrow(ValidationError); - }); - }); -}); +// Note: parseTraceTarget parsing tests are in test/lib/trace-target.test.ts describe("parseSort", () => { test("accepts 'date'", () => { diff --git a/test/commands/span/view.test.ts b/test/commands/span/view.test.ts index 699c1956..05117fc3 100644 --- a/test/commands/span/view.test.ts +++ b/test/commands/span/view.test.ts @@ -74,9 +74,9 @@ describe("parsePositionalArgs", () => { describe("trace-id + single span-id", () => { test("parses trace ID and span ID as two positional args", () => { const result = parsePositionalArgs([VALID_TRACE_ID, VALID_SPAN_ID]); - expect(result.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.type).toBe("auto-detect"); expect(result.spanIds).toEqual([VALID_SPAN_ID]); - expect(result.targetArg).toBeUndefined(); }); }); @@ -87,9 +87,8 @@ describe("parsePositionalArgs", () => { VALID_SPAN_ID, VALID_SPAN_ID_2, ]); - expect(result.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.traceId).toBe(VALID_TRACE_ID); expect(result.spanIds).toEqual([VALID_SPAN_ID, VALID_SPAN_ID_2]); - expect(result.targetArg).toBeUndefined(); }); }); @@ -99,9 +98,9 @@ describe("parsePositionalArgs", () => { `my-org/my-project/${VALID_TRACE_ID}`, VALID_SPAN_ID, ]); - expect(result.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.type).toBe("explicit"); expect(result.spanIds).toEqual([VALID_SPAN_ID]); - expect(result.targetArg).toBe("my-org/my-project"); }); test("parses slash-separated target with multiple span IDs", () => { @@ -110,9 +109,9 @@ describe("parsePositionalArgs", () => { VALID_SPAN_ID, VALID_SPAN_ID_2, ]); - expect(result.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.traceId).toBe(VALID_TRACE_ID); + expect(result.traceTarget.type).toBe("explicit"); expect(result.spanIds).toEqual([VALID_SPAN_ID, VALID_SPAN_ID_2]); - expect(result.targetArg).toBe("my-org/my-project"); }); }); diff --git a/test/commands/trace/logs.test.ts b/test/commands/trace/logs.test.ts index 4c9a4f06..31a54520 100644 --- a/test/commands/trace/logs.test.ts +++ b/test/commands/trace/logs.test.ts @@ -20,137 +20,15 @@ import { spyOn, test, } from "bun:test"; -import { - logsCommand, - parsePositionalArgs, -} from "../../../src/commands/trace/logs.js"; +import { logsCommand } from "../../../src/commands/trace/logs.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; -import { ContextError, ValidationError } from "../../../src/lib/errors.js"; +import { ContextError } from "../../../src/lib/errors.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as resolveTarget from "../../../src/lib/resolve-target.js"; import type { TraceLog } from "../../../src/types/sentry.js"; -// ============================================================================ -// parsePositionalArgs -// ============================================================================ - -const VALID_TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; - -describe("parsePositionalArgs", () => { - describe("no arguments", () => { - test("throws ContextError when called with empty args", () => { - expect(() => parsePositionalArgs([])).toThrow(ContextError); - }); - - test("error mentions 'Trace ID'", () => { - try { - parsePositionalArgs([]); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ContextError); - expect((error as ContextError).message).toContain("Trace ID"); - } - }); - }); - - describe("single argument — plain trace ID", () => { - test("parses 32-char hex trace ID with no org", () => { - const result = parsePositionalArgs([VALID_TRACE_ID]); - expect(result.traceId).toBe(VALID_TRACE_ID); - expect(result.orgArg).toBeUndefined(); - }); - - test("normalizes mixed-case hex trace ID to lowercase", () => { - const mixedCase = "AAAA1111bbbb2222CCCC3333dddd4444"; - const result = parsePositionalArgs([mixedCase]); - expect(result.traceId).toBe(mixedCase.toLowerCase()); - expect(result.orgArg).toBeUndefined(); - }); - }); - - describe("single argument — slash-separated org/traceId", () => { - test("parses 'org/traceId' as org + trace ID", () => { - const result = parsePositionalArgs([`my-org/${VALID_TRACE_ID}`]); - expect(result.orgArg).toBe("my-org"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("throws ContextError when trace ID is missing after slash", () => { - expect(() => parsePositionalArgs(["my-org/"])).toThrow(ContextError); - }); - - test("throws ContextError when org is missing before slash", () => { - expect(() => parsePositionalArgs([`/${VALID_TRACE_ID}`])).toThrow( - ContextError - ); - }); - }); - - describe("two arguments — space-separated org and trace ID", () => { - test("parses org and trace ID from two args", () => { - const result = parsePositionalArgs(["my-org", VALID_TRACE_ID]); - expect(result.orgArg).toBe("my-org"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("uses first arg as org and second as trace ID", () => { - const result = parsePositionalArgs(["acme-corp", VALID_TRACE_ID]); - expect(result.orgArg).toBe("acme-corp"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - }); - - describe("trace ID validation", () => { - test("throws ValidationError for non-hex trace ID", () => { - expect(() => parsePositionalArgs(["not-a-valid-trace-id"])).toThrow( - ValidationError - ); - }); - - test("throws ValidationError for trace ID shorter than 32 chars", () => { - expect(() => parsePositionalArgs(["abc123"])).toThrow(ValidationError); - }); - - test("throws ValidationError for trace ID longer than 32 chars", () => { - expect(() => parsePositionalArgs([`${VALID_TRACE_ID}extra`])).toThrow( - ValidationError - ); - }); - - test("throws ValidationError for trace ID with non-hex chars", () => { - expect(() => - parsePositionalArgs(["aaaa1111bbbb2222cccc3333ddddgggg"]) - ).toThrow(ValidationError); - }); - - test("ValidationError mentions the invalid trace ID", () => { - try { - parsePositionalArgs(["short-id"]); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ValidationError); - expect((error as ValidationError).message).toContain("short-id"); - } - }); - - test("validates trace ID in two-arg form as well", () => { - expect(() => parsePositionalArgs(["my-org", "not-valid-trace"])).toThrow( - ValidationError - ); - }); - - test("validates trace ID in slash-separated form", () => { - expect(() => parsePositionalArgs(["my-org/short-id"])).toThrow( - ValidationError - ); - }); - }); -}); - -// ============================================================================ -// logsCommand.func() -// ============================================================================ +// Note: parseTraceTarget parsing tests are in test/lib/trace-target.test.ts const TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; const ORG = "test-org"; @@ -195,7 +73,7 @@ function createMockContext() { stdout: { write: stdoutWrite }, cwd: "/tmp", setContext: mock(() => { - // no-op for test + /* no-op for test */ }), }, stdoutWrite, diff --git a/test/commands/trace/view.property.test.ts b/test/commands/trace/view.property.test.ts index b6fa2622..40991702 100644 --- a/test/commands/trace/view.property.test.ts +++ b/test/commands/trace/view.property.test.ts @@ -1,31 +1,32 @@ /** - * Property-Based Tests for Trace View Command + * Property-Based Tests for Trace Target Parsing * - * Uses fast-check to verify invariants of parsePositionalArgs() - * that should hold for any valid input. + * Uses fast-check to verify invariants of parseTraceTarget() + * that should hold for any valid input. These tests cover the + * shared abstraction used by trace view, span list, span view, + * and trace logs. */ import { describe, expect, test } from "bun:test"; import { - array, assert as fcAssert, property, - string, stringMatching, tuple, } from "fast-check"; -import { parsePositionalArgs } from "../../../src/commands/trace/view.js"; import { ContextError, ValidationError } from "../../../src/lib/errors.js"; +import { parseTraceTarget } from "../../../src/lib/trace-target.js"; import { DEFAULT_NUM_RUNS } from "../../model-based/helpers.js"; /** Valid trace IDs (32-char hex) */ const traceIdArb = stringMatching(/^[a-f0-9]{32}$/); -/** Valid org/project slugs */ -const slugArb = stringMatching(/^[a-z][a-z0-9-]{1,20}[a-z0-9]$/); +/** Valid org/project slugs (no xn-- punycode prefix) */ +const slugArb = stringMatching(/^[a-z][a-z0-9-]{1,20}[a-z0-9]$/).filter( + (s) => !s.startsWith("xn--") +); -/** Non-empty strings for general args */ -const nonEmptyStringArb = string({ minLength: 1, maxLength: 50 }); +const HINT = "sentry trace view [//]"; /** * Insert dashes at UUID positions (8-4-4-4-12) into a 32-char hex string. @@ -34,85 +35,60 @@ function toUuidFormat(hex: string): string { return `${hex.slice(0, 8)}-${hex.slice(8, 12)}-${hex.slice(12, 16)}-${hex.slice(16, 20)}-${hex.slice(20)}`; } -describe("parsePositionalArgs properties", () => { - test("single valid trace ID: returns it as traceId with undefined targetArg", async () => { +describe("parseTraceTarget properties", () => { + test("single valid trace ID: returns auto-detect with correct traceId", async () => { await fcAssert( property(traceIdArb, (input) => { - const result = parsePositionalArgs([input]); + const result = parseTraceTarget([input], HINT); + expect(result.type).toBe("auto-detect"); expect(result.traceId).toBe(input); - expect(result.targetArg).toBeUndefined(); - expect(result.warning).toBeUndefined(); }), { numRuns: DEFAULT_NUM_RUNS } ); }); - test("single arg org/project/traceId: splits into target and traceId", async () => { + test("single arg org/project/traceId: returns explicit with correct fields", async () => { await fcAssert( property( tuple(slugArb, slugArb, traceIdArb), ([org, project, traceId]) => { const combined = `${org}/${project}/${traceId}`; - const result = parsePositionalArgs([combined]); - expect(result.targetArg).toBe(`${org}/${project}`); + const result = parseTraceTarget([combined], HINT); + expect(result.type).toBe("explicit"); expect(result.traceId).toBe(traceId); + if (result.type === "explicit") { + expect(result.org).toBe(org); + expect(result.project).toBe(project); + } } ), { numRuns: DEFAULT_NUM_RUNS } ); }); - test("single arg with one slash: throws ContextError (missing trace ID)", async () => { + test("single arg org/traceId: returns org-scoped", async () => { await fcAssert( - property(tuple(slugArb, slugArb), ([org, project]) => { - expect(() => parsePositionalArgs([`${org}/${project}`])).toThrow( - ContextError - ); - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); - - test("two args with valid trace ID: first is targetArg, second is traceId", async () => { - await fcAssert( - property(tuple(nonEmptyStringArb, traceIdArb), ([first, traceId]) => { - const result = parsePositionalArgs([first, traceId]); - expect(result.targetArg).toBe(first); + property(tuple(slugArb, traceIdArb), ([org, traceId]) => { + const combined = `${org}/${traceId}`; + const result = parseTraceTarget([combined], HINT); + expect(result.type).toBe("org-scoped"); expect(result.traceId).toBe(traceId); + if (result.type === "org-scoped") { + expect(result.org).toBe(org); + } }), { numRuns: DEFAULT_NUM_RUNS } ); }); - test("org/project target format: correctly splits target and traceId", async () => { + test("two args: target + trace-id uses second arg as trace ID", async () => { await fcAssert( property( tuple(slugArb, slugArb, traceIdArb), ([org, project, traceId]) => { const target = `${org}/${project}`; - const result = parsePositionalArgs([target, traceId]); - - expect(result.targetArg).toBe(target); - expect(result.traceId).toBe(traceId); - } - ), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); - - test("extra args are ignored: only first two matter", async () => { - await fcAssert( - property( - tuple( - nonEmptyStringArb, - traceIdArb, - array(nonEmptyStringArb, { minLength: 1, maxLength: 5 }) - ), - ([first, traceId, extras]) => { - const args = [first, traceId, ...extras]; - const result = parsePositionalArgs(args); - - expect(result.targetArg).toBe(first); + const result = parseTraceTarget([target, traceId], HINT); + expect(result.type).toBe("explicit"); expect(result.traceId).toBe(traceId); } ), @@ -120,12 +96,12 @@ describe("parsePositionalArgs properties", () => { ); }); - test("parsing is deterministic: same input always produces same output", async () => { + test("parsing is deterministic", async () => { await fcAssert( property(tuple(slugArb, traceIdArb), ([target, traceId]) => { const args = [target, traceId]; - const result1 = parsePositionalArgs(args); - const result2 = parsePositionalArgs(args); + const result1 = parseTraceTarget(args, HINT); + const result2 = parseTraceTarget(args, HINT); expect(result1).toEqual(result2); }), { numRuns: DEFAULT_NUM_RUNS } @@ -133,13 +109,13 @@ describe("parsePositionalArgs properties", () => { }); test("empty args always throws ContextError", () => { - expect(() => parsePositionalArgs([])).toThrow(ContextError); + expect(() => parseTraceTarget([], HINT)).toThrow(ContextError); }); - test("result always has traceId property defined (valid inputs)", async () => { + test("result always has traceId property defined", async () => { await fcAssert( property(traceIdArb, (traceId) => { - const result = parsePositionalArgs([traceId]); + const result = parseTraceTarget([traceId], HINT); expect(result.traceId).toBeDefined(); expect(typeof result.traceId).toBe("string"); }), @@ -151,20 +127,8 @@ describe("parsePositionalArgs properties", () => { await fcAssert( property(traceIdArb, (hex) => { const uuid = toUuidFormat(hex); - const result = parsePositionalArgs([uuid]); - expect(result.traceId).toBe(hex); - }), - { numRuns: DEFAULT_NUM_RUNS } - ); - }); - - test("UUID-format trace IDs work in two-arg case", async () => { - await fcAssert( - property(tuple(slugArb, traceIdArb), ([target, hex]) => { - const uuid = toUuidFormat(hex); - const result = parsePositionalArgs([target, uuid]); + const result = parseTraceTarget([uuid], HINT); expect(result.traceId).toBe(hex); - expect(result.targetArg).toBe(target); }), { numRuns: DEFAULT_NUM_RUNS } ); @@ -174,7 +138,7 @@ describe("parsePositionalArgs properties", () => { const invalidIdArb = stringMatching(/^[g-z]{10,20}$/); await fcAssert( property(invalidIdArb, (badId) => { - expect(() => parsePositionalArgs([badId])).toThrow(ValidationError); + expect(() => parseTraceTarget([badId], HINT)).toThrow(ValidationError); }), { numRuns: DEFAULT_NUM_RUNS } ); diff --git a/test/commands/trace/view.test.ts b/test/commands/trace/view.test.ts index 0daad540..9f52453a 100644 --- a/test/commands/trace/view.test.ts +++ b/test/commands/trace/view.test.ts @@ -1,169 +1,57 @@ /** * Trace View Command Tests * - * Tests for positional argument parsing and project resolution - * in src/commands/trace/view.ts + * Tests for pre-processing (swap detection, issue ID detection) + * and project resolution in src/commands/trace/view.ts. + * + * Note: Core trace target parsing tests are in test/lib/trace-target.test.ts. */ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; -import { parsePositionalArgs } from "../../../src/commands/trace/view.js"; +import { preProcessArgs } from "../../../src/commands/trace/view.js"; import type { ProjectWithOrg } from "../../../src/lib/api-client.js"; // biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking import * as apiClient from "../../../src/lib/api-client.js"; -import { - ContextError, - ResolutionError, - ValidationError, -} from "../../../src/lib/errors.js"; +import { ResolutionError, ValidationError } from "../../../src/lib/errors.js"; import { resolveProjectBySlug } from "../../../src/lib/resolve-target.js"; const VALID_TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; -const VALID_TRACE_ID_2 = "deadbeef12345678deadbeef12345678"; -const VALID_UUID = "ed29abc8-71c4-475b-9675-4655ef1a02d0"; -const VALID_UUID_STRIPPED = "ed29abc871c4475b96754655ef1a02d0"; - -describe("parsePositionalArgs", () => { - describe("single argument (trace ID only)", () => { - test("parses single arg as trace ID", () => { - const result = parsePositionalArgs([VALID_TRACE_ID]); - expect(result.traceId).toBe(VALID_TRACE_ID); - expect(result.targetArg).toBeUndefined(); - }); - test("parses 32-char hex trace ID", () => { - const result = parsePositionalArgs([VALID_TRACE_ID]); - expect(result.traceId).toBe(VALID_TRACE_ID); - expect(result.targetArg).toBeUndefined(); - }); - - test("normalizes uppercase trace ID to lowercase", () => { - const result = parsePositionalArgs(["AAAA1111BBBB2222CCCC3333DDDD4444"]); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); +describe("preProcessArgs", () => { + test("returns args unchanged for single arg", () => { + const result = preProcessArgs([VALID_TRACE_ID]); + expect(result.correctedArgs).toEqual([VALID_TRACE_ID]); + expect(result.warning).toBeUndefined(); + expect(result.suggestion).toBeUndefined(); }); - describe("two arguments (target + trace ID)", () => { - test("parses org/project target and trace ID", () => { - const result = parsePositionalArgs(["my-org/frontend", VALID_TRACE_ID]); - expect(result.targetArg).toBe("my-org/frontend"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("parses project-only target and trace ID", () => { - const result = parsePositionalArgs(["frontend", VALID_TRACE_ID]); - expect(result.targetArg).toBe("frontend"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); + test("returns args unchanged for normal two-arg order", () => { + const result = preProcessArgs(["my-org/frontend", VALID_TRACE_ID]); + expect(result.correctedArgs).toEqual(["my-org/frontend", VALID_TRACE_ID]); + expect(result.warning).toBeUndefined(); }); - describe("error cases", () => { - test("throws ContextError for empty args", () => { - expect(() => parsePositionalArgs([])).toThrow(ContextError); - }); - - test("throws ContextError with usage hint", () => { - try { - parsePositionalArgs([]); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ContextError); - expect((error as ContextError).message).toContain("Trace ID"); - } - }); - - test("throws ValidationError for invalid trace ID", () => { - expect(() => parsePositionalArgs(["not-a-valid-trace-id"])).toThrow( - ValidationError - ); - }); - - test("throws ValidationError for short hex", () => { - expect(() => parsePositionalArgs(["abc123"])).toThrow(ValidationError); - }); - - test("throws ValidationError for empty trace ID in two-arg case", () => { - expect(() => parsePositionalArgs(["my-org/frontend", ""])).toThrow( - ValidationError - ); - }); + test("detects swapped args and corrects order", () => { + // User put trace-id first, org/project second + const result = preProcessArgs([VALID_TRACE_ID, "my-org/frontend"]); + expect(result.correctedArgs).toEqual(["my-org/frontend", VALID_TRACE_ID]); + expect(result.warning).toContain("reversed"); }); - describe("slash-separated org/project/traceId (single arg)", () => { - test("parses org/project/traceId as target + trace ID", () => { - const result = parsePositionalArgs([`sentry/cli/${VALID_TRACE_ID}`]); - expect(result.targetArg).toBe("sentry/cli"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - - test("handles hyphenated org and project slugs", () => { - const result = parsePositionalArgs([ - `my-org/my-project/${VALID_TRACE_ID_2}`, - ]); - expect(result.targetArg).toBe("my-org/my-project"); - expect(result.traceId).toBe(VALID_TRACE_ID_2); - }); - - test("one slash (org/project, missing trace ID) throws ContextError", () => { - expect(() => parsePositionalArgs(["sentry/cli"])).toThrow(ContextError); - }); - - test("trailing slash (org/project/) throws ContextError", () => { - expect(() => parsePositionalArgs(["sentry/cli/"])).toThrow(ContextError); - }); - - test("one-slash ContextError mentions Trace ID", () => { - try { - parsePositionalArgs(["sentry/cli"]); - expect.unreachable("Should have thrown"); - } catch (error) { - expect(error).toBeInstanceOf(ContextError); - expect((error as ContextError).message).toContain("Trace ID"); - } - }); + test("detects issue short ID and suggests issue view", () => { + const result = preProcessArgs(["CAM-82X", VALID_TRACE_ID]); + expect(result.correctedArgs).toEqual(["CAM-82X", VALID_TRACE_ID]); + expect(result.suggestion).toContain("sentry issue view CAM-82X"); }); - describe("edge cases", () => { - test("handles more than two args (ignores extras)", () => { - const result = parsePositionalArgs([ - "my-org/frontend", - VALID_TRACE_ID, - "extra-arg", - ]); - expect(result.targetArg).toBe("my-org/frontend"); - expect(result.traceId).toBe(VALID_TRACE_ID); - }); - }); - - describe("UUID auto-correction", () => { - test("strips dashes from UUID trace ID (single arg)", () => { - const result = parsePositionalArgs([VALID_UUID]); - expect(result.traceId).toBe(VALID_UUID_STRIPPED); - expect(result.targetArg).toBeUndefined(); - }); - - test("strips dashes from UUID trace ID (two-arg case)", () => { - const result = parsePositionalArgs(["my-org/frontend", VALID_UUID]); - expect(result.traceId).toBe(VALID_UUID_STRIPPED); - expect(result.targetArg).toBe("my-org/frontend"); - }); - - test("strips dashes from UUID in slash-separated form", () => { - const result = parsePositionalArgs([`sentry/cli/${VALID_UUID}`]); - expect(result.traceId).toBe(VALID_UUID_STRIPPED); - expect(result.targetArg).toBe("sentry/cli"); - }); - - test("handles real user input from CLI-7Z", () => { - const result = parsePositionalArgs([ - "ed29abc8-71c4-475b-9675-4655ef1a02d0", - ]); - expect(result.traceId).toBe("ed29abc871c4475b96754655ef1a02d0"); - }); + test("returns empty args unchanged", () => { + const result = preProcessArgs([]); + expect(result.correctedArgs).toEqual([]); }); }); describe("resolveProjectBySlug", () => { - const HINT = "sentry trace view / "; + const HINT = "sentry trace view [//]"; let findProjectsBySlugSpy: ReturnType; beforeEach(() => { @@ -197,7 +85,6 @@ describe("resolveProjectBySlug", () => { expect((error as ResolutionError).message).toContain( "Check that you have access" ); - // Message says "not found", not "is required" expect((error as ResolutionError).message).toContain("not found"); } }); @@ -221,8 +108,18 @@ describe("resolveProjectBySlug", () => { test("includes all orgs and trace ID in error message", async () => { findProjectsBySlugSpy.mockResolvedValue({ projects: [ - { slug: "frontend", orgSlug: "acme-corp", id: "1", name: "Frontend" }, - { slug: "frontend", orgSlug: "beta-inc", id: "2", name: "Frontend" }, + { + slug: "frontend", + orgSlug: "acme-corp", + id: "1", + name: "Frontend", + }, + { + slug: "frontend", + orgSlug: "beta-inc", + id: "2", + name: "Frontend", + }, ] as ProjectWithOrg[], orgs: [], }); @@ -249,7 +146,12 @@ describe("resolveProjectBySlug", () => { test("returns resolved target for single match", async () => { findProjectsBySlugSpy.mockResolvedValue({ projects: [ - { slug: "backend", orgSlug: "my-company", id: "42", name: "Backend" }, + { + slug: "backend", + orgSlug: "my-company", + id: "42", + name: "Backend", + }, ] as ProjectWithOrg[], orgs: [], }); diff --git a/test/lib/trace-target.test.ts b/test/lib/trace-target.test.ts new file mode 100644 index 00000000..f33422f7 --- /dev/null +++ b/test/lib/trace-target.test.ts @@ -0,0 +1,170 @@ +/** + * Tests for shared trace-target parsing and resolution. + * + * Tests parseTraceTarget, parseSlashSeparatedTraceTarget, + * and targetArgToTraceTarget from src/lib/trace-target.ts. + */ + +import { describe, expect, test } from "bun:test"; +import { ContextError, ValidationError } from "../../src/lib/errors.js"; +import { + parseSlashSeparatedTraceTarget, + parseTraceTarget, + targetArgToTraceTarget, +} from "../../src/lib/trace-target.js"; + +const VALID_TRACE_ID = "aaaa1111bbbb2222cccc3333dddd4444"; + +describe("parseSlashSeparatedTraceTarget", () => { + const HINT = "sentry span list [//]"; + + test("bare trace ID → auto-detect", () => { + const result = parseSlashSeparatedTraceTarget(VALID_TRACE_ID, HINT); + expect(result.type).toBe("auto-detect"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("normalizes uppercase trace ID", () => { + const result = parseSlashSeparatedTraceTarget( + "AAAA1111BBBB2222CCCC3333DDDD4444", + HINT + ); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("strips UUID dashes from trace ID", () => { + const result = parseSlashSeparatedTraceTarget( + "aaaa1111-bbbb-2222-cccc-3333dddd4444", + HINT + ); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("org/trace-id → org-scoped", () => { + const result = parseSlashSeparatedTraceTarget( + `my-org/${VALID_TRACE_ID}`, + HINT + ); + expect(result.type).toBe("org-scoped"); + expect(result.traceId).toBe(VALID_TRACE_ID); + if (result.type === "org-scoped") { + expect(result.org).toBe("my-org"); + } + }); + + test("org/project/trace-id → explicit", () => { + const result = parseSlashSeparatedTraceTarget( + `my-org/my-project/${VALID_TRACE_ID}`, + HINT + ); + expect(result.type).toBe("explicit"); + expect(result.traceId).toBe(VALID_TRACE_ID); + if (result.type === "explicit") { + expect(result.org).toBe("my-org"); + expect(result.project).toBe("my-project"); + } + }); + + test("normalizes underscores in slugs", () => { + const result = parseSlashSeparatedTraceTarget( + `my_org/my_project/${VALID_TRACE_ID}`, + HINT + ); + expect(result.type).toBe("explicit"); + if (result.type === "explicit") { + expect(result.org).toBe("my-org"); + expect(result.project).toBe("my-project"); + expect(result.normalized).toBe(true); + } + }); + + test("trailing slash without trace ID throws", () => { + expect(() => parseSlashSeparatedTraceTarget("my-org/", HINT)).toThrow( + ContextError + ); + }); + + test("invalid trace ID throws ValidationError", () => { + expect(() => + parseSlashSeparatedTraceTarget("not-a-trace-id", HINT) + ).toThrow(ValidationError); + }); +}); + +describe("targetArgToTraceTarget", () => { + test("explicit target (org/project)", () => { + const result = targetArgToTraceTarget("my-org/my-project", VALID_TRACE_ID); + expect(result.type).toBe("explicit"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("org-all target (org/) → org-scoped", () => { + const result = targetArgToTraceTarget("my-org/", VALID_TRACE_ID); + expect(result.type).toBe("org-scoped"); + if (result.type === "org-scoped") { + expect(result.org).toBe("my-org"); + } + }); + + test("bare slug → project-search", () => { + const result = targetArgToTraceTarget("frontend", VALID_TRACE_ID); + expect(result.type).toBe("project-search"); + if (result.type === "project-search") { + expect(result.projectSlug).toBe("frontend"); + } + }); + + test("empty string → auto-detect", () => { + const result = targetArgToTraceTarget("", VALID_TRACE_ID); + expect(result.type).toBe("auto-detect"); + }); +}); + +describe("parseTraceTarget", () => { + const HINT = "sentry span list [//]"; + + test("empty args throws ContextError", () => { + expect(() => parseTraceTarget([], HINT)).toThrow(ContextError); + }); + + test("single arg: bare trace ID → auto-detect", () => { + const result = parseTraceTarget([VALID_TRACE_ID], HINT); + expect(result.type).toBe("auto-detect"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("single arg: org/project/trace-id → explicit", () => { + const result = parseTraceTarget( + [`my-org/my-project/${VALID_TRACE_ID}`], + HINT + ); + expect(result.type).toBe("explicit"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("single arg: org/trace-id → org-scoped", () => { + const result = parseTraceTarget([`my-org/${VALID_TRACE_ID}`], HINT); + expect(result.type).toBe("org-scoped"); + }); + + test("two args: target + trace-id", () => { + const result = parseTraceTarget( + ["my-org/my-project", VALID_TRACE_ID], + HINT + ); + expect(result.type).toBe("explicit"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("two args: bare slug + trace-id → project-search", () => { + const result = parseTraceTarget(["frontend", VALID_TRACE_ID], HINT); + expect(result.type).toBe("project-search"); + expect(result.traceId).toBe(VALID_TRACE_ID); + }); + + test("invalid trace ID throws ValidationError", () => { + expect(() => parseTraceTarget(["not-valid"], HINT)).toThrow( + ValidationError + ); + }); +});