fix: consolidate Cursor BugBot PRs (#908, #947, #973, #1023, #1044)

AGENTS.md

@@ -510,7 +510,7 @@ CliError (base, exitCode=1)
 - Pass `alternatives: []` when defaults are irrelevant (e.g., for missing Trace ID, Event ID)
 - Use `" and "` in `resource` for plural grammar: `"Trace ID and span ID"` → "are required"
 
-**CI enforcement:** `bun run check:errors` scans for `ContextError` with multiline commands and `CliError` with ad-hoc "Try:" strings.
+**CI enforcement:** `bun run check:errors` scans for `ContextError` with multiline commands, `CliError` with ad-hoc "Try:" strings, and silent `catch` blocks (advisory).
 
 ```typescript
 // Usage examples
@@ -554,6 +554,12 @@ catch (error) {
 
 Use `logger.withTag("command-name")` for tagged logging in command files.
 
+**CI enforcement:** `bun run check:errors` includes a silent-catch scan that flags
+`catch` blocks which are empty, comment-only, or return-only without surfacing the
+error. It is currently **advisory** (warns, does not fail CI) because of a pre-existing
+backlog; run with `SENTRY_STRICT_SILENT_CATCH=1` to enforce. Do not add new silent
+catches — they will appear in the scan output during review.
+
 ### Auto-Recovery for Wrong Entity Types
 
 When a user provides the wrong type of identifier (e.g., an issue short ID
@@ -961,6 +967,38 @@ mock.module("./some-module", () => ({
 | Add documentation | `docs/src/content/docs/` |
 | Hand-written command doc content | `docs/src/fragments/commands/` |
 
+## Automated Fix PRs (BugBot / agents)
+
+Automated bug-fix PRs (e.g. Cursor BugBot) must follow these rules to avoid the
+duplication and staleness that caused five overlapping PRs to pile up:
+
+1. **Check for existing work first.** Before opening a PR, search open PRs and
+   recently-closed PRs/issues for the same file + symbol:
+   ```bash
+   gh pr list --state open --search "in:title <file-or-symbol>"
+   gh issue list --state all --search "<symbol>"
+   ```
+   If an open PR already touches the target function, **comment on it** or extend
+   it instead of opening a duplicate. Multiple BugBot PRs independently re-fixed
+   the same `JSON.parse` guard, `withTTY` helper, and pagination code.
+
+2. **Rebase before review.** A PR that is many commits behind `main` may fail CI
+   on unrelated drift (e.g. a lint error in a file the PR never touched) and its
+   fix may already be superseded. Rebase onto `main` and re-verify the bug still
+   exists before requesting review. Verify against current `main`, not the
+   snapshot the PR was generated from.
+
+3. **Separate correctness fixes from opinion.** A real bug (wrong output, crash,
+   skipped data) is in scope. A subjective UX change (different hint wording,
+   different default) is **not** a bug — `main`'s current behavior is often
+   deliberate. Do not bundle UX opinions into bug-fix PRs; they waste review
+   cycles and are usually dropped.
+
+4. **Prefer shared helpers over re-deriving fixes.** If a correct implementation
+   already exists (e.g. `autoPaginate()` for pagination, `safeParseJson()` for
+   cached JSON), use it rather than hand-rolling a one-off fix. The recurring
+   pagination-overshoot and parse-crash bugs were classes solved once centrally.
+
 <!-- This section is maintained by the coding agent via lore (https://github.com/BYK/loreai) -->
 ## Long-term Knowledge
 

script/check-error-patterns.ts

@@ -10,6 +10,12 @@
  * 2. `new CliError(... "Try:" ...)` — ad-hoc "Try:" strings
  *    → Should use ResolutionError with structured hint/suggestions
  *
+ * 3. Silent catch blocks — `catch { ... }` whose body has no logging, no
+ *    re-throw, and at most a bare `return`. Errors must be surfaced via
+ *    `log.debug`/`log.warn` (or re-thrown) per AGENTS.md. Biome's
+ *    `noEmptyBlockStatements` only catches syntactically empty `catch {}`;
+ *    this catches comment-only and return-only blocks too.
+ *
  * Usage:
  *   tsx script/check-error-patterns.ts
  *
@@ -27,8 +33,20 @@
 const TRY_PATTERN_RE = /["'`]Try:/;
 
 const files = await glob("src/**/*.ts");
