diff --git a/server/src/__tests__/github-transport.test.ts b/server/src/__tests__/github-transport.test.ts new file mode 100644 index 00000000000..9bb65ed0062 --- /dev/null +++ b/server/src/__tests__/github-transport.test.ts @@ -0,0 +1,80 @@ +import { generateKeyPairSync } from "node:crypto"; +import { describe, expect, it, vi } from "vitest"; +import { createGitHubAppTransport } from "../services/repository-providers/github-transport.js"; +import { makeGitHubRepository } from "./helpers/fake-github-transport.js"; + +const privateKey = generateKeyPairSync("rsa", { modulusLength: 2048 }) + .privateKey.export({ type: "pkcs8", format: "pem" }) + .toString(); + +function jsonResponse(body: unknown, status = 200) { + return new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); +} + +describe("GitHub App transport", () => { + it("searches the complete installation catalog before paginating matches", async () => { + const repositories = Array.from({ length: 205 }, (_, index) => makeGitHubRepository({ + id: index + 1, + owner: index === 100 || index === 101 || index === 200 ? "matching-owner" : "other-owner", + name: `repo-${index + 1}`, + })); + const fetchImpl = vi.fn(async (input: string | URL | Request) => { + const url = new URL(typeof input === "string" ? input : input instanceof URL ? input : input.url); + if (url.pathname.endsWith("/access_tokens")) { + return jsonResponse({ token: "ghs_test", expires_at: "2099-01-01T00:00:00.000Z" }); + } + const page = Number(url.searchParams.get("page")); + const perPage = Number(url.searchParams.get("per_page")); + const start = (page - 1) * perPage; + return jsonResponse({ total_count: repositories.length, repositories: repositories.slice(start, start + perPage) }); + }); + const transport = createGitHubAppTransport({ appId: "1", privateKey, fetchImpl }); + + await expect(transport.listRepositories({ + installationId: "installation-1", + page: 1, + perPage: 2, + query: "matching-owner", + })).resolves.toMatchObject({ + totalCount: 3, + items: [{ id: 101 }, { id: 102 }], + }); + await expect(transport.listRepositories({ + installationId: "installation-1", + page: 2, + perPage: 2, + query: "matching-owner", + })).resolves.toMatchObject({ + totalCount: 3, + items: [{ id: 201 }], + }); + expect(fetchImpl.mock.calls.some(([input]) => String(input).includes("per_page=100&page=3"))).toBe(true); + }); + + it("does not expose provider request paths in operator-facing failures", async () => { + const fetchImpl = vi.fn(async (input: string | URL | Request) => { + const url = new URL(typeof input === "string" ? input : input instanceof URL ? input : input.url); + if (url.pathname.endsWith("/access_tokens")) { + return jsonResponse({ token: "ghs_test", expires_at: "2099-01-01T00:00:00.000Z" }); + } + return jsonResponse({ message: "unavailable" }, 503); + }); + const transport = createGitHubAppTransport({ appId: "1", privateKey, fetchImpl }); + + await expect(transport.listRepositories({ + installationId: "sensitive-installation", + page: 1, + perPage: 20, + query: null, + })).rejects.toThrow("GitHub repository provider request failed with status 503"); + await expect(transport.listRepositories({ + installationId: "sensitive-installation", + page: 1, + perPage: 20, + query: null, + })).rejects.not.toThrow("sensitive-installation"); + }); +}); diff --git a/server/src/__tests__/repositories.test.ts b/server/src/__tests__/repositories.test.ts index 2648e9054a4..65fab73f117 100644 --- a/server/src/__tests__/repositories.test.ts +++ b/server/src/__tests__/repositories.test.ts @@ -533,11 +533,18 @@ describeEmbeddedPostgres("repository services and routes", () => { provider: "test_provider", sync: async () => ({ cursor: "cursor-1", - repositories: [{ - providerRepositoryId: "provider-repo-1", - cloneUrl: "https://git.example.com/acme/provider-repo.git", - visibility: "private", - }], + repositories: [ + { + providerRepositoryId: "provider-repo-1", + cloneUrl: "https://git.example.com/acme/provider-repo-renamed.git", + visibility: "private", + }, + { + providerRepositoryId: "provider-repo-not-imported", + cloneUrl: "https://git.example.com/acme/not-imported.git", + visibility: "private", + }, + ], }), disconnect: async () => {}, }; @@ -560,9 +567,24 @@ describeEmbeddedPostgres("repository services and routes", () => { expect(created.created).toBe(true); expect(duplicate.created).toBe(false); + const providerRepository = await repositoryService(db).upsertProviderRepository( + seeded.companyA.id, + created.connection.id, + provider.provider, + { + providerRepositoryId: "provider-repo-1", + cloneUrl: "https://git.example.com/acme/provider-repo.git", + visibility: "private", + }, + ); const synced = await connections.sync(seeded.companyA.id, created.connection.id); expect(synced?.repositories).toHaveLength(1); - const providerRepository = synced!.repositories[0]!; + expect(synced?.repositories[0]).toMatchObject({ + id: providerRepository.id, + name: "provider-repo-renamed", + }); + expect((await repositoryService(db).list(seeded.companyA.id)).map((repository) => repository.name)) + .not.toContain("not-imported"); await repositoryAccessService(db).attachProjectRepository({ companyId: seeded.companyA.id, projectId: seeded.projectOne.id, diff --git a/server/src/services/repositories.ts b/server/src/services/repositories.ts index 4bfb1676aba..bdd35931aed 100644 --- a/server/src/services/repositories.ts +++ b/server/src/services/repositories.ts @@ -378,5 +378,25 @@ export function repositoryService(db: Db) { .returning(); return rows.map(toRepository); }, + + markProviderRepositoriesUnavailable: async ( + companyId: string, + connectionId: string, + providerRepositoryIds: string[], + reason: string, + ) => { + if (providerRepositoryIds.length === 0) return []; + const rows = await db + .update(repositories) + .set({ state: "unavailable", unavailableReason: reason, updatedAt: new Date() }) + .where(and( + eq(repositories.companyId, companyId), + eq(repositories.connectionId, connectionId), + inArray(repositories.providerRepositoryId, providerRepositoryIds), + ne(repositories.state, "archived"), + )) + .returning(); + return rows.map(toRepository); + }, }; } diff --git a/server/src/services/repository-connections.ts b/server/src/services/repository-connections.ts index 556e53a7e18..10a90943152 100644 --- a/server/src/services/repository-connections.ts +++ b/server/src/services/repository-connections.ts @@ -1,6 +1,6 @@ import { and, asc, eq } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; -import { repositoryConnections } from "@paperclipai/db"; +import { repositoryConnections, repositories } from "@paperclipai/db"; import type { CreateRepositoryConnection, RepositoryConnection } from "@paperclipai/shared"; import { REPOSITORY_PROVIDER_ANY_HOST, @@ -337,12 +337,36 @@ export function repositoryConnectionService( }); return await db.transaction(async (tx) => { const transactionalRepositories = repositoryService(tx as unknown as Db); + const importedRows = await tx + .select({ providerRepositoryId: repositories.providerRepositoryId }) + .from(repositories) + .where(and( + eq(repositories.companyId, companyId), + eq(repositories.connectionId, id), + )); + const importedIds = new Set( + importedRows + .map((repository) => repository.providerRepositoryId) + .filter((providerRepositoryId): providerRepositoryId is string => Boolean(providerRepositoryId)), + ); + const snapshotsById = new Map( + result.repositories.map((snapshot) => [snapshot.providerRepositoryId, snapshot]), + ); const syncedRepositories = []; - for (const snapshot of result.repositories) { + for (const providerRepositoryId of importedIds) { + const snapshot = snapshotsById.get(providerRepositoryId); + if (!snapshot) continue; syncedRepositories.push( await transactionalRepositories.upsertProviderRepository(companyId, id, row.provider, snapshot), ); } + const unavailableIds = [...importedIds].filter((providerRepositoryId) => !snapshotsById.has(providerRepositoryId)); + syncedRepositories.push(...await transactionalRepositories.markProviderRepositoriesUnavailable( + companyId, + id, + unavailableIds, + "Repository is no longer available through its provider connection", + )); const completed = await tx .update(repositoryConnections) .set({ diff --git a/server/src/services/repository-providers/github-provider.ts b/server/src/services/repository-providers/github-provider.ts index 93d95c08885..ae9cd70ebb6 100644 --- a/server/src/services/repository-providers/github-provider.ts +++ b/server/src/services/repository-providers/github-provider.ts @@ -228,7 +228,10 @@ export function createGitHubProvider(deps: GitHubProviderDeps): RepositoryProvid query: input.query?.trim() || null, }); const items = result.items.map(toDiscovered); - const nextCursor = items.length === perPage ? String(page + 1) : null; + const hasNextPage = result.totalCount !== null + ? page * perPage < result.totalCount + : items.length === perPage; + const nextCursor = hasNextPage ? String(page + 1) : null; return { items, nextCursor, total: result.totalCount }; }, diff --git a/server/src/services/repository-providers/github-transport.ts b/server/src/services/repository-providers/github-transport.ts index 691a3b0c63d..d1ae01f0922 100644 --- a/server/src/services/repository-providers/github-transport.ts +++ b/server/src/services/repository-providers/github-transport.ts @@ -68,7 +68,7 @@ export function createGitHubAppTransport(config: GitHubAppTransportConfig): GitH }, }); if (!response.ok) { - throw new Error(`GitHub app request ${path} failed with ${response.status}`); + throw new Error(`GitHub App request failed with status ${response.status}`); } return (await response.json()) as T; } @@ -94,7 +94,7 @@ export function createGitHubAppTransport(config: GitHubAppTransportConfig): GitH }, }); if (!response.ok) { - throw new Error(`GitHub installation request ${path} failed with ${response.status}`); + throw new Error(`GitHub repository provider request failed with status ${response.status}`); } return (await response.json()) as T; } @@ -122,20 +122,40 @@ export function createGitHubAppTransport(config: GitHubAppTransportConfig): GitH createInstallationAccessToken, async listRepositories(input) { - const params = new URLSearchParams({ per_page: String(input.perPage), page: String(input.page) }); - const body = await installationRequest<{ total_count: number; repositories: GitHubRepository[] }>( - input.installationId, - `/installation/repositories?${params.toString()}`, - ); const query = input.query?.trim().toLowerCase(); if (!query) { + const params = new URLSearchParams({ per_page: String(input.perPage), page: String(input.page) }); + const body = await installationRequest<{ total_count: number; repositories: GitHubRepository[] }>( + input.installationId, + `/installation/repositories?${params.toString()}`, + ); return { totalCount: body.total_count ?? null, items: body.repositories ?? [] }; } - // The installation-repositories endpoint has no server-side search; filter - // the page client-side. total_count no longer reflects the filtered set. + + // GitHub does not search installation repositories server-side. Walk the + // complete installation catalog before paginating so a match on a later + // upstream page is neither omitted nor given a misleading next cursor. + const matches: GitHubRepository[] = []; + let upstreamPage = 1; + const upstreamPageSize = 100; + for (;;) { + const params = new URLSearchParams({ + per_page: String(upstreamPageSize), + page: String(upstreamPage), + }); + const body = await installationRequest<{ total_count: number; repositories: GitHubRepository[] }>( + input.installationId, + `/installation/repositories?${params.toString()}`, + ); + const pageItems = body.repositories ?? []; + matches.push(...pageItems.filter((repo) => repo.full_name.toLowerCase().includes(query))); + if (pageItems.length < upstreamPageSize || upstreamPage * upstreamPageSize >= body.total_count) break; + upstreamPage += 1; + } + const start = (input.page - 1) * input.perPage; return { - totalCount: null, - items: (body.repositories ?? []).filter((repo) => repo.full_name.toLowerCase().includes(query)), + totalCount: matches.length, + items: matches.slice(start, start + input.perPage), }; },