Skip to content

Commit f1484dd

Browse files
committed
refactor(profile): replace --org/--project flags with positional args
Migrate profile view command from --org/--project flags to use the <org>/<project> positional argument syntax for consistency with other commands. Changes: - profile view now uses: sentry profile view [<org>/<project>] <transaction> - Export parsePositionalArgs for unit testing - Add unit tests for parsePositionalArgs - Regenerate SKILL.md
1 parent 86c2ab1 commit f1484dd

File tree

3 files changed

+251
-43
lines changed

3 files changed

+251
-43
lines changed

plugins/sentry-cli/skills/sentry-cli/SKILL.md

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -410,13 +410,11 @@ List transactions with profiling data
410410
- `-n, --limit <value> - Maximum number of transactions to return - (default: "20")`
411411
- `--json - Output as JSON`
412412

413-
#### `sentry profile view <transaction>`
413+
#### `sentry profile view <args...>`
414414

415415
View CPU profiling analysis for a transaction
416416

417417
**Flags:**
418-
- `--org <value> - Organization slug`
419-
- `--project <value> - Project slug`
420418
- `--period <value> - Stats period: 1h, 24h, 7d, 14d, 30d - (default: "7d")`
421419
- `-n, --limit <value> - Number of hot paths to show (max 20) - (default: "10")`
422420
- `--allFrames - Include library/system frames (default: user code only)`

src/commands/profile/view.ts

Lines changed: 147 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,15 @@
77

