Skip to content

Commit 169f8fe

Browse files
authored
fix(arg-parsing): accept underscores in Sentry slugs (#770) (#771)
## Summary Fixes #770. The CLI was converting underscores to dashes in `normalizeSlug()` on the assumption that "Sentry slugs never contain underscores". That assumption is wrong — the Sentry platform accepts underscores in project slugs at creation time, and rewriting them at the CLI layer made it impossible for affected customers to look up their own projects. ## Verification Live API test against a real org before the fix: ``` $ sentry api --method POST /teams/byk-test/byk-test/projects/ \ --field slug=test_underscore_slug \ --field name=test_underscore_project \ --field platform=javascript # → 200 OK, slug preserved exactly as "test_underscore_slug" $ sentry api /projects/byk-test/test_underscore_slug/ # → 200 OK, round-trips cleanly ``` Before (the bug): ``` $ sentry issue list byk-test/test_underscore_slug [arg-parsing] WARN Normalized slug to 'byk-test/test-underscore-slug' (Sentry slugs use dashes, never underscores) Error: Project 'test-underscore-slug' not found in organization 'byk-test'. ``` After this PR (running the dev build against the same project): ``` $ bun run dev issue list byk-test/test_underscore_slug No issues found. ``` Space normalization still works as before: ``` $ bun run dev issue list "BYK Test/My Project" [arg-parsing] WARN Normalized slug to 'byk-test/my-project' (Sentry slugs use lowercase with dashes, not spaces) ``` ## History (for reviewers) - The underscore rule was introduced in #363 as typo-correction for one specific customer (`selfbase_admin_backend`), never verified against the platform. - #757 folded underscore and space handling into a combined `[_ ]+` regex. The reporter on #770 correctly spotted that regex — but the root cause predates that PR. - The legacy `sentry-cli` (Rust) accepts underscored slugs, so the new CLI diverging on this is a regression from the user's perspective. ## Changes - `normalizeSlug()`: only normalize when the input contains a space. Underscored input is returned unchanged (`normalized: false`). - Remove the `NormalizeReason` discriminator. With underscores gone, only the "spaces" path remains, so `reason` collapses to `normalized: true` (and the warning message becomes a single string). - `ParsedOrgProject`: drop the `reason?` field from all three variants. - `trace-target.ts`: no source change required — already only read `.slug` and `.normalized`. - Tests: replace underscore-normalization tests with underscore-preservation tests. Mixed inputs like `my_org/My Project` now correctly yield `{ org: "my_org", project: "my-project" }`. Delete the three command-level underscore-warning tests (the code path no longer exists; unit-level space coverage is sufficient). ## Verification - `bun run typecheck` — clean - `bun run lint` — clean (pre-existing warning in `markdown.ts` unrelated) - `bun run test:unit` — 5036 / 5036 pass - `bun run test:isolated` — 138 / 138 pass - `bun run check:errors`, `bun run check:deps` — clean - Manual end-to-end repro against real Sentry org (above). Net diff: −110 lines.
1 parent 19e88e8 commit 169f8fe

9 files changed

Lines changed: 159 additions & 267 deletions

File tree

src/lib/arg-parsing.ts

Lines changed: 38 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -18,92 +18,56 @@ import { isAllDigits } from "./utils.js";
1818
// Slug normalization
1919
// ---------------------------------------------------------------------------
2020

21-
/** Describes what was normalized in a slug — used for targeted warning messages. */
22-
export type NormalizeReason = "underscores" | "spaces" | "both";
23-
2421
/**
25-
* Normalize a Sentry slug by replacing underscores and spaces with dashes.
22+
* Normalize a Sentry slug by converting display-name-style input to slug form.
2623
*
27-
* Sentry enforces that slugs use lowercase letters, numbers, and dashes.
28-
* Users frequently type underscores by mistake or paste display names
29-
* (e.g., `"My Project"` instead of `"my-project"`).
24+
* Sentry slugs are always lowercase and use dashes as separators. Users
25+
* sometimes paste display names like `"My Project"` instead of
26+
* `"my-project"`; without normalization, `validateResourceId` would reject
27+
* those outright for containing spaces. Normalization gives us a chance to
28+
* recover the intent before validation runs.
3029
*
31-
* Normalization rules:
32-
* 1. Underscores → dashes (existing)
33-
* 2. Spaces → dashes, with lowercase (spaces imply a display name)
34-
* 3. Consecutive dashes collapsed (`"My Project"` → `"my-project"`)
35-
* 4. Leading/trailing dashes trimmed
30+
* Normalization rules (applied only when spaces are present):
31+
* 1. Lowercase the entire string
32+
* 2. Collapse runs of spaces into a single dash
33+
* 3. Trim leading/trailing dashes (from leading/trailing whitespace)
3634
*
37-
* Lowercasing is only applied when spaces are present. Pure underscore
38-
* normalization preserves casing for backward compatibility.
35+
* Underscores are **not** normalized — Sentry accepts underscores in project
36+
* slugs at creation time, so rewriting them here would silently break
37+
* lookups for any customer who has one (see #770). Since underscores are
38+
* not in `validateResourceId`'s forbidden set either, they flow through
39+
* untouched.
3940
*
4041
* @param slug - Raw slug string from CLI input
41-
* @returns Normalized slug, whether normalization was applied, and reason
42+
* @returns Normalized slug and whether normalization was applied
4243
*
4344
* @example
44-
* normalizeSlug("selfbase_admin_backend") // { slug: "selfbase-admin-backend", normalized: true, reason: "underscores" }
45-
* normalizeSlug("My Project") // { slug: "my-project", normalized: true, reason: "spaces" }
46-
* normalizeSlug("My_Project Name") // { slug: "my-project-name", normalized: true, reason: "both" }
45+
* normalizeSlug("My Project") // { slug: "my-project", normalized: true }
46+
* normalizeSlug("My Project") // { slug: "my-project", normalized: true }
47+
* normalizeSlug("selfbase_admin_backend") // { slug: "selfbase_admin_backend", normalized: false }
4748
* normalizeSlug("my-project") // { slug: "my-project", normalized: false }
4849
*/
4950
export function normalizeSlug(slug: string): {
5051
slug: string;
5152
normalized: boolean;
52-
reason?: NormalizeReason;
5353
} {
54-
const hasUnderscores = slug.includes("_");
55-
const hasSpaces = slug.includes(" ");
56-
57-
if (!(hasUnderscores || hasSpaces)) {
54+
if (!slug.includes(" ")) {
5855
return { slug, normalized: false };
5956
}
6057

61-
let result = slug;
62-
63-
// When spaces are present, lowercase the entire slug.
64-
// Spaces imply a display name like "My Project" — slugs are always lowercase.
65-
if (hasSpaces) {
66-
result = result.toLowerCase();
67-
}
68-
69-
// Replace runs of underscores/spaces with a single dash.
70-
// Using [_ ]+ collapses "My Project" and "a__b" in one pass.
71-
result = result.replace(/[_ ]+/g, "-");
72-
73-
// Trim leading/trailing dashes (from " My Project " → "-my-project-")
74-
result = result.replace(/^-+|-+$/g, "");
58+
// Spaces imply a display name — slugs are always lowercase.
59+
// Collapse runs of spaces into a single dash, then trim any edge dashes
60+
// introduced by leading/trailing whitespace in the input.
61+
const result = slug
62+
.toLowerCase()
63+
.replace(/ +/g, "-")
64+
.replace(/^-+|-+$/g, "");
7565

76-
let reason: NormalizeReason;
77-
if (hasUnderscores && hasSpaces) {
78-
reason = "both";
79-
} else if (hasSpaces) {
80-
reason = "spaces";
81-
} else {
82-
reason = "underscores";
83-
}
84-
85-
return { slug: result, normalized: true, reason };
66+
return { slug: result, normalized: true };
8667
}
8768

8869
const log = logger.withTag("arg-parsing");
8970

90-
/**
91-
* Combine two normalization reasons into the most descriptive one.
92-
* Used when org and project slugs have different normalizations.
93-
*/
94-
function combineReasons(
95-
a?: NormalizeReason,
96-
b?: NormalizeReason
97-
): NormalizeReason {
98-
if (a === "both" || b === "both") {
99-
return "both";
100-
}
101-
if (a && b && a !== b) {
102-
return "both";
103-
}
104-
return a ?? b ?? "underscores";
105-
}
106-
10771
/**
10872
* Emit a warning when slug normalization changed the input.
10973
* Called internally by {@link parseOrgProjectArg} — callers do not need to
@@ -127,21 +91,9 @@ function warnNormalized(
12791
return;
12892
}
12993

130-
let explanation: string;
131-
switch (parsed.reason) {
132-
case "spaces":
133-
explanation = "Sentry slugs use lowercase with dashes, not spaces";
134-
break;
135-
case "both":
136-
explanation =
137-
"Sentry slugs use lowercase with dashes, not spaces or underscores";
138-
break;
139-
default:
140-
explanation = "Sentry slugs use dashes, never underscores";
141-
break;
142-
}
143-
144-
log.warn(`Normalized slug to '${slug}' (${explanation})`);
94+
log.warn(
95+
`Normalized slug to '${slug}' (Sentry slugs use lowercase with dashes, not spaces)`
96+
);
14597
}
14698

14799
// ---------------------------------------------------------------------------
@@ -438,8 +390,9 @@ export const ProjectSpecificationType = {
438390
* Parsed result from an org/project positional argument.
439391
* Discriminated union based on the `type` field.
440392
*
441-
* When `normalized` is true, the slug was auto-corrected (underscores → dashes,
442-
* spaces → dashes with lowercasing). `reason` describes what was normalized.
393+
* When `normalized` is true, the slug was auto-corrected from display-name
394+
* form (spaces → dashes with lowercasing). Underscores are preserved — Sentry
395+
* allows underscored project slugs and the CLI must not rewrite them.
443396
*/
444397
export type ParsedOrgProject =
445398
| {
@@ -448,24 +401,18 @@ export type ParsedOrgProject =
448401
project: string;
449402
/** True if any slug was normalized */
450403
normalized?: boolean;
451-
/** What was normalized — used for targeted warning messages */
452-
reason?: NormalizeReason;
453404
}
454405
| {
455406
type: typeof ProjectSpecificationType.OrgAll;
456407
org: string;
457408
/** True if org slug was normalized */
458409
normalized?: boolean;
459-
/** What was normalized — used for targeted warning messages */
460-
reason?: NormalizeReason;
461410
}
462411
| {
463412
type: typeof ProjectSpecificationType.ProjectSearch;
464413
projectSlug: string;
465414
/** True if project slug was normalized */
466415
normalized?: boolean;
467-
/** What was normalized — used for targeted warning messages */
468-
reason?: NormalizeReason;
469416
}
470417
| { type: typeof ProjectSpecificationType.AutoDetect };
471418

@@ -599,7 +546,7 @@ function parseSlashOrgProject(input: string): ParsedOrgProject {
599546
return {
600547
type: "project-search",
601548
projectSlug: np.slug,
602-
...(np.normalized && { normalized: true, reason: np.reason }),
549+
...(np.normalized && { normalized: true }),
603550
};
604551
}
605552

@@ -612,7 +559,7 @@ function parseSlashOrgProject(input: string): ParsedOrgProject {
612559
return {
613560
type: "org-all",
614561
org: no.slug,
615-
...(no.normalized && { normalized: true, reason: no.reason }),
562+
...(no.normalized && { normalized: true }),
616563
};
617564
}
618565

@@ -621,12 +568,11 @@ function parseSlashOrgProject(input: string): ParsedOrgProject {
621568
const np = normalizeSlug(rawProject);
622569
validateResourceId(np.slug, "project slug");
623570
const normalized = no.normalized || np.normalized;
624-
const reason = normalized ? combineReasons(no.reason, np.reason) : undefined;
625571
return {
626572
type: "explicit",
627573
org: no.slug,
628574
project: np.slug,
629-
...(normalized && { normalized: true, reason }),
575+
...(normalized && { normalized: true }),
630576
};
631577
}
632578

@@ -676,7 +622,7 @@ export function parseOrgProjectArg(arg: string | undefined): ParsedOrgProject {
676622
parsed = {
677623
type: "project-search",
678624
projectSlug: np.slug,
679-
...(np.normalized && { normalized: true, reason: np.reason }),
625+
...(np.normalized && { normalized: true }),
680626
};
681627
}
682628

src/lib/trace-target.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export type ParsedTraceTarget =
4747
traceId: string;
4848
org: string;
4949
project: string;
50-
/** True if any slug was normalized (underscores → dashes) */
50+
/** True if any slug was normalized (spaces → dashes with lowercasing) */
5151
normalized?: boolean;
5252
}
5353
| {
@@ -267,7 +267,9 @@ export function targetArgToTraceTarget(
267267
export function warnIfNormalized(parsed: ParsedTraceTarget, tag: string): void {
268268
if ("normalized" in parsed && parsed.normalized) {
269269
const log = logger.withTag(tag);
270-
log.warn("Normalized slug (Sentry slugs use dashes, not underscores)");
270+
log.warn(
271+
"Normalized slug (Sentry slugs use lowercase with dashes, not spaces)"
272+
);
271273
}
272274
}
273275

test/commands/event/view.test.ts

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -888,31 +888,6 @@ describe("viewCommand.func", () => {
888888
getLatestEventSpy.mockRestore();
889889
});
890890

891-
test("logs normalized slug warning when underscores present", async () => {
892-
getEventSpy.mockResolvedValue(sampleEvent);
893-
getSpanTreeLinesSpy.mockResolvedValue({
894-
lines: [],
895-
spans: null,
896-
traceId: null,
897-
success: false,
898-
});
899-
setOrgRegion("test-org", DEFAULT_SENTRY_URL);
900-
901-
const { context } = createMockContext();
902-
const func = await viewCommand.loader();
903-
// Underscores in the slug trigger normalized warning
904-
await func.call(
905-
context,
906-
{ json: true, web: false, spans: 0 },
907-
"test_org/test_proj",
908-
VALID_EVENT_ID
909-
);
910-
911-
// parseOrgProjectArg normalizes "test_org/test_proj" → "test-org/test-proj"
912-
// and sets normalized=true, triggering the warning path (line 343-345)
913-
expect(getEventSpy).toHaveBeenCalled();
914-
});
915-
916891
test("throws ValidationError for flag-like event ID (--h)", async () => {
917892
const { context } = createMockContext();
918893
const func = await viewCommand.loader();

test/commands/init.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,8 @@ describe("init command func", () => {
350350
});
351351

352352
test("org slug with whitespace is normalized (not rejected)", async () => {
353-
// Spaces in slugs are normalized to dashes (like underscore normalization)
353+
// Spaces in slugs are normalized to dashes — the "display name"
354+
// recovery path (e.g., "My Org" → "my-org").
354355
const ctx = makeContext();
355356
await func.call(ctx, DEFAULT_FLAGS, "acme corp/");
356357
expect(capturedArgs?.org).toBe("acme-corp");

test/commands/log/view.test.ts

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -459,23 +459,6 @@ describe("viewCommand.func", () => {
459459
).rejects.toThrow(ValidationError);
460460
});
461461

462-
test("logs normalized slug warning when underscores present", async () => {
463-
getLogsSpy.mockResolvedValue([sampleLog]);
464-
setOrgRegion("test-org", DEFAULT_SENTRY_URL);
465-
466-
const { context } = createMockContext();
467-
const func = await viewCommand.loader();
468-
// Underscores in the slug trigger normalized warning (line 161-163)
469-
await func.call(
470-
context,
471-
{ json: true, web: false },
472-
"test_org/test_proj",
473-
"968c763c740cfda8b6728f27fb9e9b01"
474-
);
475-
476-
expect(getLogsSpy).toHaveBeenCalled();
477-
});
478-
479462
test("resolves project-search target via resolveProjectBySlug", async () => {
480463
findProjectsBySlugSpy.mockResolvedValue({
481464
projects: [

test/commands/trace/view.func.test.ts

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -348,24 +348,6 @@ describe("viewCommand.func", () => {
348348
expect(getDetailedTraceSpy).toHaveBeenCalled();
349349
});
350350

351-
test("logs normalized slug warning when underscores present", async () => {
352-
getDetailedTraceSpy.mockResolvedValue(sampleSpans);
353-
354-
const { context } = createMockContext();
355-
const func = await viewCommand.loader();
356-
// Underscores in the slug trigger normalized warning (line 172-173)
357-
await func.call(
358-
context,
359-
{ json: true, web: false, spans: 100 },
360-
"test_org/test_project",
361-
"aaaa1111bbbb2222cccc3333dddd4444"
362-
);
363-
364-
// parseOrgProjectArg normalizes "test_org/test_project" → "test-org/test-project"
365-
// and sets normalized=true, triggering the log.warn (line 173)
366-
expect(getDetailedTraceSpy).toHaveBeenCalled();
367-
});
368-
369351
test("logs suggestion when first arg looks like issue short ID", async () => {
370352
// "CAM-82X" as first arg matches issue short ID pattern.
371353
// parseOrgProjectArg("CAM-82X") → project-search, so we mock findProjectsBySlug.

test/lib/arg-parsing.property.test.ts

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -398,29 +398,44 @@ describe("normalizeSlug properties", () => {
398398
);
399399
});
400400

401-
test("normalized is true iff input contained underscores or spaces", async () => {
401+
test("normalized is true iff input contained a space", async () => {
402+
// Underscores are intentionally preserved (see #770) — only spaces
403+
// trigger normalization.
402404
await fcAssert(
403405
property(displayNameLikeArb, (input) => {
404406
const result = normalizeSlug(input);
405-
expect(result.normalized).toBe(
406-
input.includes("_") || input.includes(" ")
407-
);
407+
expect(result.normalized).toBe(input.includes(" "));
408408
}),
409409
{ numRuns: DEFAULT_NUM_RUNS }
410410
);
411411
});
412412

413-
test("result slug never contains underscores or spaces", async () => {
413+
test("result slug never contains spaces", async () => {
414414
await fcAssert(
415415
property(displayNameLikeArb, (input) => {
416416
const result = normalizeSlug(input);
417-
expect(result.slug.includes("_")).toBe(false);
418417
expect(result.slug.includes(" ")).toBe(false);
419418
}),
420419
{ numRuns: DEFAULT_NUM_RUNS }
421420
);
422421
});
423422

423+
test("underscores in input survive to output unchanged in count", async () => {
424+
// Each underscore in the input must appear in the output — normalization
425+
// never rewrites underscores. Using a comparison on count makes the
426+
// property robust to other transforms (lowercasing, space→dash) that
427+
// may or may not happen depending on whether spaces are present.
428+
await fcAssert(
429+
property(displayNameLikeArb, (input) => {
430+
const result = normalizeSlug(input);
431+
const inputUnderscores = (input.match(/_/g) ?? []).length;
432+
const outputUnderscores = (result.slug.match(/_/g) ?? []).length;
433+
expect(outputUnderscores).toBe(inputUnderscores);
434+
}),
435+
{ numRuns: DEFAULT_NUM_RUNS }
436+
);
437+
});
438+
424439
test("result slug length is <= input length", async () => {
425440
await fcAssert(
426441
property(displayNameLikeArb, (input) => {
@@ -447,7 +462,7 @@ describe("normalizeSlug properties", () => {
447462
property(withSpaceArb, (input) => {
448463
const result = normalizeSlug(input);
449464
expect(result.slug).toBe(result.slug.toLowerCase());
450-
expect(result.reason).toMatch(/^(spaces|both)$/);
465+
expect(result.normalized).toBe(true);
451466
}),
452467
{ numRuns: DEFAULT_NUM_RUNS }
453468
);

0 commit comments

Comments
 (0)