fix: exact maintainer status check with negative-cache lockdown (#871)

src/inngest/functions/maintainer-discover.test.ts

@@ -12,7 +12,11 @@ vi.mock('@/lib/maintainer/discover', () => ({
   decideRepoGrant: vi.fn(),
   reconcileGrants: vi.fn(),
 }));
-vi.mock('@/lib/cache', () => ({ cacheGet: vi.fn(), cacheSet: vi.fn() }));
+vi.mock('@/lib/cache', () => ({
+  cacheGet: vi.fn(),
+  cacheSet: vi.fn(),
+  cacheDel: vi.fn(),
+}));
 
 const mockSend = vi.fn();
 vi.mock('../client', () => ({

src/inngest/functions/maintainer-discover.ts

@@ -8,7 +8,7 @@ import {
   reconcileGrants,
   type ProposedGrant,
 } from '@/lib/maintainer/discover';
-import { cacheGet, cacheSet } from '@/lib/cache';
+import { cacheGet, cacheSet, cacheDel } from '@/lib/cache';
 
 /**
  * Revalidates a user's installs + repos and reconciles the
@@ -267,7 +267,7 @@ async function discoverForUser(
   }
 
   await cacheSet(`maint:discovered:${userId}`, { ranAt: Date.now() }, DEDUP_TTL_S);
-  await cacheSet(`maint:status:${userId}`, false, 1);
+  await cacheDel(`maint:status:${userId}`);
 
   return {
     user: userId,

src/inngest/functions/process-installation-event.ts

@@ -1,6 +1,7 @@
 import { inngest } from '../client';
 import { getServiceSupabase } from '@/lib/supabase/service';
 import { getInstallOctokit } from '@/lib/github/app';
+import { cacheDel } from '@/lib/cache';
 
 /**
  * GitHub App installation lifecycle:
@@ -82,6 +83,10 @@ export const processInstallationEvent = inngest.createFunction(
             },
             { onConflict: 'installation_id,user_id' },
           );
+          // New grant for this user — drop the cached 1h denial (if any) so
+          // the grant takes effect immediately instead of after the next
+          // discovery run.
+          await cacheDel(`maint:status:${profile.id}`);
         }
 
         // GitHub only includes `repositories` in the payload when the user

src/lib/maintainer/detect.test.ts

@@ -12,18 +12,42 @@ vi.mock('@/lib/cache', () => ({
   cacheSet: vi.fn(),
 }));
 
+const applyFilters = (rows: unknown, filters: Array<[string, unknown]>): unknown => {
+  if (!Array.isArray(rows)) return rows;
+  let filtered = rows as Array<Record<string, unknown>>;
+  for (const [col, val] of filters) {
+    if (col.startsWith('github_installations.')) {
+      const field = col.split('.')[1] as string;
+      filtered = filtered.filter((r) => {
+        const install = r.github_installations;
+        const joined = (Array.isArray(install) ? install[0] : install) as
+          | Record<string, unknown>
+          | null
+          | undefined;
+        if (!joined) return val === null ? false : true;
+        return joined[field] === val;
+      });
+    }
+  }
+  return filtered;
+};
+
 const mockSupabase = (mockTables: Record<string, unknown>) => {
   const mockClient = {
     from: vi.fn().mockImplementation((table: string) => {
+      const eqFilters: Array<[string, unknown]> = [];
       const chain = {
         select: vi.fn().mockReturnThis(),
-        eq: vi.fn().mockReturnThis(),
+        eq: vi.fn().mockImplementation((col: string, val: unknown) => {
+          eqFilters.push([col, val]);
+          return chain;
+        }),
         limit: vi.fn().mockReturnThis(),
         maybeSingle: vi.fn().mockImplementation(() => {
-          return Promise.resolve({ data: (mockTables[table] as unknown) ?? null });
+          return Promise.resolve({ data: applyFilters(mockTables[table], eqFilters) ?? null });
         }),
         then: function (resolve: (value: unknown) => void) {
-          resolve({ data: (mockTables[table] as unknown) ?? null });
+          resolve({ data: applyFilters(mockTables[table], eqFilters) ?? null });
         },
       };
       return chain;
@@ -92,13 +116,13 @@ describe('isUserMaintainer', () => {
     expect(cacheSet).toHaveBeenCalledWith('maint:status:user1', false, 3600);
   });
 
-  it('returns false when service client is not configured', async () => {
+  it('returns false when service client is not configured (without caching the denial)', async () => {
     vi.mocked(cacheGet).mockResolvedValue(null);
     vi.mocked(getServiceSupabase).mockReturnValue(null);
 
     const result = await isUserMaintainer('user1');
     expect(result).toBe(false);
-    expect(cacheSet).toHaveBeenCalledWith('maint:status:user1', false, 3600);
+    expect(cacheSet).not.toHaveBeenCalled();
   });
 
   it('returns cached result when cache is warm (should not hit DB)', async () => {

src/lib/maintainer/detect.ts

@@ -8,7 +8,8 @@ import { unwrapJoin } from '@/lib/supabase/inner-join';
  *
  * Cached per user for 1h. Cache is busted by:
  *   - maintainer-discover function (after writing junction changes)
- *   - process-installation-event (on installation.deleted)
+ *   - process-installation-event (on installation.created / deleted)
+ *   - any grant path that writes junction rows
  */
 
 const TTL_S = 60 * 60;
@@ -19,22 +20,22 @@ export async function isUserMaintainer(userId: string): Promise<boolean> {
   if (cached !== null) return cached;
 
   const sb = getServiceSupabase();
-  if (!sb) {
-    await cacheSet(cacheKey, false, TTL_S);
-    return false;
-  }
-
+  // A missing service client is a transient infra condition — do NOT cache
+  // the denial, otherwise a blip becomes a 1-hour maintainer lockout.
+  if (!sb) return false;
+
+  // DB-side existence check: only junction rows joined to an active
+  // (non-uninstalled) install count. No arbitrary row cap, so a user with
+  // more than 20 junction rows is still classified correctly. limit(1) just
+  // bounds the payload — we only need to know whether any active row exists.
   const { data } = await sb
     .from('github_installation_users')
     .select('installation_id, github_installations!inner(uninstalled_at)')
     .eq('user_id', userId)
-    .limit(20);
+    .eq('github_installations.uninstalled_at', null)
+    .limit(1);
 
-  const has = (data ?? []).some((row) => {
-    const r = row as unknown as { github_installations: unknown };
-    const i = unwrapJoin<{ uninstalled_at: string | null }>(r.github_installations);
-    return i && i.uninstalled_at === null;
-  });
+  const has = (data ?? []).length > 0;
 
   await cacheSet(cacheKey, has, TTL_S);
   return has;