Skip to content

Commit 453b52b

Browse files
authored
fix: improve argument parsing for common user mistakes (#363)
## Fix ResolutionError UX: Argument Swapping + Smarter Fallbacks Fixes CLI-BG, CLI-BB, CLI-BA, CLI-B9, CLI-B8 Users hit `ResolutionError`s because the CLI misinterprets their input in several common scenarios: | Issue | User Input | Root Cause | |-------|-----------|------------| | CLI-BG | `sentry log view a9b4ad2c mv-software/mvsoftware` | Args reversed (ID first, target second) | | CLI-B8 | `sentry event view CAM-82X 95fd7f5a` | First arg is issue short ID, not a target | | CLI-BB | `sentry issue view org/project/101149101` | Multi-slash parsed incorrectly | | CLI-BA | `sentry issues techdoctor-inc/selfbase_admin_backend` | Underscores in project slug | | CLI-B9 | `sentry issue list kaiko-systems-gmbh` | Bare org slug misinterpreted as project-search | ### Changes 1. **Argument swap detection** — `detectSwappedViewArgs()` detects when users reverse `<target> <id>` ordering in view commands. Auto-corrects with a warning. 2. **Underscore normalization** — `normalizeSlug()` converts underscores to dashes at the parsing layer since Sentry slugs never contain underscores. 3. **Multi-slash issue arg** — `parseMultiSlashIssueArg()` correctly splits `org/project/issueId` instead of producing a broken suffix. 4. **Org-as-project auto-redirect** — When a bare slug matches an organization (not a project), list commands auto-redirect to org-all mode. View commands show a clear "is an organization, not a project" error. Refactored `findProjectsBySlug` to return `{ projects, orgs }` to avoid double API calls. 5. **Issue short ID detection** — Suggests `sentry issue view X` when the first arg in event/log/trace view matches the issue short ID pattern (e.g., `CAM-82X`). 6. **Removed `stderr` threading** — Replaced all `stderr.write()` usage in resolve-target and view commands with Consola logger (`log.warn()`), simplifying function signatures. ### Tests - 35 new unit + property tests for helpers (`normalizeSlug`, `looksLikeIssueShortId`, `detectSwappedViewArgs`, underscore normalization, multi-slash parsing) - Updated 12 existing test files to accommodate return type and signature changes - All 2738 unit tests pass
1 parent a5fa054 commit 453b52b

27 files changed

+1692
-335
lines changed

AGENTS.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -651,6 +651,15 @@ mock.module("./some-module", () => ({
651651
652652
### Decision
653653
654+
<!-- lore:019cc2ef-9be5-722d-bc9f-b07a8197eeed -->
655+
* **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.
656+
657+
<!-- lore:019c99d5-69f2-74eb-8c86-411f8512801d -->
658+
* **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.
659+
660+
<!-- lore:00166785-609d-4ab5-911e-ee205d17b90c -->
661+
* **whoami should be separate from auth status command**: The \`sentry auth whoami\` command should be a dedicated command separate from \`sentry auth status\`. They serve different purposes: \`status\` shows everything about auth state (token, expiry, defaults, org verification), while \`whoami\` just shows user identity (name, email, username, ID) by fetching live from \`/auth/\` endpoint. \`sentry whoami\` should be a top-level alias (like \`sentry issues\`\`sentry issue list\`). \`whoami\` should support \`--json\` for machine consumption and be lightweight — no credential verification, no defaults listing.
662+
654663
<!-- lore:019c8f3b-84be-79be-9518-e5ecd2ea64b9 -->
655664
* **Use -t (not -p) as shortcut alias for --period flag**: The --period flag on issue list uses -t (for 'time period') as its short alias, not -p. The rationale: -p could be confused with --platform from other CLI tools/contexts. -t maps naturally to 'time period' and avoids collision. This was a deliberate choice after initial implementation used -p.
656665
@@ -676,6 +685,9 @@ mock.module("./some-module", () => ({
676685
<!-- lore:dbd63348-2049-42b3-bb99-d6a3d64369c7 -->
677686
* **Branch naming and commit message conventions for Sentry CLI**: Branch naming: \`feat/\<short-description>\` or \`fix/\<issue-number>-\<short-description>\` (e.g., \`feat/ghcr-nightly-distribution\`, \`fix/268-limit-auto-pagination\`). Commit message format: \`type(scope): description (#issue)\` (e.g., \`fix(issue-list): auto-paginate --limit beyond 100 (#268)\`, \`feat(nightly): distribute via GHCR instead of GitHub Releases\`). Types seen: fix, refactor, meta, release, feat. PRs are created as drafts via \`gh pr create --draft\`. Implementation plans are attached to commits via \`git notes add\` rather than in PR body or commit message.
678687
688+
<!-- lore:5ac4e219-ea1f-41cb-8e97-7e946f5848c0 -->
689+
* **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\`.
690+
679691
<!-- lore:019cc3e6-0cf5-720d-beb7-97c9c9901295 -->
680692
* **Codecov patch coverage only counts test:unit and test:isolated, not E2E**: CI coverage merges \`test:unit\` (\`test/lib test/commands test/types --coverage\`) and \`test:isolated\` (\`test/isolated --coverage\`) into \`coverage/merged.lcov\`. E2E tests (\`test/e2e\`) are NOT included in coverage reports. So func tests that spy on exports (e.g., \`spyOn(apiClient, 'getLogs')\`) give zero coverage to the mocked function's body. To cover \`api-client.ts\` function bodies in unit tests, mock \`globalThis.fetch\` + \`setOrgRegion()\` + \`setAuthToken()\` and call the real function.
681693

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: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,9 +354,29 @@ 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+
// Unlike simpler list commands that auto-redirect via orgAllFallback,
364+
// issue list has custom per-project query logic (query rewriting,
365+
// budget redistribution) that doesn't support org-all mode here.
366+
// Throwing with actionable hints is the correct behavior.
367+
const isOrg = orgs.some((o) => o.slug === parsed.projectSlug);
368+
if (isOrg) {
369+
throw new ResolutionError(
370+
`'${parsed.projectSlug}'`,
371+
"is an organization, not a project",
372+
`sentry issue list ${parsed.projectSlug}/`,
373+
[
374+
`List projects: sentry project list ${parsed.projectSlug}/`,
375+
`Specify a project: sentry issue list ${parsed.projectSlug}/<project>`,
376+
]
377+
);
378+
}
379+
360380
throw new ResolutionError(
361381
`Project '${parsed.projectSlug}'`,
362382
"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: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { isatty } from "node:tty";
99
import type { SentryContext } from "../../context.js";
1010
import { getLogs } from "../../lib/api-client.js";
1111
import {
12+
looksLikeIssueShortId,
1213
parseOrgProjectArg,
1314
parseSlashSeparatedArg,
1415
} from "../../lib/arg-parsing.js";
@@ -73,6 +74,8 @@ function splitLogIds(arg: string): string[] {
7374
export function parsePositionalArgs(args: string[]): {
7475
logIds: string[];
7576
targetArg: string | undefined;
77+
/** Suggestion when first arg looks like an issue short ID */
78+
suggestion?: string;
7679
} {
7780
if (args.length === 0) {
7881
throw new ContextError("Log ID", USAGE_HINT);
@@ -105,7 +108,15 @@ export function parsePositionalArgs(args: string[]): {
105108
if (logIds.length === 0) {
106109
throw new ContextError("Log ID", USAGE_HINT);
107110
}
108-
return { logIds, targetArg: first };
111+
// Swap detection is not useful here: validateHexId above rejects non-hex
112+
// log IDs (which include any containing "/"), so detectSwappedViewArgs
113+
// (which checks for "/" in the second arg) can never trigger.
114+
// We still check for issue short IDs in the first (target) position.
115+
const suggestion = looksLikeIssueShortId(first)
116+
? `Did you mean: sentry issue view ${first}`
117+
: undefined;
118+
119+
return { logIds, targetArg: first, suggestion };
109120
}
110121

111122
/**
@@ -331,10 +342,17 @@ export const viewCommand = buildCommand({
331342
...args: string[]
332343
): Promise<void> {
333344
const { stdout, cwd, setContext } = this;
345+
const cmdLog = logger.withTag("log.view");
334346

335347
// Parse positional args
336-
const { logIds, targetArg } = parsePositionalArgs(args);
348+
const { logIds, targetArg, suggestion } = parsePositionalArgs(args);
349+
if (suggestion) {
350+
cmdLog.warn(suggestion);
351+
}
337352
const parsed = parseOrgProjectArg(targetArg);
353+
if (parsed.type !== "auto-detect" && parsed.normalized) {
354+
cmdLog.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
355+
}
338356

339357
const target = await resolveTarget(parsed, logIds, cwd);
340358

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: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
import type { SentryContext } from "../../context.js";
88
import { getDetailedTrace } from "../../lib/api-client.js";
99
import {
10+
detectSwappedViewArgs,
11+
looksLikeIssueShortId,
1012
parseOrgProjectArg,
1113
parseSlashSeparatedArg,
1214
spansFlag,
@@ -21,6 +23,7 @@ import {
2123
writeFooter,
2224
writeJson,
2325
} from "../../lib/formatters/index.js";
26+
import { logger } from "../../lib/logger.js";
2427
import {
2528
resolveOrgAndProject,
2629
resolveProjectBySlug,
@@ -48,6 +51,10 @@ const USAGE_HINT = "sentry trace view <org>/<project> <trace-id>";
4851
export function parsePositionalArgs(args: string[]): {
4952
traceId: string;
5053
targetArg: string | undefined;
54+
/** Warning message if arguments appear to be in the wrong order */
55+
warning?: string;
56+
/** Suggestion when first arg looks like an issue short ID */
57+
suggestion?: string;
5158
} {
5259
if (args.length === 0) {
5360
throw new ContextError("Trace ID", USAGE_HINT);
@@ -72,8 +79,19 @@ export function parsePositionalArgs(args: string[]): {
7279
return { traceId: first, targetArg: undefined };
7380
}
7481

82+
// Detect swapped args: user put ID first and target second
83+
const swapWarning = detectSwappedViewArgs(first, second);
84+
if (swapWarning) {
85+
return { traceId: first, targetArg: second, warning: swapWarning };
86+
}
87+
88+
// Detect issue short ID passed as first arg (e.g., "CAM-82X some-trace-id")
89+
const suggestion = looksLikeIssueShortId(first)
90+
? `Did you mean: sentry issue view ${first}`
91+
: undefined;
92+
7593
// Two or more args - first is target, second is trace ID
76-
return { traceId: second, targetArg: first };
94+
return { traceId: second, targetArg: first, suggestion };
7795
}
7896

7997
/**
@@ -151,10 +169,21 @@ export const viewCommand = buildCommand({
151169
...args: string[]
152170
): Promise<void> {
153171
const { stdout, cwd, setContext } = this;
172+
const log = logger.withTag("trace.view");
154173

155174
// Parse positional args
156-
const { traceId, targetArg } = parsePositionalArgs(args);
175+
const { traceId, targetArg, warning, suggestion } =
176+
parsePositionalArgs(args);
177+
if (warning) {
178+
log.warn(warning);
179+
}
180+
if (suggestion) {
181+
log.warn(suggestion);
182+
}
157183
const parsed = parseOrgProjectArg(targetArg);
184+
if (parsed.type !== "auto-detect" && parsed.normalized) {
185+
log.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
186+
}
158187

159188
let target: ResolvedTraceTarget | null = null;
160189

@@ -170,8 +199,7 @@ export const viewCommand = buildCommand({
170199
target = await resolveProjectBySlug(
171200
parsed.projectSlug,
172201
USAGE_HINT,
173-
`sentry trace view <org>/${parsed.projectSlug} ${traceId}`,
174-
this.stderr
202+
`sentry trace view <org>/${parsed.projectSlug} ${traceId}`
175203
);
176204
break;
177205

0 commit comments

Comments
 (0)