diff --git a/apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts b/apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts index 9ffe3fd474..3119a616e6 100644 --- a/apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts +++ b/apps/web/src/lib/cloud-agent/github-integration-helpers.test.ts @@ -232,7 +232,7 @@ describe('github-integration-helpers', () => { ]); }); - it('lists a repository shared by two installations exactly once, from the primary installation', async () => { + it('lists a repository shared by two installations exactly once, preferring the owning installation', async () => { mockGetIntegrationsByOrganization.mockResolvedValue([ buildIntegration({ id: 'integration-1', @@ -261,14 +261,101 @@ describe('github-integration-helpers', () => { expect.objectContaining({ fullName: 'acme-core/api', platformIntegrationId: 'integration-1', + platformAccountLogin: 'acme-core', }), expect.objectContaining({ fullName: 'acme-core/shared', platformIntegrationId: 'integration-1', + platformAccountLogin: 'acme-core', }), expect.objectContaining({ fullName: 'acme-labs/scanner', platformIntegrationId: 'integration-2', + platformAccountLogin: 'acme-labs', + }), + ]); + }); + + it('deduplicates a repository reachable through multiple installations, preferring the owning account', async () => { + mockGetIntegrationsByOrganization.mockResolvedValue([ + buildIntegration({ + id: 'integration-1', + platform_installation_id: 'installation-1', + platform_account_login: 'alice', + repositories: [ + { id: 1, name: 'api', full_name: 'acme/api', private: true }, + { id: 2, name: 'docs', full_name: 'acme/docs', private: false }, + ], + }), + buildIntegration({ + id: 'integration-2', + platform_installation_id: 'installation-2', + platform_account_login: 'acme', + repositories: [ + { id: 1, name: 'api', full_name: 'acme/api', private: true }, + { id: 3, name: 'scanner', full_name: 'acme/scanner', private: true }, + ], + }), + ]); + + const { fetchAllGitHubRepositoriesForOrganization } = + await import('./github-integration-helpers'); + const result = await fetchAllGitHubRepositoriesForOrganization('org-123'); + + expect(result.repositories).toEqual([ + expect.objectContaining({ + fullName: 'acme/api', + platformIntegrationId: 'integration-2', + platformAccountLogin: 'acme', + }), + expect.objectContaining({ + fullName: 'acme/docs', + platformIntegrationId: 'integration-1', + platformAccountLogin: 'alice', + }), + expect.objectContaining({ + fullName: 'acme/scanner', + platformIntegrationId: 'integration-2', + platformAccountLogin: 'acme', + }), + ]); + }); + + it('deduplicates repositories across multiple active installations of the same account, keeping the primary', async () => { + // getIntegrationsByOrganization returns installations oldest-first, so the + // first entry is the primary installation a session resolves by default. + mockGetIntegrationsByOrganization.mockResolvedValue([ + buildIntegration({ + id: 'integration-primary', + platform_installation_id: 'installation-1', + platform_account_login: 'acme', + repositories: [ + { id: 1, name: 'api', full_name: 'acme/api', private: true }, + { id: 2, name: 'docs', full_name: 'acme/docs', private: false }, + ], + }), + buildIntegration({ + id: 'integration-newer', + platform_installation_id: 'installation-2', + platform_account_login: 'acme', + repositories: [{ id: 1, name: 'api', full_name: 'acme/api', private: true }], + }), + ]); + + const { fetchAllGitHubRepositoriesForOrganization } = + await import('./github-integration-helpers'); + const result = await fetchAllGitHubRepositoriesForOrganization('org-123'); + + expect(result.repositories).toEqual([ + expect.objectContaining({ + fullName: 'acme/api', + platformIntegrationId: 'integration-primary', + platformAccountLogin: 'acme', + }), + expect.objectContaining({ + fullName: 'acme/docs', + platformIntegrationId: 'integration-primary', + platformAccountLogin: 'acme', }), ]); }); diff --git a/apps/web/src/lib/cloud-agent/github-integration-helpers.ts b/apps/web/src/lib/cloud-agent/github-integration-helpers.ts index 394a55f18f..b85d33a718 100644 --- a/apps/web/src/lib/cloud-agent/github-integration-helpers.ts +++ b/apps/web/src/lib/cloud-agent/github-integration-helpers.ts @@ -61,23 +61,6 @@ const missingIntegrationResponse = (message: string): GitHubRepositoriesResult = errorMessage: message, }); -/** - * A repository can be reachable from more than one installation. Keep the - * first occurrence: integrations arrive oldest-first, matching the primary - * installation a session resolves by default. - */ -const dedupeRepositories = ( - repositories: GitHubRepositoriesResult['repositories'] -): GitHubRepositoriesResult['repositories'] => { - const seen = new Set(); - return repositories.filter(repo => { - const key = repo.fullName.toLowerCase(); - if (seen.has(key)) return false; - seen.add(key); - return true; - }); -}; - export async function getGitHubTokenForOrganization( organizationId: string ): Promise { @@ -216,6 +199,42 @@ export async function fetchAllGitHubRepositoriesForOrganization( return fetchRepositoriesForIntegrations(integrations, forceRefresh); } +const repositoryOwner = (fullName: string) => { + const slash = fullName.indexOf('/'); + return slash === -1 ? fullName : fullName.slice(0, slash); +}; + +/** + * Keeps a single entry per repository when the same repo is reachable through + * more than one GitHub installation (for example the app is installed on two + * accounts that share a repo, or a reinstall left two active installation rows + * for the same account). Prefer the installation whose GitHub account owns the + * repository so sessions run against the canonical write identity. Otherwise + * keep the first entry: `getIntegrationsByOrganization` returns installations + * oldest-first, matching the primary installation a session resolves to by + * default. + */ +function dedupeRepositories( + repositories: GitHubRepositoriesResult['repositories'] +): GitHubRepositoriesResult['repositories'] { + const byFullName = new Map(); + for (const repo of repositories) { + const key = repo.fullName.toLowerCase(); + const existing = byFullName.get(key); + if (!existing) { + byFullName.set(key, repo); + continue; + } + const owner = repositoryOwner(existing.fullName).toLowerCase(); + const existingIsOwner = (existing.platformAccountLogin ?? '').toLowerCase() === owner; + const candidateIsOwner = (repo.platformAccountLogin ?? '').toLowerCase() === owner; + if (candidateIsOwner && !existingIsOwner) { + byFullName.set(key, repo); + } + } + return [...byFullName.values()]; +} + async function fetchRepositoriesForIntegrations( integrations: Awaited>, forceRefresh: boolean