Skip to content

Commit 70b2ac1

Browse files
authored
fix: add org-slug pre-check to dispatchOrgScopedList (CLI-9A) (#485)
## Problem When a user runs `sentry project list ftmo` (intending to list projects in org "ftmo"), the CLI classifies the bare slug as a project-search and makes N API calls to `getProject()` across all orgs. When no project matches, it throws a confusing `ContextError: Project is required` — 190 occurrences affecting 98 users ([CLI-9A](https://sentry.sentry.io/issues/CLI-9A)). ## Fix Adds a centralized org-cache pre-check in `dispatchOrgScopedList` via a new `orgSlugMatchBehavior` option on `DispatchOptions`: - **`"redirect"`**: Convert to org-all mode with a warning (used by `project list`, `team list`, `repo list`) - **`"error"`**: Throw a `ResolutionError` with actionable hints (for commands like `issue list` that can't auto-redirect) - **`undefined`** (default): No pre-check — fully backward compatible The check uses SQLite-cached orgs (`getCachedOrganizations()`) so it's O(1) and avoids the expensive N API calls entirely. If the cache is cold/stale, falls through to the handler's own org-slug check as a safety net. ### Why centralize instead of patching individual handlers? The bug existed because `project/list.ts` had a custom `handleProjectSearch` that was missing the org-match check present in the shared handler (`org-list.ts`). By moving the check into `dispatchOrgScopedList` itself, no handler can forget it. ## Changes - `src/lib/org-list.ts` — `orgSlugMatchBehavior` option + `resolveOrgSlugMatch()` helper - `src/commands/project/list.ts` — Opt in with `orgSlugMatchBehavior: "redirect"` - `src/lib/list-command.ts` — Opt in for `buildOrgListCommand` commands (team/repo list) - `AGENTS.md` — Document the pattern for future list commands - `test/lib/org-list.test.ts` — 6 new tests covering redirect, error, fallthrough, and cache-miss scenarios Fixes CLI-9A
1 parent 0ac022e commit 70b2ac1

File tree

5 files changed

+250
-8
lines changed

5 files changed

+250
-8
lines changed

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,7 @@ Key rules when writing overrides:
469469
- Commands with extra fields (e.g., `stderr`, `setContext`) spread the context and add them: `(ctx) => handle({ ...ctx, flags, stderr, setContext })`. Override `ctx.flags` with the command-specific flags type when needed.
470470
- `resolveCursor()` must be called **inside** the `org-all` override closure, not before `dispatchOrgScopedList`, so that `--cursor` validation errors fire correctly for non-org-all modes.
471471
- `handleProjectSearch` errors must use `"Project"` as the `ContextError` resource, not `config.entityName`.
472+
- Always set `orgSlugMatchBehavior` on `dispatchOrgScopedList` to declare how bare-slug org matches are handled. Use `"redirect"` for commands where listing all entities in the org makes sense (e.g., `project list`, `team list`). Use `"error"` for commands with custom per-project logic that can't auto-redirect (e.g., `issue list`). The pre-check uses cached orgs to avoid N API calls — individual handlers don't need their own org-slug fallback.
472473
473474
3. **Standalone list commands** (e.g., `span list`, `trace list`) that don't use org-scoped dispatch wire pagination directly in `func()`. See the "List Command Pagination" section above for the pattern.
474475

src/commands/project/list.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,7 @@ export const listCommand = buildListCommand("project", {
644644
cwd,
645645
flags,
646646
parsed,
647+
orgSlugMatchBehavior: "redirect",
647648
overrides: {
648649
"auto-detect": (ctx) => handleAutoDetect(ctx.cwd, flags),
649650
explicit: (ctx) =>

src/lib/list-command.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -579,6 +579,7 @@ export function buildOrgListCommand<TEntity, TWithOrg>(
579579
cwd,
580580
flags,
581581
parsed,
582+
orgSlugMatchBehavior: "redirect",
582583
});
583584
yield new CommandOutput(result);
584585
// Only forward hint to the footer when items exist — empty results

src/lib/org-list.ts

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -839,8 +839,63 @@ export type DispatchOptions<TEntity = unknown, TWithOrg = unknown> = {
839839
* `issue list`) can list those mode types here to bypass the guard.
840840
*/
841841
allowCursorInModes?: readonly ParsedOrgProject["type"][];
842+
/**
843+
* Behavior when a project-search slug matches a cached organization.
844+
*
845+
* Before cursor validation and handler dispatch, the dispatcher checks
846+
* whether the bare slug matches a cached org. This prevents commands
847+
* from accidentally treating an org slug as a project name.
848+
*
849+
* - `"redirect"` (default): Convert to org-all mode with a warning log.
850+
* The existing org-all handler runs naturally.
851+
* - `"error"`: Throw a {@link ResolutionError} with actionable hints.
852+
* Use this when org-all redirect is inappropriate (e.g., issue list
853+
* has custom per-project query logic that doesn't support org-all).
854+
*
855+
* Cache-only: if the cache is cold or stale, the check is skipped and
856+
* the handler's own org-slug check serves as a safety net.
857+
*/
858+
orgSlugMatchBehavior?: "redirect" | "error";
842859
};
843860

861+
/**
862+
* Pre-check: when a bare slug matches a cached organization, redirect to
863+
* org-all mode or throw an error before handler dispatch. This prevents
864+
* commands from accidentally treating an org slug as a project name (CLI-9A).
865+
*
866+
* Cache-only: if the cache is cold or stale, returns the original parsed
867+
* value and the handler's own org-slug check serves as a safety net.
868+
*/
869+
async function resolveOrgSlugMatch(
870+
parsed: ParsedOrgProject & { type: "project-search" },
871+
behavior: "redirect" | "error",
872+
config: ListCommandMeta
873+
): Promise<ParsedOrgProject> {
874+
const slug = parsed.projectSlug;
875+
const { getCachedOrganizations } = await import("./db/regions.js");
876+
const cachedOrgs = await getCachedOrganizations();
877+
const matchingOrg = cachedOrgs.find((o) => o.slug === slug);
878+
if (!matchingOrg) {
879+
return parsed;
880+
}
881+
if (behavior === "error") {
882+
throw new ResolutionError(
883+
`'${slug}'`,
884+
"is an organization, not a project",
885+
`${config.commandPrefix} ${slug}/`,
886+
[
887+
`List projects: sentry project list ${slug}/`,
888+
`Specify a project: ${config.commandPrefix} ${slug}/<project>`,
889+
]
890+
);
891+
}
892+
log.warn(
893+
`'${slug}' is an organization, not a project. ` +
894+
`Listing all ${config.entityPlural} in '${slug}'.`
895+
);
896+
return { type: "org-all", org: matchingOrg.slug };
897+
}
898+
844899
/**
845900
* Validate the cursor flag and dispatch to the correct mode handler.
846901
*
@@ -859,14 +914,27 @@ export async function dispatchOrgScopedList<TEntity, TWithOrg>(
859914
): Promise<ListResult<any>> {
860915
const { config, cwd, flags, parsed, overrides } = options;
861916

917+
let effectiveParsed: ParsedOrgProject = parsed;
918+
919+
if (
920+
effectiveParsed.type === "project-search" &&
921+
options.orgSlugMatchBehavior
922+
) {
923+
effectiveParsed = await resolveOrgSlugMatch(
924+
effectiveParsed,
925+
options.orgSlugMatchBehavior,
926+
config
927+
);
928+
}
929+
862930
const cursorAllowedModes: readonly ParsedOrgProject["type"][] = [
863931
"org-all",
864932
...(options.allowCursorInModes ?? []),
865933
];
866-
if (flags.cursor && !cursorAllowedModes.includes(parsed.type)) {
934+
if (flags.cursor && !cursorAllowedModes.includes(effectiveParsed.type)) {
867935
const hint =
868-
parsed.type === "project-search"
869-
? `\n\nDid you mean '${config.commandPrefix} ${parsed.projectSlug}/'? ` +
936+
effectiveParsed.type === "project-search"
937+
? `\n\nDid you mean '${config.commandPrefix} ${effectiveParsed.projectSlug}/'? ` +
870938
`A bare name searches for a project — add a trailing slash to list an org's ${config.entityPlural}.`
871939
: "";
872940
throw new ValidationError(
@@ -877,11 +945,13 @@ export async function dispatchOrgScopedList<TEntity, TWithOrg>(
877945
);
878946
}
879947

880-
let effectiveParsed: ParsedOrgProject = parsed;
881-
if (parsed.type === "explicit" || parsed.type === "org-all") {
882-
const effectiveOrg = await resolveEffectiveOrg(parsed.org);
883-
if (effectiveOrg !== parsed.org) {
884-
effectiveParsed = { ...parsed, org: effectiveOrg };
948+
if (
949+
effectiveParsed.type === "explicit" ||
950+
effectiveParsed.type === "org-all"
951+
) {
952+
const effectiveOrg = await resolveEffectiveOrg(effectiveParsed.org);
953+
if (effectiveOrg !== effectiveParsed.org) {
954+
effectiveParsed = { ...effectiveParsed, org: effectiveOrg };
885955
}
886956
}
887957

test/lib/org-list.test.ts

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ import * as apiClient from "../../src/lib/api-client.js";
2121
import * as defaults from "../../src/lib/db/defaults.js";
2222
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
2323
import * as paginationDb from "../../src/lib/db/pagination.js";
24+
// biome-ignore lint/performance/noNamespaceImport: needed for spyOn mocking
25+
import * as regions from "../../src/lib/db/regions.js";
2426
import {
2527
AuthError,
2628
ResolutionError,
@@ -901,4 +903,171 @@ describe("dispatchOrgScopedList", () => {
901903
localSetPaginationSpy.mockRestore();
902904
localClearPaginationSpy.mockRestore();
903905
});
906+
907+
// -------------------------------------------------------------------------
908+
// orgSlugMatchBehavior pre-check
909+
// -------------------------------------------------------------------------
910+
911+
describe("orgSlugMatchBehavior", () => {
912+
let getCachedOrgsSpy: ReturnType<typeof spyOn>;
913+
914+
beforeEach(() => {
915+
getCachedOrgsSpy = spyOn(
916+
regions,
917+
"getCachedOrganizations"
918+
).mockResolvedValue([]);
919+
});
920+
921+
afterEach(() => {
922+
getCachedOrgsSpy.mockRestore();
923+
});
924+
925+
test("redirect converts project-search to org-all when slug matches cached org", async () => {
926+
getCachedOrgsSpy.mockResolvedValue([
927+
{ slug: "acme-corp", id: "1", name: "Acme Corp" },
928+
]);
929+
930+
const items: FakeEntity[] = [{ id: "1", name: "Widget A" }];
931+
const config = makeConfig({
932+
listPaginated: mock(() =>
933+
Promise.resolve({ data: items, nextCursor: undefined })
934+
),
935+
});
936+
937+
const result = await dispatchOrgScopedList({
938+
config,
939+
cwd: "/tmp",
940+
flags: { limit: 10, json: true },
941+
parsed: { type: "project-search", projectSlug: "acme-corp" },
942+
orgSlugMatchBehavior: "redirect",
943+
});
944+
945+
// Should have redirected to org-all → listPaginated called
946+
expect(config.listPaginated).toHaveBeenCalled();
947+
expect(result.items).toHaveLength(1);
948+
expect(result.items[0].orgSlug).toBe("acme-corp");
949+
});
950+
951+
test("error throws ResolutionError when slug matches cached org", async () => {
952+
getCachedOrgsSpy.mockResolvedValue([
953+
{ slug: "acme-corp", id: "1", name: "Acme Corp" },
954+
]);
955+
956+
const config = makeConfig();
957+
958+
await expect(
959+
dispatchOrgScopedList({
960+
config,
961+
cwd: "/tmp",
962+
flags: { limit: 10, json: false },
963+
parsed: { type: "project-search", projectSlug: "acme-corp" },
964+
orgSlugMatchBehavior: "error",
965+
})
966+
).rejects.toThrow(ResolutionError);
967+
});
968+
969+
test("error message includes actionable hints", async () => {
970+
getCachedOrgsSpy.mockResolvedValue([
971+
{ slug: "acme-corp", id: "1", name: "Acme Corp" },
972+
]);
973+
974+
const config = makeConfig();
975+
976+
try {
977+
await dispatchOrgScopedList({
978+
config,
979+
cwd: "/tmp",
980+
flags: { limit: 10, json: false },
981+
parsed: { type: "project-search", projectSlug: "acme-corp" },
982+
orgSlugMatchBehavior: "error",
983+
});
984+
expect.unreachable("should have thrown");
985+
} catch (e) {
986+
expect(e).toBeInstanceOf(ResolutionError);
987+
const err = e as ResolutionError;
988+
expect(err.message).toContain("is an organization, not a project");
989+
expect(err.message).toContain("acme-corp/");
990+
}
991+
});
992+
993+
test("no orgSlugMatchBehavior skips pre-check and calls handler", async () => {
994+
getCachedOrgsSpy.mockResolvedValue([
995+
{ slug: "acme-corp", id: "1", name: "Acme Corp" },
996+
]);
997+
998+
const handler = mock(() =>
999+
Promise.resolve({ items: [] } as ListResult<FakeWithOrg>)
1000+
);
1001+
1002+
await dispatchOrgScopedList({
1003+
config: META_ONLY,
1004+
cwd: "/tmp",
1005+
flags: { limit: 10, json: false },
1006+
parsed: { type: "project-search", projectSlug: "acme-corp" },
1007+
overrides: {
1008+
"auto-detect": handler,
1009+
explicit: handler,
1010+
"project-search": handler,
1011+
"org-all": handler,
1012+
},
1013+
});
1014+
1015+
// Without orgSlugMatchBehavior, the project-search handler runs
1016+
expect(handler).toHaveBeenCalledTimes(1);
1017+
// getCachedOrganizations should NOT have been called
1018+
expect(getCachedOrgsSpy).not.toHaveBeenCalled();
1019+
});
1020+
1021+
test("redirect with no cache match falls through to project-search handler", async () => {
1022+
getCachedOrgsSpy.mockResolvedValue([
1023+
{ slug: "other-org", id: "2", name: "Other Org" },
1024+
]);
1025+
1026+
const handler = mock(() =>
1027+
Promise.resolve({ items: [] } as ListResult<FakeWithOrg>)
1028+
);
1029+
1030+
await dispatchOrgScopedList({
1031+
config: META_ONLY,
1032+
cwd: "/tmp",
1033+
flags: { limit: 10, json: false },
1034+
parsed: { type: "project-search", projectSlug: "acme-corp" },
1035+
orgSlugMatchBehavior: "redirect",
1036+
overrides: {
1037+
"auto-detect": handler,
1038+
explicit: handler,
1039+
"project-search": handler,
1040+
"org-all": handler,
1041+
},
1042+
});
1043+
1044+
// No cache match → project-search handler still called
1045+
expect(handler).toHaveBeenCalledTimes(1);
1046+
});
1047+
1048+
test("redirect with empty cache falls through to project-search handler", async () => {
1049+
getCachedOrgsSpy.mockResolvedValue([]);
1050+
1051+
const handler = mock(() =>
1052+
Promise.resolve({ items: [] } as ListResult<FakeWithOrg>)
1053+
);
1054+
1055+
await dispatchOrgScopedList({
1056+
config: META_ONLY,
1057+
cwd: "/tmp",
1058+
flags: { limit: 10, json: false },
1059+
parsed: { type: "project-search", projectSlug: "acme-corp" },
1060+
orgSlugMatchBehavior: "redirect",
1061+
overrides: {
1062+
"auto-detect": handler,
1063+
explicit: handler,
1064+
"project-search": handler,
1065+
"org-all": handler,
1066+
},
1067+
});
1068+
1069+
// Empty cache → project-search handler still called
1070+
expect(handler).toHaveBeenCalledTimes(1);
1071+
});
1072+
});
9041073
});

0 commit comments

Comments
 (0)