fix: address review round 4 — simplify parallel(), add Sentry cache spans

Review comments addressed:

#1: issue/list.ts FRESH_ALIASES — verified no conflict, only log/list.ts
    has f: 'follow' and it doesn't use FRESH_ALIASES
#2: cacheHeuristic 0.1 vs CLEANUP_PROBABILITY — different semantics,
    not shared (RFC heuristic vs probabilistic cleanup trigger)
#3: Simplified parallel() — replaced custom wrapper with p-limit's
    built-in .map() method (cacheIO.map(items, fn)) at all 3 call sites
#4: evictExcessEntries/deleteExpiredEntries — run both in parallel via
    Promise.all() since they operate on disjoint file sets
#5: Added Sentry instrumentation — withCacheSpan() helper in telemetry.ts,
    cache.lookup and cache.store spans in response-cache.ts with URL attrs

Author: BYK

AGENTS.md

@@ -635,7 +635,7 @@ mock.module("./some-module", () => ({
 * **Sentry API: events require org+project, issues have legacy global endpoint**: Sentry API scoping: Events require org+project in URL path (\`/projects/{org}/{project}/events/{id}/\`). Issues use legacy global endpoint (\`/api/0/issues/{id}/\`) without org context. Traces need only org (\`/organizations/{org}/trace/{traceId}/\`). Two-step lookup for events: fetch issue → extract org/project from response → fetch event. Cross-project event search possible via Discover endpoint \`/organizations/{org}/events/\` with \`query=id:{eventId}\`.
 
 <!-- lore:019cb6ab-ab98-7a9c-a25f-e154a5adbbe1 -->
-* **Sentry CLI authenticated fetch architecture with response caching**: \`createAuthenticatedFetch()\` in \`sentry-client.ts\` wraps native \`fetch()\` with auth injection, 30s timeout, retry with backoff (max 2), 401 refresh, and HTTP span tracing. Response caching (src/lib/response-cache.ts) integrates BEFORE auth/retry via \`http-cache-semantics\` (RFC 7234) with filesystem storage at \`~/.sentry/cache/responses/\`. URL-based fallback TTL tiers: immutable (1hr), stable (5min), volatile (60s), no-cache (0). Only GET 2xx cached. \`--fresh\` flag and \`SENTRY\_NO\_CACHE=1\` bypass cache. Cache cleared on login/logout via \`clearAuth()\` → \`clearResponseCache()\`. Corrupted \`CachePolicy\` entries trigger cache miss with best-effort cleanup.
+* **Sentry CLI authenticated fetch architecture with response caching**: \`createAuthenticatedFetch()\` in \`sentry-client.ts\` wraps native \`fetch()\` with auth injection, 30s timeout, retry with backoff (max 2), 401 refresh, and HTTP span tracing. Response caching (src/lib/response-cache.ts) integrates BEFORE auth/retry via \`http-cache-semantics\` (RFC 7234) with filesystem storage at \`~/.sentry/cache/responses/\`. URL-based fallback TTL tiers: immutable (24hr), stable (5min), volatile (60s), no-cache (0). URL patterns stored as \`Record\<TtlTier, RegExp\[]>\`. Only GET 2xx cached. \`--fresh\` flag (with exported \`FRESH\_ALIASES\` constant shared across all 15+ command files) and \`SENTRY\_NO\_CACHE=1\` bypass cache. Cache entries store \`expiresAt\` for O(1) cleanup. All cache I/O parallelized with concurrency limiter (max 8). Cache cleared on login/logout via \`clearAuth()\` → \`clearResponseCache()\`. Corrupted \`CachePolicy\` entries trigger cache miss with best-effort cleanup.
 
 <!-- lore:019c8c72-b871-7d5e-a1a4-5214359a5a77 -->
 * **Sentry CLI has two distribution channels with different runtimes**: Sentry CLI ships two ways: (1) Standalone binary via \`Bun.build()\` with \`compile: true\`. (2) npm package via esbuild producing CJS \`dist/bin.cjs\` for Node 22+, with Bun API polyfills from \`script/node-polyfills.ts\`. \`Bun.$\` has NO polyfill — use \`execSync\` instead. \`require()\` in ESM is safe (Bun native, esbuild resolves at bundle time).

src/lib/response-cache.ts

@@ -28,6 +28,7 @@ import CachePolicy from "http-cache-semantics";
 import pLimit from "p-limit";
 
 import { getConfigDir } from "./db/index.js";
+import { withCacheSpan } from "./telemetry.js";
 
 // ---------------------------------------------------------------------------
 // TTL tiers — used as fallback when the server sends no cache headers
@@ -341,31 +342,37 @@ export async function getCachedResponse(
     return;
   }
 
-  const key = buildCacheKey(method, url);
-  const entry = await readCacheEntry(key);
-  if (!entry) {
-    return;
-  }
-
-  try {
-    const policy = CachePolicy.fromObject(entry.policy);
-    if (!isEntryFresh(policy, entry, requestHeaders, url)) {
-      return;
-    }
+  return await withCacheSpan(
+    "cache.lookup",
+    async () => {
+      const key = buildCacheKey(method, url);
+      const entry = await readCacheEntry(key);
+      if (!entry) {
+        return;
+      }
 
-    const responseHeaders = buildResponseHeaders(policy, entry);
-    return new Response(JSON.stringify(entry.body), {
-      status: entry.status,
-      headers: responseHeaders,
-    });
-  } catch {
-    // Corrupted or version-incompatible policy object — treat as cache miss.
-    // Best-effort cleanup of the broken entry.
-    unlink(cacheFilePath(key)).catch(() => {
-      // Ignored — fire-and-forget
-    });
-    return;
-  }
+      try {
+        const policy = CachePolicy.fromObject(entry.policy);
+        if (!isEntryFresh(policy, entry, requestHeaders, url)) {
+          return;
+        }
+
+        const responseHeaders = buildResponseHeaders(policy, entry);
+        return new Response(JSON.stringify(entry.body), {
+          status: entry.status,
+          headers: responseHeaders,
+        });
+      } catch {
+        // Corrupted or version-incompatible policy object — treat as cache miss.
+        // Best-effort cleanup of the broken entry.
+        unlink(cacheFilePath(key)).catch(() => {
+          // Ignored — fire-and-forget
+        });
+        return;
+      }
+    },
+    { "cache.url": url }
+  );
 }
 
 /**
@@ -424,7 +431,11 @@ export async function storeCachedResponse(
   }
 
   try {
-    await writeResponseToCache(method, url, requestHeaders, response);
+    await withCacheSpan(
+      "cache.store",
+      () => writeResponseToCache(method, url, requestHeaders, response),
+      { "cache.url": url }
+    );
   } catch {
     // Cache write failures are non-fatal — silently ignore
   }
@@ -498,17 +509,8 @@ export async function clearResponseCache(): Promise<void> {
 /** Concurrency limit for parallel cache file I/O operations */
 const CACHE_IO_CONCURRENCY = 8;
 
-/**
- * Run an async function over items with bounded concurrency.
- * Uses p-limit to prevent overwhelming the filesystem with simultaneous reads.
- */
-async function parallel<T>(
-  items: readonly T[],
-  fn: (item: T) => Promise<void>
-): Promise<void> {
-  const limit = pLimit(CACHE_IO_CONCURRENCY);
-  await Promise.all(items.map((item) => limit(() => fn(item))));
-}
+/** Shared concurrency limiter for all cache I/O — created once, reused across calls */
+const cacheIO = pLimit(CACHE_IO_CONCURRENCY);
 
 // ---------------------------------------------------------------------------
 // Cache cleanup
@@ -539,11 +541,11 @@ async function cleanupCache(): Promise<void> {
 
   const entries = await collectEntryMetadata(cacheDir, jsonFiles);
 
-  // Delete expired entries
-  await deleteExpiredEntries(cacheDir, entries);
-
-  // Enforce max entry count — evict oldest first
-  await evictExcessEntries(cacheDir, entries);
+  // Both operations are best-effort — run them in parallel without blocking
+  await Promise.all([
+    deleteExpiredEntries(cacheDir, entries),
+    evictExcessEntries(cacheDir, entries),
+  ]);
 }
 
 /** Metadata for a cache entry, used for cleanup decisions */
@@ -563,7 +565,7 @@ async function collectEntryMetadata(
   const entries: EntryMetadata[] = [];
   const now = Date.now();
 
-  await parallel(jsonFiles, async (file) => {
+  await cacheIO.map(jsonFiles, async (file) => {
     const filePath = join(cacheDir, file);
     try {
       const raw = await readFile(filePath, "utf-8");
@@ -591,11 +593,11 @@ async function deleteExpiredEntries(
   entries: EntryMetadata[]
 ): Promise<void> {
   const expired = entries.filter((e) => e.expired);
-  await parallel(expired, async (entry) => {
-    await unlink(join(cacheDir, entry.file)).catch(() => {
+  await cacheIO.map(expired, (entry) =>
+    unlink(join(cacheDir, entry.file)).catch(() => {
       // Best-effort: file may have been deleted by another process
-    });
-  });
+    })
+  );
 }
 
 /** Evict the oldest entries when over the max count */
@@ -610,9 +612,9 @@ async function evictExcessEntries(
 
   remaining.sort((a, b) => a.createdAt - b.createdAt);
   const toEvict = remaining.slice(0, remaining.length - MAX_CACHE_ENTRIES);
-  await parallel(toEvict, async (entry) => {
-    await unlink(join(cacheDir, entry.file)).catch(() => {
+  await cacheIO.map(toEvict, (entry) =>
+    unlink(join(cacheDir, entry.file)).catch(() => {
       // Best-effort eviction
-    });
-  });
+    })
+  );
 }

src/lib/telemetry.ts

@@ -945,3 +945,22 @@ export function withFsSpan<T>(
 ): Promise<T> {
   return withTracing(operation, "file", fn);
 }
+
+/**
+ * Wrap a cache operation with a span for tracing.
+ *
+ * Creates a child span under the current active span to track
+ * response cache hit/miss/store operations.
+ *
+ * @param operation - Name of the operation (e.g., "cache.lookup", "cache.store")
+ * @param fn - The function that performs the cache operation
+ * @param attributes - Optional span attributes (e.g., url, cache.hit)
+ * @returns The result of the function
+ */
+export function withCacheSpan<T>(
+  operation: string,
+  fn: () => T | Promise<T>,
+  attributes?: Record<string, string | number | boolean>
+): Promise<T> {
+  return withTracing(operation, "cache", fn, attributes);
+}