88
import { buildCommand, numberParser } from "@stricli/core";
99
import type { SentryContext } from "../../context.js";
10-
import { getFlamegraph, getProject } from "../../lib/api-client.js";
10+
import {
11+
findProjectsBySlug,
12+
getFlamegraph,
13+
getProject,
14+
} from "../../lib/api-client.js";
15+
import {
16+
ProjectSpecificationType,
17+
parseOrgProjectArg,
18+
} from "../../lib/arg-parsing.js";
1119
import { openInBrowser } from "../../lib/browser.js";
1220
import { ContextError } from "../../lib/errors.js";
1321
import {
@@ -24,8 +32,6 @@ import { resolveTransaction } from "../../lib/resolve-transaction.js";
2432
import { buildProfileUrl } from "../../lib/sentry-urls.js";
2533

2634
type ViewFlags = {
27-
readonly org?: string;
28-
readonly project?: string;
2935
readonly period: string;
3036
readonly limit: number;
3137
readonly allFrames: boolean;
@@ -36,6 +42,9 @@ type ViewFlags = {
3642
/** Valid period values */
3743
const VALID_PERIODS = ["1h", "24h", "7d", "14d", "30d"];
3844

45+
/** Usage hint for ContextError messages */
46+
const USAGE_HINT = "sentry profile view <org>/<project> <transaction>";
47+
3948
/**
4049
* Parse and validate the stats period.
4150
*/
@@ -48,6 +57,91 @@ function parsePeriod(value: string): string {
4857
return value;
4958
}
5059

60+
/**
61+
* Parse positional arguments for profile view.
62+
* Handles: `<transaction>` or `<target> <transaction>`
63+
*
64+
* @returns Parsed transaction and optional target arg
65+
*/
66+
export function parsePositionalArgs(args: string[]): {
67+
transactionRef: string;
68+
targetArg: string | undefined;
69+
} {
70+
if (args.length === 0) {
71+
throw new ContextError("Transaction name or alias", USAGE_HINT);
72+
}
73+
74+
const first = args[0];
75+
if (first === undefined) {
76+
throw new ContextError("Transaction name or alias", USAGE_HINT);
77+
}
78+
79+
if (args.length === 1) {
80+
// Single arg - must be transaction reference
81+
return { transactionRef: first, targetArg: undefined };
82+
}
83+
84+
const second = args[1];
85+
if (second === undefined) {
86+
// Should not happen given length check, but TypeScript needs this
87+
return { transactionRef: first, targetArg: undefined };
88+
}
89+
90+
// Two or more args - first is target, second is transaction
91+
return { transactionRef: second, targetArg: first };
92+
}
93+
94+
/** Resolved target type for internal use */
95+
type ResolvedProfileTarget = {
96+
org: string;
97+
project: string;
98+
orgDisplay: string;
99+
projectDisplay: string;
100+
detectedFrom?: string;
101+
};
102+
103+
/**
104+
* Resolve target from a project search result.
105+
*/
106+
async function resolveFromProjectSearch(
107+
projectSlug: string,
108+
transactionRef: string
109+
): Promise<ResolvedProfileTarget> {
110+
const found = await findProjectsBySlug(projectSlug);
111+
if (found.length === 0) {
112+
throw new ContextError(`Project "${projectSlug}"`, USAGE_HINT, [
113+
"Check that you have access to a project with this slug",
114+
]);
115+
}
116+
if (found.length > 1) {
117+
const alternatives = found.map(
118+
(p) => `${p.organization?.slug ?? "unknown"}/${p.slug}`
119+
);
120+
throw new ContextError(
121+
`Project "${projectSlug}" exists in multiple organizations`,
122+
`sentry profile view <org>/${projectSlug} ${transactionRef}`,
123+
alternatives
124+
);
125+
}
126+
const foundProject = found[0];
127+
if (!foundProject) {
128+
throw new ContextError(`Project "${projectSlug}" not found`, USAGE_HINT);
129+
}
130+
const orgSlug = foundProject.organization?.slug;
131+
if (!orgSlug) {
132+
throw new ContextError(
133+
`Could not determine organization for project "${projectSlug}"`,
134+
USAGE_HINT
135+
);
136+
}
137+
return {
138+
org: orgSlug,
139+
project: foundProject.slug,
140+
orgDisplay: orgSlug,
141+
projectDisplay: foundProject.slug,
142+
};
143+
}
144+
51145
export const viewCommand = buildCommand({
52146
docs: {
53147
brief: "View CPU profiling analysis for a transaction",
@@ -58,36 +152,22 @@ export const viewCommand = buildCommand({
58152
" - Hot paths (functions consuming the most CPU time)\n" +
59153
" - Recommendations for optimization\n\n" +
60154
"By default, only user application code is shown. Use --all-frames to include library code.\n\n" +
61-
"The organization and project are resolved from:\n" +
62-
" 1. --org and --project flags\n" +
63-
" 2. Config defaults\n" +
64-
" 3. SENTRY_DSN environment variable or source code detection",
155+
"Target specification:\n" +
156+
" sentry profile view <transaction> # auto-detect from DSN or config\n" +
157+
" sentry profile view <org>/<proj> <transaction> # explicit org and project\n" +
158+
" sentry profile view <project> <transaction> # find project across all orgs",
65159
},
66160
parameters: {
67161
positional: {
68-
kind: "tuple",
69-
parameters: [
70-
{
71-
placeholder: "transaction",
72-
brief:
73-
'Transaction: index (1), alias (i), or full name ("/api/users")',
74-
parse: String,
75-
},
76-
],
77-
},
78-
flags: {
79-
org: {
80-
kind: "parsed",
81-
parse: String,
82-
brief: "Organization slug",
83-
optional: true,
84-
},
85-
project: {
86-
kind: "parsed",
162+
kind: "array",
163+
parameter: {
164+
placeholder: "args",
165+
brief:
166+
'[<org>/<project>] <transaction> - Target (optional) and transaction (required). Transaction can be index (1), alias (i), or full name ("/api/users")',
87167
parse: String,
88-
brief: "Project slug",
89-
optional: true,
90168
},
169+
},
170+
flags: {
91171
period: {
92172
kind: "parsed",
93173
parse: parsePeriod,
@@ -121,23 +201,50 @@ export const viewCommand = buildCommand({
121201
async func(
122202
this: SentryContext,
123203
flags: ViewFlags,
124-
transactionRef: string
204+
...args: string[]
125205
): Promise<void> {
126206
const { stdout, cwd, setContext } = this;
127207

128-
// Resolve org and project from flags or detection
129-
const target = await resolveOrgAndProject({
130-
org: flags.org,
131-
project: flags.project,
132-
cwd,
133-
usageHint: `sentry profile view "${transactionRef}" --org <org> --project <project>`,
134-
});
208+
// Parse positional args
209+
const { transactionRef, targetArg } = parsePositionalArgs(args);
210+
const parsed = parseOrgProjectArg(targetArg);
211+
212+
let target: ResolvedProfileTarget | null = null;
213+
214+
switch (parsed.type) {
215+
case ProjectSpecificationType.Explicit:
216+
target = {
217+
org: parsed.org,
218+
project: parsed.project,
219+
orgDisplay: parsed.org,
220+
projectDisplay: parsed.project,
221+
};
222+
break;
223+
224+
case ProjectSpecificationType.ProjectSearch:
225+
target = await resolveFromProjectSearch(
226+
parsed.projectSlug,
227+
transactionRef
228+
);
229+
break;
230+
231+
case ProjectSpecificationType.OrgAll:
232+
throw new ContextError(
233+
"A specific project is required for profile view",
234+
USAGE_HINT
235+
);
236+
237+
case ProjectSpecificationType.AutoDetect:
238+
target = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
239+
break;
240+
241+
default:
242+
// Exhaustive check - should never reach here
243+
throw new ContextError("Invalid target specification", USAGE_HINT);
244+
}
135245

136246
if (!target) {
137-
throw new ContextError(
138-
"Organization and project",
139-
`sentry profile view "${transactionRef}" --org <org-slug> --project <project-slug>`
140-
);
247+
throw new ContextError("Organization and project", USAGE_HINT);
141248
}
142249

143250
// Resolve transaction reference (alias, index, or full name)

test/commands/profile/view.test.ts

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* Profile View Command Tests
3+
*
4+
* Tests for positional argument parsing in src/commands/profile/view.ts
5+
*/
6+
7+
import { describe, expect, test } from "bun:test";
8+
import { parsePositionalArgs } from "../../../src/commands/profile/view.js";
9+
import { ContextError } from "../../../src/lib/errors.js";
10+
11+
describe("parsePositionalArgs", () => {
12+
describe("single argument (transaction only)", () => {
13+
test("parses single arg as transaction name", () => {
14+
const result = parsePositionalArgs(["/api/users"]);
15+
expect(result.transactionRef).toBe("/api/users");
16+
expect(result.targetArg).toBeUndefined();
17+
});
18+
19+
test("parses transaction index", () => {
20+
const result = parsePositionalArgs(["1"]);
21+
expect(result.transactionRef).toBe("1");
22+
expect(result.targetArg).toBeUndefined();
23+
});
24+
25+
test("parses transaction alias", () => {
26+
const result = parsePositionalArgs(["a"]);
27+
expect(result.transactionRef).toBe("a");
28+
expect(result.targetArg).toBeUndefined();
29+
});
30+
31+
test("parses complex transaction name", () => {
32+
const result = parsePositionalArgs(["POST /api/v2/users/:id/settings"]);
33+
expect(result.transactionRef).toBe("POST /api/v2/users/:id/settings");
34+
expect(result.targetArg).toBeUndefined();
35+
});
36+
});
37+
38+
describe("two arguments (target + transaction)", () => {
39+
test("parses org/project target and transaction name", () => {
40+
const result = parsePositionalArgs(["my-org/backend", "/api/users"]);
41+
expect(result.targetArg).toBe("my-org/backend");
42+
expect(result.transactionRef).toBe("/api/users");
43+
});
44+
45+
test("parses project-only target and transaction", () => {
46+
const result = parsePositionalArgs(["backend", "/api/users"]);
47+
expect(result.targetArg).toBe("backend");
48+
expect(result.transactionRef).toBe("/api/users");
49+
});
50+
51+
test("parses org/ target (all projects) and transaction", () => {
52+
const result = parsePositionalArgs(["my-org/", "/api/users"]);
53+
expect(result.targetArg).toBe("my-org/");
54+
expect(result.transactionRef).toBe("/api/users");
55+
});
56+
57+
test("parses target and transaction index", () => {
58+
const result = parsePositionalArgs(["my-org/backend", "1"]);
59+
expect(result.targetArg).toBe("my-org/backend");
60+
expect(result.transactionRef).toBe("1");
61+
});
62+
63+
test("parses target and transaction alias", () => {
64+
const result = parsePositionalArgs(["my-org/backend", "a"]);
65+
expect(result.targetArg).toBe("my-org/backend");
66+
expect(result.transactionRef).toBe("a");
67+
});
68+
});
69+
70+
describe("error cases", () => {
71+
test("throws ContextError for empty args", () => {
72+
expect(() => parsePositionalArgs([])).toThrow(ContextError);
73+
});
74+
75+
test("throws ContextError with usage hint", () => {
76+
try {
77+
parsePositionalArgs([]);
78+
expect.unreachable("Should have thrown");
79+
} catch (error) {
80+
expect(error).toBeInstanceOf(ContextError);
81+
expect((error as ContextError).message).toContain("Transaction");
82+
}
83+
});
84+
});
85+
86+
describe("edge cases", () => {
87+
test("handles more than two args (ignores extras)", () => {
88+
const result = parsePositionalArgs([
89+
"my-org/backend",
90+
"/api/users",
91+
"extra-arg",
92+
]);
93+
expect(result.targetArg).toBe("my-org/backend");
94+
expect(result.transactionRef).toBe("/api/users");
95+
});
96+
97+
test("handles empty string transaction in two-arg case", () => {
98+
const result = parsePositionalArgs(["my-org/backend", ""]);
99+
expect(result.targetArg).toBe("my-org/backend");
100+
expect(result.transactionRef).toBe("");
101+
});
102+
});
103+
});

0 commit comments

Comments
 (0)