Skip to content

Commit e9e4868

Browse files
committed
fix: improve argument parsing for common user mistakes
Address 5 ResolutionError patterns from production Sentry issues: - CLI-BG: Detect swapped view args (ID first, target second) and auto-correct with warning - CLI-BB: Parse multi-slash args like org/project/issueId correctly - CLI-BA: Normalize underscores to hyphens in project/org slugs - CLI-B9: Auto-redirect to org-all when bare slug matches an org name - CLI-B8: Detect issue short IDs in event/log/trace view and suggest the correct command Technical changes: - Add detectSwappedViewArgs(), looksLikeIssueShortId(), normalizeSlug() helpers to arg-parsing.ts - Add parseMultiSlashIssueArg() for org/project/id patterns - Change findProjectsBySlug() to return { projects, orgs } (was bare array) to enable org-slug matching without extra API calls - Add orgAllFallback option to handleProjectSearch for auto-redirect - Remove stderr parameter from resolveProjectBySlug and related functions — use Consola logger instead - Update all view commands (event, log, trace) with swap detection and issue-short-ID suggestions All 2726 unit tests pass. Typecheck and lint clean.
1 parent 328d262 commit e9e4868

28 files changed

+1219
-340
lines changed

AGENTS.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,9 @@ mock.module("./some-module", () => ({
648648
649649
### Decision
650650
651+
<!-- lore:019cc2ef-9be5-722d-bc9f-b07a8197eeed -->
652+
* **All view subcommands should use \<target> \<id> positional pattern**: All \`\* view\` subcommands (\`event view\`, \`log view\`, \`trace view\`, etc.) should follow a consistent \`\<target> \<id>\` positional argument pattern where target is the optional \`org/project\` specifier. During migration, use opportunistic argument swapping with a stderr warning when the user provides args in the wrong order. This follows the CLI UX auto-correction pattern: safe when input is already invalid, no ambiguity, and warning goes to stderr.
653+
651654
<!-- lore:019c99d5-69f2-74eb-8c86-411f8512801d -->
652655
* **Raw markdown output for non-interactive terminals, rendered for TTY**: Markdown-first output pipeline: custom renderer in \`src/lib/formatters/markdown.ts\` walks \`marked\` tokens to produce ANSI-styled output. Commands build CommonMark using helpers (\`mdKvTable()\`, \`mdRow()\`, \`colorTag()\`, \`escapeMarkdownCell()\`, \`safeCodeSpan()\`) and pass through \`renderMarkdown()\`. \`isPlainOutput()\` precedence: \`SENTRY\_PLAIN\_OUTPUT\` > \`NO\_COLOR\` > \`FORCE\_COLOR\` > \`!isTTY\`. \`--json\` always outputs JSON. Colors defined in \`COLORS\` object in \`colors.ts\`. Tests run non-TTY so assertions match raw CommonMark; use \`stripAnsi()\` helper for rendered-mode assertions.
653656
@@ -677,13 +680,16 @@ mock.module("./some-module", () => ({
677680
<!-- lore:019c9741-d78e-73b1-87c2-e360ef6c7475 -->
678681
* **useTestConfigDir without isolateProjectRoot causes DSN scanning of repo tree**: \`useTestConfigDir()\` creates temp dirs under \`.test-tmp/\` in the repo tree. Without \`{ isolateProjectRoot: true }\`, \`findProjectRoot\` walks up and finds the repo's \`.git\`, causing DSN detection to scan real source code and trigger network calls against test mocks (timeouts). Always pass \`isolateProjectRoot: true\` when tests exercise \`resolveOrg\`, \`detectDsn\`, or \`findProjectRoot\`.
679682
683+
<!-- lore:019cc303-e397-75b9-9762-6f6ad108f50a -->
684+
* **Zod z.coerce.number() converts null to 0 silently**: Zod gotchas in this codebase: (1) \`z.coerce.number()\` passes input through \`Number()\`, so \`null\` silently becomes \`0\`. Be aware if \`null\` vs \`0\` distinction matters. (2) Zod v4 \`.default({})\` short-circuits — it returns the default value without parsing through inner schema defaults. So \`.object({ enabled: z.boolean().default(true) }).default({})\` returns \`{}\`, not \`{ enabled: true }\`. Fix: provide fully-populated default objects. This affected nested config sections in src/config.ts during the v3→v4 upgrade.
685+
680686
### Pattern
681687
682688
<!-- lore:019c972c-9f11-7c0d-96ce-3f8cc2641175 -->
683689
* **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves.
684690
685691
<!-- lore:5ac4e219-ea1f-41cb-8e97-7e946f5848c0 -->
686-
* **PR workflow: wait for Seer and Cursor BugBot before resolving**: After pushing a PR in the getsentry/cli repo, the CI pipeline includes Seer Code Review and Cursor Bugbot as required or advisory checks. Both typically take 2-3 minutes. The workflow is: push → wait for all CI (including npm build jobs which test the actual bundle) → check for inline review comments from Seer/BugBot → fix if needed → repeat. Use \`gh pr checks \<PR> --watch\` to monitor. Review comments are fetched via \`gh api repos/OWNER/REPO/pulls/NUM/comments\` and \`gh api repos/OWNER/REPO/pulls/NUM/reviews\`.
692+
* **PR workflow: wait for Seer and Cursor BugBot before resolving**: After pushing a PR in the getsentry/cli repo, the CI pipeline includes Seer Code Review and Cursor Bugbot as advisory checks. Both typically take 2-3 minutes but may not trigger on draft PRs — only ready-for-review PRs reliably get bot reviews. The workflow is: push → wait for all CI (including npm build jobs which test the actual bundle) → check for inline review comments from Seer/BugBot → fix if needed → repeat. Use \`gh pr checks \<PR> --watch\` to monitor. Review comments are fetched via \`gh api repos/OWNER/REPO/pulls/NUM/comments\` and \`gh api repos/OWNER/REPO/pulls/NUM/reviews\`.
687693
688694
<!-- lore:019cb162-d3ad-7b05-ab4f-f87892d517a6 -->
689695
* **Shared pagination infrastructure: buildPaginationContextKey and parseCursorFlag**: List commands with cursor pagination use \`buildPaginationContextKey(type, identifier, flags)\` for composite context keys and \`parseCursorFlag(value)\` accepting \`"last"\` magic value. Critical: \`resolveCursor()\` must be called inside the \`org-all\` override closure, not before \`dispatchOrgScopedList\` — otherwise cursor validation errors fire before the correct mode-specific error.

src/commands/event/view.ts

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import {
1212
resolveEventInOrg,
1313
} from "../../lib/api-client.js";
1414
import {
15+
detectSwappedViewArgs,
16+
looksLikeIssueShortId,
1517
ProjectSpecificationType,
1618
parseOrgProjectArg,
1719
parseSlashSeparatedArg,
@@ -21,6 +23,7 @@ import { openInBrowser } from "../../lib/browser.js";
2123
import { buildCommand } from "../../lib/command.js";
2224
import { ContextError, ResolutionError } from "../../lib/errors.js";
2325
import { formatEventDetails, writeJson } from "../../lib/formatters/index.js";
26+
import { logger } from "../../lib/logger.js";
2427
import { resolveEffectiveOrg } from "../../lib/region.js";
2528
import {
2629
resolveOrgAndProject,
@@ -88,6 +91,10 @@ const USAGE_HINT = "sentry event view <org>/<project> <event-id>";
8891
export function parsePositionalArgs(args: string[]): {
8992
eventId: string;
9093
targetArg: string | undefined;
94+
/** Warning message if arguments appear to be in the wrong order */
95+
warning?: string;
96+
/** Suggestion when the user likely meant a different command */
97+
suggestion?: string;
9198
} {
9299
if (args.length === 0) {
93100
throw new ContextError("Event ID", USAGE_HINT);
@@ -130,8 +137,19 @@ export function parsePositionalArgs(args: string[]): {
130137
return { eventId: first, targetArg: undefined };
131138
}
132139

140+
// Detect swapped args: user put ID first and target second
141+
const swapWarning = detectSwappedViewArgs(first, second);
142+
if (swapWarning) {
143+
return { eventId: first, targetArg: second, warning: swapWarning };
144+
}
145+
146+
// Detect issue short ID passed as first arg (e.g., "CAM-82X 95fd7f5a")
147+
const suggestion = looksLikeIssueShortId(first)
148+
? `Did you mean: sentry issue view ${first}`
149+
: undefined;
150+
133151
// Two or more args - first is target, second is event ID
134-
return { eventId: second, targetArg: first };
152+
return { eventId: second, targetArg: first, suggestion };
135153
}
136154

137155
/**
@@ -153,7 +171,6 @@ type ResolveTargetOptions = {
153171
parsed: ReturnType<typeof parseOrgProjectArg>;
154172
eventId: string;
155173
cwd: string;
156-
stderr: { write(s: string): void };
157174
};
158175

159176
/**
@@ -166,7 +183,7 @@ type ResolveTargetOptions = {
166183
export async function resolveEventTarget(
167184
options: ResolveTargetOptions
168185
): Promise<ResolvedEventTarget | null> {
169-
const { parsed, eventId, cwd, stderr } = options;
186+
const { parsed, eventId, cwd } = options;
170187

171188
switch (parsed.type) {
172189
case ProjectSpecificationType.Explicit: {
@@ -183,8 +200,7 @@ export async function resolveEventTarget(
183200
const resolved = await resolveProjectBySlug(
184201
parsed.projectSlug,
185202
USAGE_HINT,
186-
`sentry event view <org>/${parsed.projectSlug} ${eventId}`,
187-
stderr
203+
`sentry event view <org>/${parsed.projectSlug} ${eventId}`
188204
);
189205
return {
190206
...resolved,
@@ -199,7 +215,7 @@ export async function resolveEventTarget(
199215
}
200216

201217
case ProjectSpecificationType.AutoDetect:
202-
return resolveAutoDetectTarget(eventId, cwd, stderr);
218+
return resolveAutoDetectTarget(eventId, cwd);
203219

204220
default:
205221
return null;
@@ -244,8 +260,7 @@ export async function resolveOrgAllTarget(
244260
/** @internal Exported for testing */
245261
export async function resolveAutoDetectTarget(
246262
eventId: string,
247-
cwd: string,
248-
stderr: { write(s: string): void }
263+
cwd: string
249264
): Promise<ResolvedEventTarget | null> {
250265
const autoTarget = await resolveOrgAndProject({ cwd, usageHint: USAGE_HINT });
251266
if (autoTarget) {
@@ -254,10 +269,12 @@ export async function resolveAutoDetectTarget(
254269

255270
const resolved = await findEventAcrossOrgs(eventId);
256271
if (resolved) {
257-
stderr.write(
258-
`Tip: Found event in ${resolved.org}/${resolved.project}. ` +
259-
`Use: sentry event view ${resolved.org}/${resolved.project} ${eventId}\n`
260-
);
272+
logger
273+
.withTag("event.view")
274+
.warn(
275+
`Found event in ${resolved.org}/${resolved.project}. ` +
276+
`Use: sentry event view ${resolved.org}/${resolved.project} ${eventId}`
277+
);
261278
return {
262279
org: resolved.org,
263280
project: resolved.project,
@@ -311,15 +328,26 @@ export const viewCommand = buildCommand({
311328
): Promise<void> {
312329
const { stdout, cwd } = this;
313330

331+
const log = logger.withTag("event.view");
332+
314333
// Parse positional args
315-
const { eventId, targetArg } = parsePositionalArgs(args);
334+
const { eventId, targetArg, warning, suggestion } =
335+
parsePositionalArgs(args);
336+
if (warning) {
337+
log.warn(warning);
338+
}
339+
if (suggestion) {
340+
log.warn(suggestion);
341+
}
316342
const parsed = parseOrgProjectArg(targetArg);
343+
if (parsed.type !== "auto-detect" && parsed.normalized) {
344+
log.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
345+
}
317346

318347
const target = await resolveEventTarget({
319348
parsed,
320349
eventId,
321350
cwd,
322-
stderr: this.stderr,
323351
});
324352

325353
if (!target) {

src/commands/issue/list.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,9 +354,25 @@ async function resolveTargetsFromParsedArg(
354354

355355
case "project-search": {
356356
// Find project across all orgs
357-
const matches = await findProjectsBySlug(parsed.projectSlug);
357+
const { projects: matches, orgs } = await findProjectsBySlug(
358+
parsed.projectSlug
359+
);
358360

359361
if (matches.length === 0) {
362+
// Check if the slug matches an organization — common mistake
363+
const isOrg = orgs.some((o) => o.slug === parsed.projectSlug);
364+
if (isOrg) {
365+
throw new ResolutionError(
366+
`'${parsed.projectSlug}'`,
367+
"is an organization, not a project",
368+
`sentry issue list ${parsed.projectSlug}/`,
369+
[
370+
`List projects: sentry project list ${parsed.projectSlug}/`,
371+
`Specify a project: sentry issue list ${parsed.projectSlug}/<project>`,
372+
]
373+
);
374+
}
375+
360376
throw new ResolutionError(
361377
`Project '${parsed.projectSlug}'`,
362378
"not found",

src/commands/issue/utils.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ async function resolveProjectSearch(
140140
}
141141

142142
// 2. Search for project across all accessible orgs
143-
const projects = await findProjectsBySlug(projectSlug.toLowerCase());
143+
const { projects } = await findProjectsBySlug(projectSlug.toLowerCase());
144144

145145
if (projects.length === 0) {
146146
throw new ResolutionError(

src/commands/log/view.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77
import type { SentryContext } from "../../context.js";
88
import { getLog } from "../../lib/api-client.js";
99
import {
10+
detectSwappedViewArgs,
1011
parseOrgProjectArg,
1112
parseSlashSeparatedArg,
1213
} from "../../lib/arg-parsing.js";
1314
import { openInBrowser } from "../../lib/browser.js";
1415
import { buildCommand } from "../../lib/command.js";
1516
import { ContextError, ValidationError } from "../../lib/errors.js";
1617
import { formatLogDetails, writeJson } from "../../lib/formatters/index.js";
18+
import { logger } from "../../lib/logger.js";
1719
import {
1820
resolveOrgAndProject,
1921
resolveProjectBySlug,
@@ -40,6 +42,8 @@ const USAGE_HINT = "sentry log view <org>/<project> <log-id>";
4042
export function parsePositionalArgs(args: string[]): {
4143
logId: string;
4244
targetArg: string | undefined;
45+
/** Warning message if arguments appear to be in the wrong order */
46+
warning?: string;
4347
} {
4448
if (args.length === 0) {
4549
throw new ContextError("Log ID", USAGE_HINT);
@@ -64,6 +68,12 @@ export function parsePositionalArgs(args: string[]): {
6468
return { logId: first, targetArg: undefined };
6569
}
6670

71+
// Detect swapped args: user put ID first and target second
72+
const swapWarning = detectSwappedViewArgs(first, second);
73+
if (swapWarning) {
74+
return { logId: first, targetArg: second, warning: swapWarning };
75+
}
76+
6777
// Two or more args - first is target, second is log ID
6878
return { logId: second, targetArg: first };
6979
}
@@ -140,10 +150,17 @@ export const viewCommand = buildCommand({
140150
...args: string[]
141151
): Promise<void> {
142152
const { stdout, cwd, setContext } = this;
153+
const cmdLog = logger.withTag("log.view");
143154

144155
// Parse positional args
145-
const { logId, targetArg } = parsePositionalArgs(args);
156+
const { logId, targetArg, warning } = parsePositionalArgs(args);
157+
if (warning) {
158+
cmdLog.warn(warning);
159+
}
146160
const parsed = parseOrgProjectArg(targetArg);
161+
if (parsed.type !== "auto-detect" && parsed.normalized) {
162+
cmdLog.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
163+
}
147164

148165
let target: ResolvedLogTarget | null = null;
149166

@@ -159,8 +176,7 @@ export const viewCommand = buildCommand({
159176
target = await resolveProjectBySlug(
160177
parsed.projectSlug,
161178
USAGE_HINT,
162-
`sentry log view <org>/${parsed.projectSlug} ${logId}`,
163-
this.stderr
179+
`sentry log view <org>/${parsed.projectSlug} ${logId}`
164180
);
165181
break;
166182

src/commands/project/list.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -485,7 +485,7 @@ export async function handleProjectSearch(
485485
projectSlug: string,
486486
flags: ListFlags
487487
): Promise<void> {
488-
const projects: ProjectWithOrg[] = await findProjectsBySlug(projectSlug);
488+
const { projects } = await findProjectsBySlug(projectSlug);
489489
const filtered = filterByPlatform(projects, flags.platform);
490490

491491
if (filtered.length === 0) {

src/commands/project/view.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,7 @@ export const viewCommand = buildCommand({
230230
const resolved = await resolveProjectBySlug(
231231
parsed.projectSlug,
232232
USAGE_HINT,
233-
`sentry project view <org>/${parsed.projectSlug}`,
234-
this.stderr
233+
`sentry project view <org>/${parsed.projectSlug}`
235234
);
236235
resolvedTargets = [
237236
{

src/commands/trace/view.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import type { SentryContext } from "../../context.js";
88
import { getDetailedTrace } from "../../lib/api-client.js";
99
import {
10+
detectSwappedViewArgs,
1011
parseOrgProjectArg,
1112
parseSlashSeparatedArg,
1213
spansFlag,
@@ -21,6 +22,7 @@ import {
2122
writeFooter,
2223
writeJson,
2324
} from "../../lib/formatters/index.js";
25+
import { logger } from "../../lib/logger.js";
2426
import {
2527
resolveOrgAndProject,
2628
resolveProjectBySlug,
@@ -48,6 +50,8 @@ const USAGE_HINT = "sentry trace view <org>/<project> <trace-id>";
4850
export function parsePositionalArgs(args: string[]): {
4951
traceId: string;
5052
targetArg: string | undefined;
53+
/** Warning message if arguments appear to be in the wrong order */
54+
warning?: string;
5155
} {
5256
if (args.length === 0) {
5357
throw new ContextError("Trace ID", USAGE_HINT);
@@ -72,6 +76,12 @@ export function parsePositionalArgs(args: string[]): {
7276
return { traceId: first, targetArg: undefined };
7377
}
7478

79+
// Detect swapped args: user put ID first and target second
80+
const swapWarning = detectSwappedViewArgs(first, second);
81+
if (swapWarning) {
82+
return { traceId: first, targetArg: second, warning: swapWarning };
83+
}
84+
7585
// Two or more args - first is target, second is trace ID
7686
return { traceId: second, targetArg: first };
7787
}
@@ -151,10 +161,17 @@ export const viewCommand = buildCommand({
151161
...args: string[]
152162
): Promise<void> {
153163
const { stdout, cwd, setContext } = this;
164+
const log = logger.withTag("trace.view");
154165

155166
// Parse positional args
156-
const { traceId, targetArg } = parsePositionalArgs(args);
167+
const { traceId, targetArg, warning } = parsePositionalArgs(args);
168+
if (warning) {
169+
log.warn(warning);
170+
}
157171
const parsed = parseOrgProjectArg(targetArg);
172+
if (parsed.type !== "auto-detect" && parsed.normalized) {
173+
log.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
174+
}
158175

159176
let target: ResolvedTraceTarget | null = null;
160177

@@ -170,8 +187,7 @@ export const viewCommand = buildCommand({
170187
target = await resolveProjectBySlug(
171188
parsed.projectSlug,
172189
USAGE_HINT,
173-
`sentry trace view <org>/${parsed.projectSlug} ${traceId}`,
174-
this.stderr
190+
`sentry trace view <org>/${parsed.projectSlug} ${traceId}`
175191
);
176192
break;
177193

0 commit comments

Comments
 (0)