fix(cache): harden invalidation helpers + align fingerprint expiry

Two bot-review findings on commit 6cbb21d:

1. **Invalidation helpers could mask successful mutations**
   (cursor-bot, src/lib/response-cache.ts:717 +
   src/lib/api/issues.ts:571).

   `invalidateCachedResponsesMatching` called
   `getIdentityFingerprint()` outside the `readdir` try/catch, so a
   DB failure inside the fingerprint lookup could throw up through
   callers. In `updateIssueStatus` (org-scoped path) the
   `invalidateIssueCaches` call was also unguarded, and in
   `mergeIssues` the per-ID invalidation sat inside the outer
   try/catch meant to only handle 204s — either could turn a
   successful mutation into a user-visible error.

   Fix: push the guard into the helpers themselves. Every invalidation
   helper now wraps its body in try/catch with an explicit
   "Never throws" contract in the JSDoc. Callers no longer need to
   remember; future call sites get the same safety automatically.
   The existing caller-side try/catch in `createProject` /
   `deleteProject` stays as defense-in-depth.

2. **`getIdentityFingerprint` skipped expired-token check vs
   `getAuthConfig`** (cursor-bot, src/lib/db/auth.ts:325).

   An expired access-only OAuth row (no refresh_token) is unusable —
   `getAuthConfig` falls past it to the env token, and the API
   client sends the env token on the next request. But the old
   fingerprint still hashed the stale access token, bucketing cache
   reads/writes under a dead identity while requests went under the
   env identity. Result: stale cached data from a previous user
   could leak to the env-token user.

   Fix: mirror `getAuthConfig`'s expiry semantics. Skip the access
   token when it has `expires_at` in the past and no refresh token;
   fall through to the env-token / anonymous path. Added two tests
   covering this and the access-token-rotation-with-refresh-token
   stability case.

Author: BYK

AGENTS.md

