diff --git a/src/orb/installed-repos-sync.ts b/src/orb/installed-repos-sync.ts index 497ef242e9..da8c0d4ed6 100644 --- a/src/orb/installed-repos-sync.ts +++ b/src/orb/installed-repos-sync.ts @@ -27,8 +27,12 @@ export type InstalledReposSyncResult = /** Fetch every repo currently accessible to this brokered installation via GitHub's own * `GET /installation/repositories`, paginated. Uses the broker token directly -- its response already carries * the bound installationId, so no separate token mint is needed for this call. */ -async function fetchAllInstallationRepos(token: string, fetchImpl: typeof fetch): Promise { +export async function fetchAllInstallationRepos(token: string, fetchImpl: typeof fetch): Promise<{ repos: GitHubRepositoryPayload[]; truncated: boolean }> { const repos: GitHubRepositoryPayload[] = []; + // #10033: TRUE only when the loop exhausts MAX_INSTALLATION_REPOS_PAGES with a still-full last page -- a + // short page means the list is complete, but a full final page means there is an untraversed tail. The caller + // must not treat a truncated list as negative evidence and un-install the repos it never saw. + let truncated = false; for (let page = 1; page <= MAX_INSTALLATION_REPOS_PAGES; page += 1) { const res = await fetchImpl(`https://api.github.com/installation/repositories?per_page=${GITHUB_INSTALLATION_REPOS_PAGE_SIZE}&page=${page}`, { headers: githubHeaders({ token }), @@ -39,8 +43,9 @@ async function fetchAllInstallationRepos(token: string, fetchImpl: typeof fetch) const batch = body.repositories ?? []; repos.push(...batch); if (batch.length < GITHUB_INSTALLATION_REPOS_PAGE_SIZE) break; + if (page === MAX_INSTALLATION_REPOS_PAGES) truncated = true; } - return repos; + return { repos, truncated }; } /** @@ -65,10 +70,19 @@ export async function syncBrokeredInstalledRepos( if (!isOrbBrokerMode(env)) return { status: "skipped" }; try { const { token, installationId } = await fetchBrokeredInstallationToken(env, fetchImpl); - const repos = await fetchAllInstallationRepos(token, fetchImpl); + const { repos, truncated } = await fetchAllInstallationRepos(token, fetchImpl); + // A partial list is valid POSITIVE evidence: still upsert every repo we DID fetch as installed. for (const repo of repos) { await upsertRepositoryFromGitHub(env, repo, installationId); } + if (truncated) { + // #10033: a page-capped crawl is not valid NEGATIVE evidence -- reconciling against it would flip every + // installed repo in the untraversed tail to isInstalled: false and take them silently dark. Skip the + // destructive reconcile entirely, but still report `synced` (the sync did useful upsert work; the cron + // must not read this as an outage) with removedCount 0, and log so an operator sees the suppression. + console.error(JSON.stringify({ level: "error", event: "installed_repos_sync_truncated", installationId, repoCount: repos.length })); + return { status: "synced", installationId, repoCount: repos.length, removedCount: 0 }; + } const freshFullNames = new Set(repos.map((repo) => repo.full_name)); const previouslyInstalled = await listInstalledRepoFullNamesForInstallation(env, installationId); const staleFullNames = previouslyInstalled.filter((fullName) => !freshFullNames.has(fullName)); diff --git a/test/unit/orb-installed-repos-sync.test.ts b/test/unit/orb-installed-repos-sync.test.ts index 5a78a2d9b5..345c625668 100644 --- a/test/unit/orb-installed-repos-sync.test.ts +++ b/test/unit/orb-installed-repos-sync.test.ts @@ -1,8 +1,13 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import * as repositories from "../../src/db/repositories"; import { getRepository } from "../../src/db/repositories"; -import { syncBrokeredInstalledRepos } from "../../src/orb/installed-repos-sync"; +import { fetchAllInstallationRepos, syncBrokeredInstalledRepos } from "../../src/orb/installed-repos-sync"; import { createTestEnv } from "../helpers/d1"; +afterEach(() => { + vi.restoreAllMocks(); +}); + type Call = { url: string; init?: RequestInit | undefined }; /** Router-style fetch stub: the broker token exchange goes to `.../v1/orb/token`, everything else is treated @@ -105,4 +110,57 @@ describe("syncBrokeredInstalledRepos", () => { const result = await syncBrokeredInstalledRepos(env, fetchImpl); expect(result).toEqual({ status: "synced", installationId: 42, repoCount: 0, removedCount: 0 }); }); + + // #10033: fetchAllInstallationRepos must report truncation so the caller does not treat a page-capped list + // as negative evidence and un-install the untraversed tail. + const fullPage = (start: number) => Response.json({ repositories: Array.from({ length: 100 }, (_, i) => repoPayload(`owner/repo-${start + i}`)) }); + + it("#10033: fetchAllInstallationRepos reports truncated=true only when the page cap is hit with a full last page", async () => { + // 50 full pages → the cap is exhausted while a full page is still pending → truncated, 5000 repos. + const capped = routedFetch({ tokenResponse: tokenResponse(42), pages: Array.from({ length: 50 }, (_, p) => fullPage(p * 100)) }); + await expect(fetchAllInstallationRepos("tok", capped.fetchImpl)).resolves.toMatchObject({ truncated: true, repos: expect.any(Array) }); + expect((await fetchAllInstallationRepos("tok", routedFetch({ tokenResponse: tokenResponse(42), pages: Array.from({ length: 50 }, (_, p) => fullPage(p * 100)) }).fetchImpl)).repos).toHaveLength(5000); + + // 100 then 40 (short page) → complete, 140 repos. + const short = routedFetch({ tokenResponse: tokenResponse(42), pages: [fullPage(0), Response.json({ repositories: Array.from({ length: 40 }, (_, i) => repoPayload(`owner/tail-${i}`)) })] }); + await expect(fetchAllInstallationRepos("tok", short.fetchImpl)).resolves.toMatchObject({ truncated: false }); + + // Exactly 100 then 0 (empty page) → complete, 100 repos. + const emptyTail = routedFetch({ tokenResponse: tokenResponse(42), pages: [fullPage(0), Response.json({ repositories: [] })] }); + const res = await fetchAllInstallationRepos("tok", emptyTail.fetchImpl); + expect(res).toMatchObject({ truncated: false }); + expect(res.repos).toHaveLength(100); + }); + + it("#10033: a truncated crawl calls markRepositoriesRemovedFromInstallation ZERO times but still upserts every fetched repo", async () => { + const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_x" }); + vi.spyOn(console, "error").mockImplementation(() => {}); + const removeSpy = vi.spyOn(repositories, "markRepositoriesRemovedFromInstallation"); + const upsertSpy = vi.spyOn(repositories, "upsertRepositoryFromGitHub"); + const capped = routedFetch({ tokenResponse: tokenResponse(42), pages: Array.from({ length: 50 }, (_, p) => fullPage(p * 100)) }); + await syncBrokeredInstalledRepos(env, capped.fetchImpl); + expect(removeSpy).not.toHaveBeenCalled(); + expect(upsertSpy).toHaveBeenCalledTimes(5000); + }); + + it("#10033 REGRESSION: a page-capped installation-repositories crawl must not un-install the untraversed tail", async () => { + const env = createTestEnv({ ORB_ENROLLMENT_SECRET: "orbsec_x" }); + // First a COMPLETE sync installs a repo that a later truncated crawl will not surface. + const first = routedFetch({ tokenResponse: tokenResponse(42), pages: [Response.json({ repositories: [repoPayload("owner/in-the-tail")] })] }); + await syncBrokeredInstalledRepos(env, first.fetchImpl); + await expect(getRepository(env, "owner/in-the-tail")).resolves.toMatchObject({ isInstalled: true }); + + // Now a TRUNCATED crawl (50 full pages, none of them the tail repo): the reconcile must be suppressed. + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const capped = routedFetch({ tokenResponse: tokenResponse(42), pages: Array.from({ length: 50 }, (_, p) => fullPage(p * 100)) }); + const result = await syncBrokeredInstalledRepos(env, capped.fetchImpl); + + expect(result).toEqual({ status: "synced", installationId: 42, repoCount: 5000, removedCount: 0 }); + // The tail repo the truncated crawl never saw stays installed — NOT flipped to isInstalled: false. + await expect(getRepository(env, "owner/in-the-tail")).resolves.toMatchObject({ isInstalled: true, installationId: 42 }); + // One structured error line records the suppressed reconcile. + const truncWarns = errorSpy.mock.calls.map((c) => String(c[0])).filter((m) => m.includes("installed_repos_sync_truncated")); + expect(truncWarns).toHaveLength(1); + expect(truncWarns[0]).toContain("\"repoCount\":5000"); + }); });