From 97f71ddc58d2db881eac7a4771fed205ca5e57c8 Mon Sep 17 00:00:00 2001 From: pavsoss Date: Mon, 13 Jul 2026 00:15:53 +0530 Subject: [PATCH] feat: add GitHub App uninstall cleanup --- src/inngest/functions/pr-backfill.test.ts | 52 ++++++++++ src/inngest/functions/pr-backfill.ts | 34 ++++++- .../process-installation-event.test.ts | 94 +++++++++++++++++-- .../functions/process-installation-event.ts | 29 +++++- 4 files changed, 197 insertions(+), 12 deletions(-) diff --git a/src/inngest/functions/pr-backfill.test.ts b/src/inngest/functions/pr-backfill.test.ts index 26eff7cd..dddf243d 100644 --- a/src/inngest/functions/pr-backfill.test.ts +++ b/src/inngest/functions/pr-backfill.test.ts @@ -35,11 +35,51 @@ describe('prBackfill', () => { vi.clearAllMocks(); }); + it('single-repo backfill skips an uninstalled installation', async () => { + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: '2026-01-01' } }), + }); + wire({ github_installations: installs }); + + const result = await run({ event: evRepo(), step }); + expect(result).toEqual({ skipped: true, reason: 'uninstalled' }); + expect(getInstallOctokit).not.toHaveBeenCalled(); + }); + + it('installation-wide backfill skips an uninstalled installation', async () => { + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: '2026-01-01' } }), + }); + wire({ github_installations: installs }); + + const result = await run({ + event: { name: 'pr-backfill/installation', data: { installationId: 1 } }, + step, + }); + expect(result).toEqual({ skipped: true, reason: 'uninstalled or empty' }); + expect(getInstallOctokit).not.toHaveBeenCalled(); + }); + + it('installation lookup database error is thrown rather than silently skipped', async () => { + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ error: { message: 'db error' } }), + }); + wire({ github_installations: installs }); + + await expect(run({ event: evRepo(), step })).rejects.toThrow( + 'Failed to check installation: db error', + ); + }); + it('backfills recent PRs within backfill window', async () => { const pull_requests = sb({ upsert: vi.fn().mockResolvedValue({}) }); + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: null } }), + }); wire({ pull_requests, + github_installations: installs, profiles: sb({ select: vi.fn().mockReturnThis(), eq: vi.fn().mockReturnThis(), @@ -87,7 +127,11 @@ describe('prBackfill', () => { }); it('stops pagination when encountering older PRs', async () => { + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: null } }), + }); wire({ + github_installations: installs, profiles: sb({ select: vi.fn().mockReturnThis(), eq: vi.fn().mockReturnThis(), @@ -124,8 +168,12 @@ describe('prBackfill', () => { it('resumes from saved page cursor', async () => { const pull_requests = sb({ upsert: vi.fn().mockResolvedValue({}) }); + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: null } }), + }); wire({ pull_requests, + github_installations: installs, profiles: sb({ select: vi.fn().mockReturnThis(), eq: vi.fn().mockReturnThis(), @@ -165,6 +213,10 @@ describe('prBackfill', () => { }); it('handles github api errors gracefully', async () => { + const installs = sb({ + maybeSingle: vi.fn().mockResolvedValue({ data: { uninstalled_at: null } }), + }); + wire({ github_installations: installs }); vi.mocked(getInstallOctokit).mockRejectedValue(new Error('API quota exceeded')); const result = await run({ event: evRepo(), step }); diff --git a/src/inngest/functions/pr-backfill.ts b/src/inngest/functions/pr-backfill.ts index 7d1b4c0e..2a871f0d 100644 --- a/src/inngest/functions/pr-backfill.ts +++ b/src/inngest/functions/pr-backfill.ts @@ -34,6 +34,22 @@ export const prBackfill = inngest.createFunction( async ({ event, step }) => { if (event.name === 'pr-backfill/repo') { const data = (event as RepoEvent).data; + + const isActive = await step.run(`check-active-${data.installationId}`, async () => { + const sb = getServiceSupabase(); + if (!sb) throw new Error('service role missing'); + const { data: install, error } = await sb + .from('github_installations') + .select('uninstalled_at') + .eq('id', data.installationId) + .maybeSingle(); + + if (error) throw new Error(`Failed to check installation: ${error.message}`); + return !!install && !install.uninstalled_at; + }); + + if (!isActive) return { skipped: true, reason: 'uninstalled' }; + const budget = await step.run( `check-budget-repo-${data.repoFullName.replace('/', '-')}`, () => checkRateBudget(data.installationId), @@ -58,13 +74,29 @@ export const prBackfill = inngest.createFunction( const repos = await step.run('list-repos', async () => { const sb = getServiceSupabase(); if (!sb) throw new Error('service role missing'); - const { data: rows } = await sb + + const { data: install, error: installError } = await sb + .from('github_installations') + .select('uninstalled_at') + .eq('id', installationId) + .maybeSingle(); + + if (installError) throw new Error(`Failed to check installation: ${installError.message}`); + if (!install || install.uninstalled_at) return []; + + const { data: rows, error: reposError } = await sb .from('installation_repositories') .select('repo_full_name') .eq('installation_id', installationId); + + if (reposError) throw new Error(`Failed to list repos: ${reposError.message}`); return (rows ?? []).map((r) => r.repo_full_name); }); + if (repos.length === 0) { + return { skipped: true, reason: 'uninstalled or empty' }; + } + const reports: Array<{ repo: string; prs: number; errors: string[] }> = []; for (const repo of repos) { const budget = await step.run(`check-budget-repo-${repo.replace('/', '-')}`, () => diff --git a/src/inngest/functions/process-installation-event.test.ts b/src/inngest/functions/process-installation-event.test.ts index 7bbead43..76c82827 100644 --- a/src/inngest/functions/process-installation-event.test.ts +++ b/src/inngest/functions/process-installation-event.test.ts @@ -64,17 +64,93 @@ describe('processInstallationEvent', () => { ); }); - it('uninstall sets uninstalled_at', async () => { - const installs = sb({ update: vi.fn().mockReturnThis() }); - wire({ github_installations: installs }); + describe('uninstall cleanup', () => { + it('marks uninstalled_at and deletes all 6 derived tables', async () => { + const installs = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const repos = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const users = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const userRepos = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const settings = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const orgs = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const cursors = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + + wire({ + github_installations: installs, + installation_repositories: repos, + github_installation_users: users, + installation_user_repos: userRepos, + installation_settings: settings, + org_communities: orgs, + repo_sync_cursors: cursors, + }); + + await installRun({ event: ev('deleted'), step }); + + // 1. Uninstall marks uninstalled_at + expect(installs.update).toHaveBeenCalledWith( + expect.objectContaining({ uninstalled_at: expect.any(String) }), + ); + + // 2. All 6 derived tables are deleted sequentially + expect(repos.delete).toHaveBeenCalled(); + expect(users.delete).toHaveBeenCalled(); + expect(userRepos.delete).toHaveBeenCalled(); + expect(settings.delete).toHaveBeenCalled(); + expect(orgs.delete).toHaveBeenCalled(); + expect(cursors.delete).toHaveBeenCalled(); + }); - await installRun({ event: ev('deleted'), step }); + it('throws if marking uninstalled_at fails', async () => { + const installs = sb({ eq: vi.fn().mockResolvedValue({ error: { message: 'db error' } }) }); + wire({ github_installations: installs }); - expect(installs.update).toHaveBeenCalledWith( - expect.objectContaining({ - uninstalled_at: expect.any(String), - }), - ); + await expect(installRun({ event: ev('deleted'), step })).rejects.toThrow( + 'Failed to mark install 100 as uninstalled: db error', + ); + }); + + it('throws if cleanup deletion fails, leaving remaining tables untouched', async () => { + const installs = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + // installation_repositories deletes successfully + const repos = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + // github_installation_users fails + const users = sb({ eq: vi.fn().mockResolvedValue({ error: { message: 'users error' } }) }); + // remaining tables should not be called + const userRepos = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + + wire({ + github_installations: installs, + installation_repositories: repos, + github_installation_users: users, + installation_user_repos: userRepos, + }); + + await expect(installRun({ event: ev('deleted'), step })).rejects.toThrow( + 'Failed to delete from github_installation_users for install 100: users error', + ); + + expect(repos.delete).toHaveBeenCalled(); + expect(users.delete).toHaveBeenCalled(); + expect(userRepos.delete).not.toHaveBeenCalled(); // Failed before reaching this + }); + + it('tolerates already-deleted rows (idempotency)', async () => { + // Supabase delete() returns { error: null } and 0 rows if nothing matches + const installs = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + const tables = sb({ eq: vi.fn().mockResolvedValue({ error: null }) }); + wire({ + github_installations: installs, + installation_repositories: tables, + github_installation_users: tables, + installation_user_repos: tables, + installation_settings: tables, + org_communities: tables, + repo_sync_cursors: tables, + }); + + const result = await installRun({ event: ev('deleted'), step }); + expect(result).toEqual({ ok: true, uninstalled: true, cleanup: true }); + }); }); it('suspend sets suspended_at', async () => { diff --git a/src/inngest/functions/process-installation-event.ts b/src/inngest/functions/process-installation-event.ts index 089c99be..ecc9960f 100644 --- a/src/inngest/functions/process-installation-event.ts +++ b/src/inngest/functions/process-installation-event.ts @@ -145,11 +145,36 @@ export const processInstallationEvent = inngest.createFunction( } if (payload.action === 'deleted') { - await sb + const { error: markError } = await sb .from('github_installations') .update({ uninstalled_at: new Date().toISOString() }) .eq('id', install.id); - return { ok: true, uninstalled: true }; + + if (markError) { + throw new Error( + `Failed to mark install ${install.id} as uninstalled: ${markError.message}`, + ); + } + + const tables = [ + 'installation_repositories', + 'github_installation_users', + 'installation_user_repos', + 'installation_settings', + 'org_communities', + 'repo_sync_cursors', + ] as const; + + for (const table of tables) { + const { error } = await sb.from(table).delete().eq('installation_id', install.id); + if (error) { + throw new Error( + `Failed to delete from ${table} for install ${install.id}: ${error.message}`, + ); + } + } + + return { ok: true, uninstalled: true, cleanup: true }; } if (payload.action === 'suspend') {