+
+/** Hard violations — these fail CI. */
 const violations: Violation[] = [];
 
+/**
+ * Advisory silent-catch findings. Reported as warnings but do NOT fail CI yet:
+ * the repo has a pre-existing backlog of intentional best-effort catches (e.g.
+ * UI teardown, cleanup paths). The check surfaces them for incremental cleanup
+ * and so new ones are visible in review. Set SENTRY_STRICT_SILENT_CATCH=1 to
+ * promote them to hard failures once the backlog is cleared.
+ */
+const silentCatchWarnings: Violation[] = [];
+const STRICT_SILENT_CATCH = process.env.SENTRY_STRICT_SILENT_CATCH === "1";
+
 /** Characters that open a nesting level in JavaScript source. */
 function isOpener(ch: string): boolean {
   return ch === "(" || ch === "[" || ch === "{";
@@ -255,10 +273,100 @@
   }
 }
 
+/** Matches the start of a catch block in both statement and promise form. */
+const CATCH_RE =
+  /\bcatch\s*(?:\(\s*(\w+)[^)]*\)\s*)?\{|\.catch\(\s*(?:\(\s*(\w+)[^)]*\)|(\w+))\s*=>\s*\{/g;
+
+/** Tokens inside a catch body that prove the error is surfaced (not silenced). */
+const SURFACING_RE =
+  /\b(?:log|logger|console)\s*\.|[^.]\bthrow\b|captureException|reportError/;
+
+/** A catch body consisting solely of a single `return ...;` statement. */
+const RETURN_ONLY_RE = /^return\b[^;]*;?$/;
+
+/**
+ * Return the source of a balanced `{...}` block given the index of its opening
+ * brace, skipping strings so braces inside literals don't break depth tracking.
+ */
+function readBlock(content: string, openBraceIdx: number): string {
+  let depth = 0;
+  let i = openBraceIdx;
+  while (i < content.length) {
+    const { next, ch } = advanceToken(content, i);
+    if (ch === "{") {
+      depth += 1;
+    } else if (ch === "}") {
+      depth -= 1;
+      if (depth === 0) {
+        return content.slice(openBraceIdx + 1, i);
+      }
+    }
+    i = next;
+  }
+  return content.slice(openBraceIdx + 1);
+}
+
+/**
+ * Strip line and block comments from a snippet so comment-only catch bodies are
+ * treated as empty.
+ */
+function stripComments(snippet: string): string {
+  return snippet.replace(/\/\*[\s\S]*?\*\//g, "").replace(/\/\/[^\n]*/g, "");
+}
+
+/**
+ * Detect silent catch blocks: catch bodies that, after removing comments, are
+ * empty or contain only a bare `return;`/`return <value>;` with no logging or
+ * re-throw. These hide errors and violate the AGENTS.md no-silent-catch rule.
+ */
+function checkSilentCatch(content: string, filePath: string): void {
+  let match = CATCH_RE.exec(content);
+  while (match !== null) {
+    const openBraceIdx = match.index + match[0].length - 1;
+    const errorParam = match[1] ?? match[2] ?? match[3];
+    const body = readBlock(content, openBraceIdx);
+    const code = stripComments(body).trim();
+    // A body that references the caught error identifier (forwarding it to a
+    // handler, attaching it, etc.) is not "silent" even if it lacks an explicit
+    // log/throw — avoids false positives like `return handleFetchError(error)`.
+    const usesError =
+      errorParam !== undefined && new RegExp(`\\b${errorParam}\\b`).test(code);
+    const returnOnly = RETURN_ONLY_RE.test(code);
+    const silent =
+      !(SURFACING_RE.test(code) || usesError) &&
+      (code.length === 0 || returnOnly);
+    if (silent) {
+      const line = content.slice(0, match.index).split("\n").length;
+      const target = STRICT_SILENT_CATCH ? violations : silentCatchWarnings;
+      target.push({
+        file: filePath,
+        line,
+        message:
+          "Silent catch block. Add log.debug()/log.warn() or re-throw — errors must not vanish (AGENTS.md).",
+      });
+    }
+    match = CATCH_RE.exec(content);
+  }
+}
+
 for (const filePath of files) {
   const content = await readFile(filePath, "utf-8");
   checkContextErrorNewlines(content, filePath);
   checkAdHocTryPatterns(content, filePath);
+  checkSilentCatch(content, filePath);
+}
+
+if (silentCatchWarnings.length > 0) {
+  console.warn(
+    `⚠ ${silentCatchWarnings.length} silent catch block(s) found (advisory; not failing CI).`
+  );
+  console.warn(
+    "  Add log.debug()/log.warn() or re-throw. Run with SENTRY_STRICT_SILENT_CATCH=1 to enforce.\n"
+  );
+  for (const v of silentCatchWarnings) {
+    console.warn(`  ${v.file}:${v.line}`);
+  }
+  console.warn("");
 }
 
 if (violations.length === 0) {

src/commands/release/set-commits.ts

@@ -6,6 +6,7 @@
 
 import type { SentryContext } from "../../context.js";
 import {
+  NO_REPO_INTEGRATIONS_MESSAGE,
   setCommitsAuto,
   setCommitsLocal,
   setCommitsWithRefs,
@@ -134,7 +135,17 @@ async function setCommitsDefault(
     clearRepoIntegrationCache(org);
     return release;
   } catch (error) {
-    if (error instanceof ApiError && error.status === 400) {
+    // Only fall back to local git when the org genuinely has no repository
+    // integration. setCommitsAuto internally calls setCommitsWithRefs, which can
+    // return a server 400 for unrelated reasons (invalid refs, bad release
+    // state) — those must propagate, not be masked as "no integration" (which
+    // would also poison the cache). Match on the exact client-side message since
+    // the API exposes no stable error code for this case.
+    if (
+      error instanceof ApiError &&
+      error.status === 400 &&
+      error.message === NO_REPO_INTEGRATIONS_MESSAGE
+    ) {
       cacheNoRepoIntegration(org);
       log.warn(
         "Could not auto-discover commits (no repository integration). " +

src/lib/api-client.ts

@@ -120,6 +120,7 @@ export {
   listReleaseDeploys,
   listReleasesForProject,
   listReleasesPaginated,
+  NO_REPO_INTEGRATIONS_MESSAGE,
   type ReleaseSortValue,
   setCommitsAuto,
   setCommitsLocal,

src/lib/api/events.ts

@@ -199,7 +199,19 @@ export type ListIssueEventsOptions = {
  *
  * Uses the SDK's `listAnIssue_sEvents` endpoint with region-aware routing.
  * When `limit` exceeds {@link API_MAX_PER_PAGE} (100), auto-paginates through
- * multiple API calls to fill the requested limit, bounded by {@link MAX_PAGINATION_PAGES}.
+ * multiple API calls to fill the requested limit, bounded by
+ * {@link MAX_PAGINATION_PAGES}.
+ *
+ * Page size is capped at `min(limit, API_MAX_PER_PAGE)` via `per_page`, which
+ * Sentry accepts on this route at runtime even though it is absent from the
+ * OpenAPI spec. Capping page size keeps the server-issued `nextCursor` aligned
+ * to a page boundary, preventing the skip bug where trim + keep-cursor would
+ * jump past items.
+ *
+ * When trimming to `limit`, `nextCursor` is PRESERVED: the events cursor is
+ * offset-based, so resuming from it re-includes any trimmed tail rather than
+ * skipping it. This prevents both the original skip bug and the stall
+ * (drop-cursor) regression.
  *
  * @param orgSlug - Organization slug for region routing
  * @param issueId - Numeric issue ID
@@ -214,12 +226,13 @@ export async function listIssueEvents(
   const { limit = 25, query, full, cursor, statsPeriod, start, end } = options;
 
   const config = await getOrgSdkConfig(orgSlug);
+  const perPage = Math.min(limit, API_MAX_PER_PAGE);
 
   const allEvents: IssueEvent[] = [];
   let currentCursor = cursor;
   let nextCursor: string | undefined;
 
-  for (let page = 0; page < MAX_PAGINATION_PAGES; page++) {
+  for (let page = 0; page < MAX_PAGINATION_PAGES; page += 1) {
     const result = await listAnIssue_sEvents({
       ...config,
       path: {
@@ -233,7 +246,10 @@ export async function listIssueEvents(
         statsPeriod,
         start,
         end,
-      },
+        // `per_page` is accepted at runtime but absent from the generated query
+        // type, so widen via cast.
+        per_page: perPage,
+      } as Parameters<typeof listAnIssue_sEvents>[0]["query"],
     });
 
     const paginated = unwrapPaginatedResult(
@@ -250,12 +266,10 @@ export async function listIssueEvents(
     currentCursor = nextCursor;
   }
 
-  // Trim to exact limit. Unlike listIssuesAllPages (which controls per_page),
-  // the issue events endpoint has no per-page parameter, so the API may return
-  // more items than requested. We preserve nextCursor so the command-level
-  // cursor stack can navigate to subsequent pages.
-  const trimmed =
-    allEvents.length > limit ? allEvents.slice(0, limit) : allEvents;
-
-  return { data: trimmed, nextCursor };
+  // Trim to limit but PRESERVE nextCursor. The events cursor is offset-based,
+  // so resuming from it re-includes any trimmed tail rather than skipping it.
+  // Preserving the cursor lets `-c next` advance; dropping it would strand all
+  // events past the first page.
+  const data = allEvents.length > limit ? allEvents.slice(0, limit) : allEvents;
+  return { data, nextCursor };
 }

src/lib/api/releases.ts

@@ -402,6 +402,18 @@ async function getPreviousReleaseCommit(
  * Set commits on a release using auto-discovery mode.
  *
  * Lists the org's repositories from the Sentry API, matches against the
+/**
+ * Message of the client-side {@link ApiError} thrown by {@link setCommitsAuto}
+ * when the org has no repository integrations. Exported so callers can
+ * distinguish this specific no-repo-integration case from unrelated 400s
+ * returned by the server (e.g. invalid commit refs), which must not be masked
+ * as "no integration". Matched on `message` (not `detail`) because this error is
+ * constructed client-side with `detail: undefined`.
+ */
+export const NO_REPO_INTEGRATIONS_MESSAGE =
+  "No repository integrations configured for this organization.";
+
+/**
  * local git remote URL to find the corresponding Sentry repo, then sends
  * a refs payload with the HEAD commit SHA. This is the equivalent of the
  * reference sentry-cli's `--auto` mode.
@@ -470,12 +482,7 @@ export async function setCommitsAuto(
 
   if (!foundAnyRepos) {
     const endpoint = `organizations/${orgSlug}/releases/${encodeURIComponent(version)}/`;
-    throw new ApiError(
-      "No repository integrations configured for this organization.",
-      400,
-      undefined,
-      endpoint
-    );
+    throw new ApiError(NO_REPO_INTEGRATIONS_MESSAGE, 400, undefined, endpoint);
   }
 
   throw new ValidationError(

src/lib/api/seer.ts

@@ -19,8 +19,14 @@ const EXPLORER_MODE_PARAMS = { mode: "explorer" };
  * Normalize agent status values to the uppercase format used throughout the CLI.
  *
  * The agent endpoint returns lowercase statuses (`processing`, `completed`,
- * `error`, `awaiting_user_input`) while the CLI expects uppercase
- * (`PROCESSING`, `COMPLETED`, `ERROR`, `WAITING_FOR_USER_RESPONSE`).
+ * `error`, `awaiting_user_input`, `canceled`) while the CLI expects uppercase
+ * (`PROCESSING`, `COMPLETED`, `ERROR`, `WAITING_FOR_USER_RESPONSE`, `CANCELLED`).
+ *
+ * Explicit cases are required for any status whose CLI name differs from a naive
+ * `toUpperCase()`. In particular `canceled` (US spelling) must map to `CANCELLED`
+ * (British, used in TERMINAL_STATUSES) — otherwise `isTerminalStatus("CANCELED")`
+ * returns false and polling spins until timeout. `awaiting_user_input` maps to
+ * `WAITING_FOR_USER_RESPONSE`.
  */
 function normalizeAgentStatus(status: string): string {
   switch (status) {
@@ -30,8 +36,13 @@ function normalizeAgentStatus(status: string): string {
       return "COMPLETED";
     case "error":
       return "ERROR";
+    case "canceled":
+    case "cancelled":
+      return "CANCELLED";
     case "awaiting_user_input":
       return "WAITING_FOR_USER_RESPONSE";
+    case "need_more_information":
+      return "NEED_MORE_INFORMATION";
     default:
       return status.toUpperCase();
   }