fix: address all bot review comments from PR #262

BYK · BYK · commit 1e4ebe09b904 · 2026-02-18T13:01:53.000Z
- Fix malformed 'project:' filter: empty projectSlug now produces empty
  string (falsy) so .filter(Boolean) correctly removes it for org-wide listing
- Fix isMultiProject=false in org-all issue listing: set to true so the ALIAS
  column shows which project each issue belongs to
- Fix missing hasMore check on empty page in issue list org-all path: show
  'try the next page' hint instead of definitive 'no issues' when hasMore=true
- Remove redundant footer tip from handleOrgAll in repo/list and team/list:
  the tip told users to do what they already did
- Deduplicate buildContextKey/resolveCursor: extract buildOrgContextKey and
  resolveOrgCursor into src/lib/db/pagination.ts and update all callers
diff --git a/src/commands/issue/list.ts b/src/commands/issue/list.ts
@@ -17,7 +17,7 @@ import { parseOrgProjectArg } from "../../lib/arg-parsing.js";
 import { buildCommand, numberParser } from "../../lib/command.js";
 import {
   clearPaginationCursor,
-  getPaginationCursor,
+  resolveOrgCursor,
   setPaginationCursor,
 } from "../../lib/db/pagination.js";
 import {
@@ -468,22 +468,9 @@ export const listCommand = buildCommand({
     // Handle org-all mode with cursor pagination (different code path)
     if (parsed.type === "org-all") {
       const org = parsed.org;
+      // Issue cursors encode sort+query so different searches don't share pages.
       const contextKey = `host:${getApiBaseUrl()}|type:org:${org}|sort:${flags.sort}${flags.query ? `|q:${flags.query}` : ""}`;
-      let cursor: string | undefined;
-      if (flags.cursor) {
-        if (flags.cursor === "last") {
-          const cached = getPaginationCursor(PAGINATION_KEY, contextKey);
-          if (!cached) {
-            throw new ContextError(
-              "Pagination cursor",
-              "No saved cursor for this query. Run without --cursor first."
-            );
-          }
-          cursor = cached;
-        } else {
-          cursor = flags.cursor;
-        }
-      }
+      const cursor = resolveOrgCursor(flags.cursor, PAGINATION_KEY, contextKey);
 
       setContext([org], []);
 
@@ -518,17 +505,24 @@ export const listCommand = buildCommand({
       }
 
       if (response.data.length === 0) {
-        stdout.write(`No issues found in organization '${org}'.\n`);
+        if (hasMore) {
+          const hint = `sentry issue list ${org}/ -c last`;
+          stdout.write(`No issues on this page. Try the next page: ${hint}\n`);
+        } else {
+          stdout.write(`No issues found in organization '${org}'.\n`);
+        }
         return;
       }
 
       writeListHeader(stdout, `Issues in ${org}`, false);
       const termWidth = process.stdout.columns || 80;
+      // isMultiProject=true so the ALIAS column shows which project each issue
+      // belongs to — essential when viewing issues across an entire org.
       const issuesWithOpts = response.data.map((issue) => ({
         issue,
         formatOptions: {
           projectSlug: issue.project?.slug ?? "",
-          isMultiProject: false,
+          isMultiProject: true,
         },
       }));
       writeIssueRows(stdout, issuesWithOpts, termWidth);
diff --git a/src/commands/repo/list.ts b/src/commands/repo/list.ts
@@ -20,14 +20,14 @@ import {
 import { parseOrgProjectArg } from "../../lib/arg-parsing.js";
 import { buildCommand, numberParser } from "../../lib/command.js";
 import {
+  buildOrgContextKey,
   clearPaginationCursor,
-  getPaginationCursor,
+  resolveOrgCursor,
   setPaginationCursor,
 } from "../../lib/db/pagination.js";
-import { AuthError, ContextError, ValidationError } from "../../lib/errors.js";
+import { AuthError, ValidationError } from "../../lib/errors.js";
 import { writeFooter, writeJson } from "../../lib/formatters/index.js";
 import { resolveOrgsForListing } from "../../lib/resolve-target.js";
-import { getApiBaseUrl } from "../../lib/sentry-client.js";
 import type { SentryRepository, Writer } from "../../types/index.js";
 
 /** Command key for pagination cursor storage */
@@ -143,38 +143,6 @@ async function fetchAllOrgRepositories(): Promise<RepositoryWithOrg[]> {
   return results;
 }
 
-/**
- * Build a context key for pagination cursor validation.
- * Captures the org so cursors from different orgs are never mixed.
- */
-function buildContextKey(org: string): string {
-  return `host:${getApiBaseUrl()}|type:org:${org}`;
-}
-
-/**
- * Resolve the cursor value from --cursor flag.
- * Handles the magic "last" value by looking up the cached cursor.
- */
-function resolveCursor(
-  cursorFlag: string | undefined,
-  contextKey: string
-): string | undefined {
-  if (!cursorFlag) {
-    return;
-  }
-  if (cursorFlag === "last") {
-    const cached = getPaginationCursor(PAGINATION_KEY, contextKey);
-    if (!cached) {
-      throw new ContextError(
-        "Pagination cursor",
-        "No saved cursor for this query. Run without --cursor first."
-      );
-    }
-    return cached;
-  }
-  return cursorFlag;
-}
-
 /** Build the CLI hint for fetching the next page. */
 function nextPageHint(org: string): string {
   return `sentry repo list ${org}/ -c last`;
@@ -237,11 +205,6 @@ async function handleOrgAll(options: OrgAllOptions): Promise<void> {
   } else {
     stdout.write(`\nShowing ${repos.length} repositories\n`);
   }
-
-  writeFooter(
-    stdout,
-    "Tip: Use 'sentry repo list <org>/' for paginated results"
-  );
 }
 
 /**
@@ -435,8 +398,12 @@ export const listCommand = buildCommand({
         break;
 
       case "org-all": {
-        const contextKey = buildContextKey(parsed.org);
-        const cursor = resolveCursor(flags.cursor, contextKey);
+        const contextKey = buildOrgContextKey(parsed.org);
+        const cursor = resolveOrgCursor(
+          flags.cursor,
+          PAGINATION_KEY,
+          contextKey
+        );
         await handleOrgAll({
           stdout,
           org: parsed.org,
diff --git a/src/commands/team/list.ts b/src/commands/team/list.ts
@@ -20,14 +20,14 @@ import {
 import { parseOrgProjectArg } from "../../lib/arg-parsing.js";
 import { buildCommand, numberParser } from "../../lib/command.js";
 import {
+  buildOrgContextKey,
   clearPaginationCursor,
-  getPaginationCursor,
+  resolveOrgCursor,
   setPaginationCursor,
 } from "../../lib/db/pagination.js";
-import { AuthError, ContextError, ValidationError } from "../../lib/errors.js";
+import { AuthError, ValidationError } from "../../lib/errors.js";
 import { writeFooter, writeJson } from "../../lib/formatters/index.js";
 import { resolveOrgsForListing } from "../../lib/resolve-target.js";
-import { getApiBaseUrl } from "../../lib/sentry-client.js";
 import type { SentryTeam, Writer } from "../../types/index.js";
 
 /** Command key for pagination cursor storage */
@@ -140,38 +140,6 @@ async function fetchAllOrgTeams(): Promise<TeamWithOrg[]> {
   return results;
 }
 
-/**
- * Build a context key for pagination cursor validation.
- * Captures the org so cursors from different orgs are never mixed.
- */
-function buildContextKey(org: string): string {
-  return `host:${getApiBaseUrl()}|type:org:${org}`;
-}
-
-/**
- * Resolve the cursor value from --cursor flag.
- * Handles the magic "last" value by looking up the cached cursor.
- */
-function resolveCursor(
-  cursorFlag: string | undefined,
-  contextKey: string
-): string | undefined {
-  if (!cursorFlag) {
-    return;
-  }
-  if (cursorFlag === "last") {
-    const cached = getPaginationCursor(PAGINATION_KEY, contextKey);
-    if (!cached) {
-      throw new ContextError(
-        "Pagination cursor",
-        "No saved cursor for this query. Run without --cursor first."
-      );
-    }
-    return cached;
-  }
-  return cursorFlag;
-}
-
 /** Build the CLI hint for fetching the next page. */
 function nextPageHint(org: string): string {
   return `sentry team list ${org}/ -c last`;
@@ -236,11 +204,6 @@ async function handleOrgAll(options: OrgAllOptions): Promise<void> {
   } else {
     stdout.write(`\nShowing ${teams.length} teams\n`);
   }
-
-  writeFooter(
-    stdout,
-    "Tip: Use 'sentry team list <org>/' for paginated results"
-  );
 }
 
 /**
@@ -431,8 +394,12 @@ export const listCommand = buildCommand({
         break;
 
       case "org-all": {
-        const contextKey = buildContextKey(parsed.org);
-        const cursor = resolveCursor(flags.cursor, contextKey);
+        const contextKey = buildOrgContextKey(parsed.org);
+        const cursor = resolveOrgCursor(
+          flags.cursor,
+          PAGINATION_KEY,
+          contextKey
+        );
         await handleOrgAll({
           stdout,
           org: parsed.org,
diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts
@@ -1000,7 +1000,10 @@ export function listIssuesPaginated(
     statsPeriod?: string;
   } = {}
 ): Promise<PaginatedResponse<SentryIssue[]>> {
-  const projectFilter = `project:${projectSlug}`;
+  // Only add project filter when projectSlug is non-empty; an empty slug would
+  // produce "project:" (a truthy string that .filter(Boolean) won't remove),
+  // sending a malformed query to the API for org-wide listing.
+  const projectFilter = projectSlug ? `project:${projectSlug}` : "";
   const fullQuery = [projectFilter, options.query].filter(Boolean).join(" ");
 
   return orgScopedRequestPaginated<SentryIssue[]>(
diff --git a/src/lib/db/pagination.ts b/src/lib/db/pagination.ts
@@ -5,8 +5,13 @@
  * using a composite primary key so different contexts (e.g., different orgs)
  * maintain independent cursors.
  * Cursors expire after a short TTL to prevent stale pagination.
+ *
+ * Also exports shared helpers for building context keys and resolving cursor
+ * flags, used by list commands that support cursor-based pagination.
  */
 
+import { ContextError } from "../errors.js";
+import { getApiBaseUrl } from "../sentry-client.js";
 import { getDatabase } from "./index.js";
 import { runUpsert } from "./utils.js";
 
@@ -97,3 +102,49 @@ export function clearPaginationCursor(
     "DELETE FROM pagination_cursors WHERE command_key = ? AND context = ?"
   ).run(commandKey, context);
 }
+
+/**
+ * Build a context key for org-scoped pagination cursor storage.
+ *
+ * Encodes the API base URL and org slug so cursors from different hosts or
+ * orgs are never mixed up in the cursor cache.
+ *
+ * @param org - Organization slug
+ * @returns Composite context key string
+ */
+export function buildOrgContextKey(org: string): string {
+  return `host:${getApiBaseUrl()}|type:org:${org}`;
+}
+
+/**
+ * Resolve the cursor value from a `--cursor` flag.
+ *
+ * Handles the magic `"last"` value by looking up the cached cursor for the
+ * given context key. Throws a {@link ContextError} if `"last"` is requested
+ * but no cursor has been cached yet.
+ *
+ * @param cursorFlag - Raw value of the `--cursor` flag (undefined if not set)
+ * @param commandKey - Command identifier used for cursor storage
+ * @param contextKey - Serialized query context used for cursor storage
+ * @returns Resolved cursor string, or `undefined` if no cursor was specified
+ */
+export function resolveOrgCursor(
+  cursorFlag: string | undefined,
+  commandKey: string,
+  contextKey: string
+): string | undefined {
+  if (!cursorFlag) {
+    return;
+  }
+  if (cursorFlag === "last") {
+    const cached = getPaginationCursor(commandKey, contextKey);
+    if (!cached) {
+      throw new ContextError(
+        "Pagination cursor",
+        "No saved cursor for this query. Run without --cursor first."
+      );
+    }
+    return cached;
+  }
+  return cursorFlag;
+}