@@ -717,24 +717,30 @@ export async function getSharedIssue(
  * operation, not per-issue-affected, since the list sweep is a full
  * cache-directory scan.
  *
- * Best-effort: failures are swallowed inside the helper — the mutation
- * has already succeeded and a stale cache entry is strictly better than
- * surfacing a filesystem error to the user.
+ * **Never throws.** Called from post-mutation paths where the server
+ * has already committed the change; surfacing a cache-housekeeping
+ * error to the caller would cause the mutation to appear failed. All
+ * failures are swallowed — stale cache is strictly better than a false
+ * error on a successful mutation.
  */
 async function invalidateIssueDetailCaches(
   regionUrl: string,
   orgSlug: string,
   issueId: string
 ): Promise<void> {
-  const base = stripTrailingSlash(regionUrl);
-  const encodedOrg = encodeURIComponent(orgSlug);
-  const encodedId = encodeURIComponent(issueId);
-  await Promise.all([
-    invalidateCachedResponse(
-      `${base}/api/0/organizations/${encodedOrg}/issues/${encodedId}/`
-    ),
-    invalidateCachedResponse(`${base}/api/0/issues/${encodedId}/`),
-  ]);
+  try {
+    const base = stripTrailingSlash(regionUrl);
+    const encodedOrg = encodeURIComponent(orgSlug);
+    const encodedId = encodeURIComponent(issueId);
+    await Promise.all([
+      invalidateCachedResponse(
+        `${base}/api/0/organizations/${encodedOrg}/issues/${encodedId}/`
+      ),
+      invalidateCachedResponse(`${base}/api/0/issues/${encodedId}/`),
+    ]);
+  } catch {
+    // Non-fatal — see JSDoc.
+  }
 }
 
 /**
@@ -746,16 +752,22 @@ async function invalidateIssueDetailCaches(
  * {@link invalidateIssueDetailCaches} and call
  * {@link invalidateOrgIssueList} once at the end, to avoid N+1 full
  * cache scans.
+ *
+ * **Never throws** — same reasoning as {@link invalidateIssueDetailCaches}.
  */
 async function invalidateIssueCaches(
   regionUrl: string,
   orgSlug: string,
   issueId: string
 ): Promise<void> {
-  await Promise.all([
-    invalidateIssueDetailCaches(regionUrl, orgSlug, issueId),
-    invalidateOrgIssueList(regionUrl, orgSlug),
-  ]);
+  try {
+    await Promise.all([
+      invalidateIssueDetailCaches(regionUrl, orgSlug, issueId),
+      invalidateOrgIssueList(regionUrl, orgSlug),
+    ]);
+  } catch {
+    // Non-fatal — see JSDoc.
+  }
 }
 
 /**
@@ -764,13 +776,19 @@ async function invalidateIssueCaches(
  * The list endpoint is keyed by query string (sort, cursor, query,
  * statsPeriod, ...) so we have to do a prefix sweep rather than a
  * single URL invalidation.
+ *
+ * **Never throws** — same reasoning as {@link invalidateIssueDetailCaches}.
  */
 async function invalidateOrgIssueList(
   regionUrl: string,
   orgSlug: string
 ): Promise<void> {
-  const base = stripTrailingSlash(regionUrl);
-  await invalidateCachedResponsesMatching(
-    `${base}/api/0/organizations/${encodeURIComponent(orgSlug)}/issues/`
-  );
+  try {
+    const base = stripTrailingSlash(regionUrl);
+    await invalidateCachedResponsesMatching(
+      `${base}/api/0/organizations/${encodeURIComponent(orgSlug)}/issues/`
+    );
+  } catch {
+    // Non-fatal — see JSDoc.
+  }
 }

src/lib/api/issues.ts

@@ -266,13 +266,23 @@ export async function deleteProject(
  *
  * Wraps the pattern-match invalidator to keep the call sites short and
  * document *why* we clear these specific prefixes.
+ *
+ * **Never throws.** Called from post-mutation paths where the server
+ * has already committed the change; surfacing a cache-housekeeping
+ * error to the caller would cause the mutation to appear failed. This
+ * lets `createProject` / `deleteProject` drop their own try/catch
+ * wrappers — the helper is self-contained.
  */
 async function invalidateOrgProjectCache(orgSlug: string): Promise<void> {
-  const regionUrl = await resolveOrgRegion(orgSlug);
-  const base = stripTrailingSlash(regionUrl);
-  await invalidateCachedResponsesMatching(
-    `${base}/api/0/organizations/${encodeURIComponent(orgSlug)}/projects/`
-  );
+  try {
+    const regionUrl = await resolveOrgRegion(orgSlug);
+    const base = stripTrailingSlash(regionUrl);
+    await invalidateCachedResponsesMatching(
+      `${base}/api/0/organizations/${encodeURIComponent(orgSlug)}/projects/`
+    );
+  } catch {
+    // Non-fatal — see JSDoc.
+  }
 }
 
 /** Result of searching for projects by slug across all organizations. */

src/lib/api/projects.ts

@@ -309,22 +309,36 @@ export function getIdentityFingerprint(): string {
   // Otherwise prefer the OAuth refresh token when present (stable
   // across hourly access-token rotation). Access-only rows without a
   // refresh_token fall through to hashing the access token itself.
+  //
+  // Must mirror `getAuthConfig`'s expiry semantics: an access-only row
+  // (no refresh_token) with a past `expires_at` is unusable — the API
+  // client will fall back to the env token for the actual request. If
+  // we still hash it here, the cache namespace diverges from the
+  // identity that sends the request and we could serve a previous
+  // user's cached data to the env-token user.
   const dbRow = withDbSpan("getIdentityFingerprint", () => {
     const db = getDatabase();
     return db
-      .query("SELECT token, refresh_token FROM auth WHERE id = 1")
+      .query("SELECT token, refresh_token, expires_at FROM auth WHERE id = 1")
       .get() as
-      | { token: string | null; refresh_token: string | null }
+      | {
+          token: string | null;
+          refresh_token: string | null;
+          expires_at: number | null;
+        }
       | undefined;
   });
   if (dbRow?.refresh_token) {
+    // Keyed by refresh_token — stable across access-token rotation,
+    // including the about-to-expire case where the API client will
+    // refresh the token before the next request.
     return hashIdentity("oauth", dbRow.refresh_token);
   }
-  if (dbRow?.token) {
+  if (dbRow?.token && !(dbRow.expires_at && Date.now() > dbRow.expires_at)) {
     return hashIdentity("oauth-access", dbRow.token);
   }
 
-  // Finally, fall back to the env token when no OAuth is stored.
+  // Finally, fall back to the env token when no usable OAuth is stored.
   const envToken = getRawEnvToken();
   if (envToken) {
     return hashIdentity("env", envToken);

src/lib/db/auth.ts

@@ -697,45 +697,57 @@ export async function invalidateCachedResponse(url: string): Promise<void> {
 export async function invalidateCachedResponsesMatching(
   prefix: string
 ): Promise<void> {
-  const cacheDir = getCacheDir();
-  let files: string[];
+  // Whole-body try/catch: this helper is called from post-mutation
+  // paths (`project create`, `issue resolve`, `mergeIssues`, …) where
+  // a successful mutation has already returned from the API. Any throw
+  // from here — \`getIdentityFingerprint\` hitting a DB error, a
+  // filesystem race, an unexpected IO failure — must not propagate,
+  // because the caller cannot undo the mutation and users would see
+  // the operation as "failed" despite the server having accepted it.
+  // A stale cache entry is strictly preferable to a false error.
   try {
-    files = await readdir(cacheDir);
-  } catch (error) {
-    if (isNotFound(error)) {
+    const cacheDir = getCacheDir();
+    let files: string[];
+    try {
+      files = await readdir(cacheDir);
+    } catch (error) {
+      if (isNotFound(error)) {
+        return;
+      }
+      // Unknown read error — best-effort: skip
       return;
     }
-    // Unknown read error — best-effort: skip
-    return;
-  }
 
-  const jsonFiles = files.filter((f) => f.endsWith(".json"));
-  if (jsonFiles.length === 0) {
-    return;
-  }
+    const jsonFiles = files.filter((f) => f.endsWith(".json"));
+    if (jsonFiles.length === 0) {
+      return;
+    }
 
-  const currentIdentity = getIdentityFingerprint();
+    const currentIdentity = getIdentityFingerprint();
 
-  await cacheIO.map(jsonFiles, async (file) => {
-    const filePath = join(cacheDir, file);
-    try {
-      const raw = await readFile(filePath, "utf-8");
-      const entry = JSON.parse(raw) as CacheEntry;
-      // Identity gate: only delete entries the current identity wrote.
-      // Missing `identity` field = legacy entry from before this check
-      // existed; skip rather than risk crossing identities.
-      if (entry.identity !== currentIdentity) {
-        return;
-      }
-      if (entry.url?.startsWith(prefix)) {
-        await unlink(filePath).catch(() => {
-          // Best-effort: another process may have deleted it
-        });
+    await cacheIO.map(jsonFiles, async (file) => {
+      const filePath = join(cacheDir, file);
+      try {
+        const raw = await readFile(filePath, "utf-8");
+        const entry = JSON.parse(raw) as CacheEntry;
+        // Identity gate: only delete entries the current identity wrote.
+        // Missing `identity` field = legacy entry from before this check
+        // existed; skip rather than risk crossing identities.
+        if (entry.identity !== currentIdentity) {
+          return;
+        }
+        if (entry.url?.startsWith(prefix)) {
+          await unlink(filePath).catch(() => {
+            // Best-effort: another process may have deleted it
+          });
+        }
+      } catch {
+        // Unparseable or missing — leave to cleanup
       }
-    } catch {
-      // Unparseable or missing — leave to cleanup
-    }
-  });
+    });
+  } catch {
+    // Best-effort — see whole-body comment above.
+  }
 }
 
 /**

src/lib/response-cache.ts

@@ -279,4 +279,44 @@ describe("getIdentityFingerprint", () => {
     const oauthFp = getIdentityFingerprint();
     expect(envFp).not.toBe(oauthFp);
   });
+
+  test("expired access-only OAuth token falls through to env token", () => {
+    // Mirrors getAuthConfig: an expired token with no refresh token is
+    // unusable — the API client will fall back to the env token for
+    // the next request. If the fingerprint still used the stale access
+    // token, cache reads/writes would land under the dead OAuth
+    // namespace while requests go under the env identity, serving
+    // another user's cached data.
+    //
+    // setAuthToken(token, expiresIn: -1) writes an already-expired row
+    // with no refresh_token (third arg omitted).
+    setAuthToken("expired_access", -1);
+    process.env.SENTRY_AUTH_TOKEN = "env_token";
+
+    const fp = getIdentityFingerprint();
+
+    // The fingerprint should match the env-token identity, not the
+    // stale DB token.
+    delete process.env.SENTRY_AUTH_TOKEN;
+    // Clear the expired DB row to isolate: anon is the only other
+    // possibility this case could collapse into.
+    setAuthToken("", -1); // idempotent clear not available; rely on positive check below
+    process.env.SENTRY_AUTH_TOKEN = "env_token";
+    const envOnlyFp = getIdentityFingerprint();
+    expect(fp).toBe(envOnlyFp);
+  });
+
+  test("expired access-only OAuth token with refresh_token uses the refresh token", () => {
+    // An expired access token that has a refresh token is still usable
+    // — the API client will perform an OAuth refresh. Fingerprint
+    // should key off the stable refresh token, not the (about-to-be-
+    // rotated) access token.
+    setAuthToken("expired_access", -1, "live_refresh");
+    const fp = getIdentityFingerprint();
+
+    // Rotate the access token; refresh stays the same. Fingerprint
+    // must not change.
+    setAuthToken("fresh_access", 3600, "live_refresh");
+    expect(getIdentityFingerprint()).toBe(fp);
+  });
 });