diff --git a/plugins/sentry-cli/skills/sentry-cli/SKILL.md b/plugins/sentry-cli/skills/sentry-cli/SKILL.md index b09fb373..cacd5f47 100644 --- a/plugins/sentry-cli/skills/sentry-cli/SKILL.md +++ b/plugins/sentry-cli/skills/sentry-cli/SKILL.md @@ -410,13 +410,11 @@ List transactions with profiling data - `-n, --limit - Maximum number of transactions to return - (default: "20")` - `--json - Output as JSON` -#### `sentry profile view ` +#### `sentry profile view ` View CPU profiling analysis for a transaction **Flags:** -- `--org - Organization slug` -- `--project - Project slug` - `--period - Stats period: 1h, 24h, 7d, 14d, 30d - (default: "7d")` - `-n, --limit - Number of hot paths to show (max 20) - (default: "10")` - `--allFrames - Include library/system frames (default: user code only)` diff --git a/src/commands/profile/view.ts b/src/commands/profile/view.ts index 6d9676db..d88614ac 100644 --- a/src/commands/profile/view.ts +++ b/src/commands/profile/view.ts @@ -7,7 +7,15 @@ import { buildCommand, numberParser } from "@stricli/core"; import type { SentryContext } from "../../context.js"; -import { getFlamegraph, getProject } from "../../lib/api-client.js"; +import { + findProjectsBySlug, + getFlamegraph, + getProject, +} from "../../lib/api-client.js"; +import { + ProjectSpecificationType, + parseOrgProjectArg, +} from "../../lib/arg-parsing.js"; import { openInBrowser } from "../../lib/browser.js"; import { ContextError } from "../../lib/errors.js"; import { @@ -24,8 +32,6 @@ import { resolveTransaction } from "../../lib/resolve-transaction.js"; import { buildProfileUrl } from "../../lib/sentry-urls.js"; type ViewFlags = { - readonly org?: string; - readonly project?: string; readonly period: string; readonly limit: number; readonly allFrames: boolean; @@ -36,6 +42,9 @@ type ViewFlags = { /** Valid period values */ const VALID_PERIODS = ["1h", "24h", "7d", "14d", "30d"]; +/** Usage hint for ContextError messages */ +const USAGE_HINT = "sentry profile view / "; + /** * Parse and validate the stats period. */ @@ -48,6 +57,91 @@ function parsePeriod(value: string): string { return value; } +/** + * Parse positional arguments for profile view. + * Handles: `` or ` ` + * + * @returns Parsed transaction and optional target arg + */ +export function parsePositionalArgs(args: string[]): { + transactionRef: string; + targetArg: string | undefined; +} { + if (args.length === 0) { + throw new ContextError("Transaction name or alias", USAGE_HINT); + } + + const first = args[0]; + if (first === undefined) { + throw new ContextError("Transaction name or alias", USAGE_HINT); + } + + if (args.length === 1) { + // Single arg - must be transaction reference + return { transactionRef: first, targetArg: undefined }; + } + + const second = args[1]; + if (second === undefined) { + // Should not happen given length check, but TypeScript needs this + return { transactionRef: first, targetArg: undefined }; + } + + // Two or more args - first is target, second is transaction + return { transactionRef: second, targetArg: first }; +} + +/** Resolved target type for internal use */ +type ResolvedProfileTarget = { + org: string; + project: string; + orgDisplay: string; + projectDisplay: string; + detectedFrom?: string; +}; + +/** + * Resolve target from a project search result. + */ +async function resolveFromProjectSearch( + projectSlug: string, + transactionRef: string +): Promise { + const found = await findProjectsBySlug(projectSlug); + if (found.length === 0) { + throw new ContextError(`Project "${projectSlug}"`, USAGE_HINT, [ + "Check that you have access to a project with this slug", + ]); + } + if (found.length > 1) { + const alternatives = found.map( + (p) => `${p.organization?.slug ?? "unknown"}/${p.slug}` + ); + throw new ContextError( + `Project "${projectSlug}" exists in multiple organizations`, + `sentry profile view /${projectSlug} ${transactionRef}`, + alternatives + ); + } + const foundProject = found[0]; + if (!foundProject) { + throw new ContextError(`Project "${projectSlug}" not found`, USAGE_HINT); + } + const orgSlug = foundProject.organization?.slug; + if (!orgSlug) { + throw new ContextError( + `Could not determine organization for project "${projectSlug}"`, + USAGE_HINT + ); + } + return { + org: orgSlug, + project: foundProject.slug, + orgDisplay: orgSlug, + projectDisplay: foundProject.slug, + }; +} + export const viewCommand = buildCommand({ docs: { brief: "View CPU profiling analysis for a transaction", @@ -58,36 +152,22 @@ export const viewCommand = buildCommand({ " - Hot paths (functions consuming the most CPU time)\n" + " - Recommendations for optimization\n\n" + "By default, only user application code is shown. Use --all-frames to include library code.\n\n" + - "The organization and project are resolved from:\n" + - " 1. --org and --project flags\n" + - " 2. Config defaults\n" + - " 3. SENTRY_DSN environment variable or source code detection", + "Target specification:\n" + + " sentry profile view # auto-detect from DSN or config\n" + + " sentry profile view / # explicit org and project\n" + + " sentry profile view # find project across all orgs", }, parameters: { positional: { - kind: "tuple", - parameters: [ - { - placeholder: "transaction", - brief: - 'Transaction: index (1), alias (i), or full name ("/api/users")', - parse: String, - }, - ], - }, - flags: { - org: { - kind: "parsed", - parse: String, - brief: "Organization slug", - optional: true, - }, - project: { - kind: "parsed", + kind: "array", + parameter: { + placeholder: "args", + brief: + '[/] - Target (optional) and transaction (required). Transaction can be index (1), alias (i), or full name ("/api/users")', parse: String, - brief: "Project slug", - optional: true, }, + }, + flags: { period: { kind: "parsed", parse: parsePeriod, @@ -121,23 +201,50 @@ export const viewCommand = buildCommand({ async func( this: SentryContext, flags: ViewFlags, - transactionRef: string + ...args: string[] ): Promise { const { stdout, cwd, setContext } = this; - // Resolve org and project from flags or detection - const target = await resolveOrgAndProject({ - org: flags.org, - project: flags.project, - cwd, - usageHint: `sentry profile view "${transactionRef}" --org --project `, - }); + // Parse positional args + const { transactionRef, targetArg } = parsePositionalArgs(args); + const parsed = parseOrgProjectArg(targetArg); + + let target: ResolvedProfileTarget | null = null; + + switch (parsed.type) { + case ProjectSpecificationType.Explicit: + target = { + org: parsed.org, + project: parsed.project, + orgDisplay: parsed.org, + projectDisplay: parsed.project, + }; + break; + + case ProjectSpecificationType.ProjectSearch: + target = await resolveFromProjectSearch( + parsed.projectSlug, + transactionRef + ); + break; + + case ProjectSpecificationType.OrgAll: + throw new ContextError( + "A specific project is required for profile view", + USAGE_HINT + ); + + case ProjectSpecificationType.AutoDetect: + target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT }); + break; + + default: + // Exhaustive check - should never reach here + throw new ContextError("Invalid target specification", USAGE_HINT); + } if (!target) { - throw new ContextError( - "Organization and project", - `sentry profile view "${transactionRef}" --org --project ` - ); + throw new ContextError("Organization and project", USAGE_HINT); } // Resolve transaction reference (alias, index, or full name) diff --git a/test/commands/profile/view.test.ts b/test/commands/profile/view.test.ts new file mode 100644 index 00000000..fa224ae1 --- /dev/null +++ b/test/commands/profile/view.test.ts @@ -0,0 +1,103 @@ +/** + * Profile View Command Tests + * + * Tests for positional argument parsing in src/commands/profile/view.ts + */ + +import { describe, expect, test } from "bun:test"; +import { parsePositionalArgs } from "../../../src/commands/profile/view.js"; +import { ContextError } from "../../../src/lib/errors.js"; + +describe("parsePositionalArgs", () => { + describe("single argument (transaction only)", () => { + test("parses single arg as transaction name", () => { + const result = parsePositionalArgs(["/api/users"]); + expect(result.transactionRef).toBe("/api/users"); + expect(result.targetArg).toBeUndefined(); + }); + + test("parses transaction index", () => { + const result = parsePositionalArgs(["1"]); + expect(result.transactionRef).toBe("1"); + expect(result.targetArg).toBeUndefined(); + }); + + test("parses transaction alias", () => { + const result = parsePositionalArgs(["a"]); + expect(result.transactionRef).toBe("a"); + expect(result.targetArg).toBeUndefined(); + }); + + test("parses complex transaction name", () => { + const result = parsePositionalArgs(["POST /api/v2/users/:id/settings"]); + expect(result.transactionRef).toBe("POST /api/v2/users/:id/settings"); + expect(result.targetArg).toBeUndefined(); + }); + }); + + describe("two arguments (target + transaction)", () => { + test("parses org/project target and transaction name", () => { + const result = parsePositionalArgs(["my-org/backend", "/api/users"]); + expect(result.targetArg).toBe("my-org/backend"); + expect(result.transactionRef).toBe("/api/users"); + }); + + test("parses project-only target and transaction", () => { + const result = parsePositionalArgs(["backend", "/api/users"]); + expect(result.targetArg).toBe("backend"); + expect(result.transactionRef).toBe("/api/users"); + }); + + test("parses org/ target (all projects) and transaction", () => { + const result = parsePositionalArgs(["my-org/", "/api/users"]); + expect(result.targetArg).toBe("my-org/"); + expect(result.transactionRef).toBe("/api/users"); + }); + + test("parses target and transaction index", () => { + const result = parsePositionalArgs(["my-org/backend", "1"]); + expect(result.targetArg).toBe("my-org/backend"); + expect(result.transactionRef).toBe("1"); + }); + + test("parses target and transaction alias", () => { + const result = parsePositionalArgs(["my-org/backend", "a"]); + expect(result.targetArg).toBe("my-org/backend"); + expect(result.transactionRef).toBe("a"); + }); + }); + + 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("Transaction"); + } + }); + }); + + describe("edge cases", () => { + test("handles more than two args (ignores extras)", () => { + const result = parsePositionalArgs([ + "my-org/backend", + "/api/users", + "extra-arg", + ]); + expect(result.targetArg).toBe("my-org/backend"); + expect(result.transactionRef).toBe("/api/users"); + }); + + test("handles empty string transaction in two-arg case", () => { + const result = parsePositionalArgs(["my-org/backend", ""]); + expect(result.targetArg).toBe("my-org/backend"); + expect(result.transactionRef).toBe(""); + }); + }); +});