refactor(backend): centralize Octokit creation

brendan-kellam · brendan-kellam · commit 25cc7753f8d7 · 2026-07-30T15:54:55.000-07:00
diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts
@@ -12,7 +12,7 @@ import { ensureFreshAccountToken, TokenRefreshError } from "./tokenRefresh.js";
 import { DelayedError, Job, Queue, Worker } from "bullmq";
 import { Redis } from "ioredis";
 import {
-    createOctokitFromToken,
+    createOctokit,
     getOAuthScopesForAuthenticatedUser as getGitHubOAuthScopesForAuthenticatedUser,
     getReposForAuthenticatedUser,
 } from "../github.js";
@@ -296,7 +296,7 @@ export class AccountPermissionSyncer {
             }
 
             if (idpConfig.provider === 'github') {
-                const { octokit } = await createOctokitFromToken({
+                const { octokit } = await createOctokit({
                     token: accessToken,
                     url: idpConfig.baseUrl,
                 });
diff --git a/packages/backend/src/ee/repoPermissionSyncer.ts b/packages/backend/src/ee/repoPermissionSyncer.ts
@@ -5,7 +5,7 @@ import { env } from "@sourcebot/shared";
 import { hasEntitlement } from "../entitlements.js";
 import { DelayedError, Job, Queue, Worker } from 'bullmq';
 import { Redis } from 'ioredis';
-import { createOctokitFromToken, getRepoCollaborators, GITHUB_CLOUD_HOSTNAME } from "../github.js";
+import { createOctokit, getRepoCollaborators, GITHUB_CLOUD_HOSTNAME } from "../github.js";
 import { createGitLabFromPersonalAccessToken, getProjectMembers } from "../gitlab.js";
 import { createBitbucketCloudClient, createBitbucketServerClient, getExplicitUserPermissionsForCloudRepo, getUserPermissionsForServerRepo } from "../bitbucket.js";
 import { repoMetadataSchema } from "@sourcebot/shared";
@@ -209,7 +209,7 @@ export class RepoPermissionSyncer {
         }> => {
             if (repo.external_codeHostType === 'github') {
                 const isGitHubCloud = credentials.hostUrl ? new URL(credentials.hostUrl).hostname === GITHUB_CLOUD_HOSTNAME : true;
-                const { octokit } = await createOctokitFromToken({
+                const { octokit } = await createOctokit({
                     token: credentials.token,
                     url: isGitHubCloud ? undefined : credentials.hostUrl,
                 });
diff --git a/packages/backend/src/github.ts b/packages/backend/src/github.ts
@@ -100,60 +100,60 @@ const isHttpError = (error: unknown, status: number): boolean => {
         && error.status === status;
 }
 
-export const createOctokitFromToken = async ({ token, url }: { token?: string, url?: string }): Promise<{ octokit: Octokit, isAuthenticated: boolean }> => {
+/**
+ * Creates an Octokit client using a GitHub App installation when one is
+ * configured for the requested owner, or falls back to the provided token.
+ */
+export const createOctokit = async ({
+    token,
+    url,
+    owner,
+    context,
+}: {
+    token?: string,
+    url?: string,
+    owner?: string,
+    context?: string,
+}): Promise<{ octokit: Octokit, isAuthenticated: boolean }> => {
+    let resolvedToken = token;
+
+    if (owner) {
+        const githubAppManager = GithubAppManager.getInstance();
+        await githubAppManager.ensureInitialized();
+
+        if (githubAppManager.appsConfigured()) {
+            if (!await hasEntitlement('github-app')) {
+                throw new Error(`GitHub App authentication is not currently licensed for ${context ?? owner}.`);
+            }
+
+            try {
+                const hostname = url ? new URL(url).hostname : GITHUB_CLOUD_HOSTNAME;
+                resolvedToken = await githubAppManager.getInstallationToken(owner, hostname);
+            } catch (error) {
+                if (error instanceof GithubAppInstallationNotFoundError) {
+                    throw error;
+                }
+
+                logger.error(`Error getting GitHub App token for ${context ?? owner}.`, error);
+                throw error;
+            }
+        }
+    }
+
     const isGitHubCloud = url ? new URL(url).hostname === GITHUB_CLOUD_HOSTNAME : true;
     const octokit = new Octokit({
-        auth: token,
+        auth: resolvedToken,
         ...(url && !isGitHubCloud ? {
             baseUrl: `${url}/api/v3`
         } : {}),
     });
 
     return {
         octokit,
-        isAuthenticated: !!token,
+        isAuthenticated: !!resolvedToken,
     };
 }
 
-/**
- * Uses GitHub App authentication when an app is configured. App initialization
- * and token failures are propagated so callers cannot mistake a partial,
- * unauthenticated response for an authoritative repository list.
- */
-export const getOctokitWithGithubApp = async (
-    octokit: Octokit,
-    owner: string,
-    url: string | undefined,
-    context: string
-): Promise<Octokit> => {
-    const githubAppManager = GithubAppManager.getInstance();
-    await githubAppManager.ensureInitialized();
-    if (!githubAppManager.appsConfigured()) {
-        return octokit;
-    }
-
-    if (!await hasEntitlement('github-app')) {
-        throw new Error(`GitHub App authentication is not currently licensed for ${context}.`);
-    }
-
-    try {
-        const hostname = url ? new URL(url).hostname : GITHUB_CLOUD_HOSTNAME;
-        const token = await githubAppManager.getInstallationToken(owner, hostname);
-        const { octokit: octokitFromToken } = await createOctokitFromToken({
-            token,
-            url,
-        });
-        return octokitFromToken;
-    } catch (error) {
-        if (error instanceof GithubAppInstallationNotFoundError) {
-            throw error;
-        }
-
-        logger.error(`Error getting GitHub App token for ${context}.`, error);
-        throw error;
-    }
-}
-
 export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, signal: AbortSignal): Promise<{ repos: OctokitRepository[], warnings: string[] }> => {
     const hostname = config.url ?
         new URL(config.url).hostname :
@@ -165,7 +165,7 @@ export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, s
             env.FALLBACK_GITHUB_CLOUD_TOKEN :
             undefined;
 
-    const { octokit, isAuthenticated } = await createOctokitFromToken({
+    const { octokit, isAuthenticated } = await createOctokit({
         token,
         url: config.url,
     });
@@ -185,19 +185,19 @@ export const getGitHubReposFromConfig = async (config: GithubConnectionConfig, s
     let allWarnings: string[] = [];
 
     if (config.orgs) {
-        const { repos, warnings } = await getReposForOrgs(config.orgs, octokit, signal, config.url);
+        const { repos, warnings } = await getReposForOrgs(config.orgs, token, signal, config.url);
         allRepos = allRepos.concat(repos);
         allWarnings = allWarnings.concat(warnings);
     }
 
     if (config.repos) {
-        const { repos, warnings } = await getRepos(config.repos, octokit, signal, config.url);
+        const { repos, warnings } = await getRepos(config.repos, token, signal, config.url);
         allRepos = allRepos.concat(repos);
         allWarnings = allWarnings.concat(warnings);
     }
 
     if (config.users) {
-        const { repos, warnings } = await getReposOwnedByUsers(config.users, octokit, signal, config.url);
+        const { repos, warnings } = await getReposOwnedByUsers(config.users, token, signal, config.url);
         allRepos = allRepos.concat(repos);
         allWarnings = allWarnings.concat(warnings);
     }
@@ -295,12 +295,17 @@ export const getOAuthScopesForAuthenticatedUser = async (octokit: Octokit, token
     }
 }
 
-const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: AbortSignal, url?: string) => {
+const getReposOwnedByUsers = async (users: string[], token: string | undefined, signal: AbortSignal, url?: string) => {
     const results = await Promise.allSettled(users.map((user) => githubQueryLimit(async () => {
         try {
             logger.debug(`Fetching repository info for user ${user}...`);
 
-            const octokitToUse = await getOctokitWithGithubApp(octokit, user, url, `user ${user}`);
+            const { octokit } = await createOctokit({
+                token,
+                url,
+                owner: user,
+                context: `user ${user}`,
+            });
             const { durationMs, data } = await measure(async () => {
                 const fetchFn = async () => {
                     let query = `user:${user}`;
@@ -317,7 +322,7 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A
                     // signal.aborted between pages. paginate() only passes the signal to
                     // individual fetch requests but doesn't check abort state between pages.
                     const allRepos: OctokitRepository[] = [];
-                    const iterator = octokitToUse.paginate.iterator(octokitToUse.rest.search.repos, {
+                    const iterator = octokit.paginate.iterator(octokit.rest.search.repos, {
                         q: query,
                         per_page: 100,
                         request: {
@@ -368,19 +373,24 @@ const getReposOwnedByUsers = async (users: string[], octokit: Octokit, signal: A
     };
 }
 
-const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSignal, url?: string) => {
+const getReposForOrgs = async (orgs: string[], token: string | undefined, signal: AbortSignal, url?: string) => {
     const results = await Promise.allSettled(orgs.map((org) => githubQueryLimit(async () => {
         try {
             logger.debug(`Fetching repository info for org ${org}...`);
 
-            const octokitToUse = await getOctokitWithGithubApp(octokit, org, url, `org ${org}`);
+            const { octokit } = await createOctokit({
+                token,
+                url,
+                owner: org,
+                context: `org ${org}`,
+            });
             const { durationMs, data } = await measure(async () => {
                 // @note: We use paginate.iterator() instead of paginate() to check
                 // signal.aborted between pages. paginate() only passes the signal to
                 // individual fetch requests but doesn't check abort state between pages.
                 const fetchFn = async () => {
                     const allRepos: OctokitRepository[] = [];
-                    const iterator = octokitToUse.paginate.iterator(octokitToUse.repos.listForOrg, {
+                    const iterator = octokit.paginate.iterator(octokit.repos.listForOrg, {
                         org: org,
                         per_page: 100,
                         request: {
@@ -440,15 +450,20 @@ const getReposForOrgs = async (orgs: string[], octokit: Octokit, signal: AbortSi
     };
 }
 
-const getRepos = async (repoList: string[], octokit: Octokit, signal: AbortSignal, url?: string) => {
+const getRepos = async (repoList: string[], token: string | undefined, signal: AbortSignal, url?: string) => {
     const results = await Promise.allSettled(repoList.map((repo) => githubQueryLimit(async () => {
         try {
             const [owner, repoName] = repo.split('/');
             logger.debug(`Fetching repository info for ${repo}...`);
 
-            const octokitToUse = await getOctokitWithGithubApp(octokit, owner, url, `repo ${repo}`);
+            const { octokit } = await createOctokit({
+                token,
+                url,
+                owner,
+                context: `repo ${repo}`,
+            });
             const { durationMs, data: result } = await measure(async () => {
-                const fetchFn = () => octokitToUse.repos.get({
+                const fetchFn = () => octokit.repos.get({
                     owner,
                     repo: repoName,
                     request: {
diff --git a/packages/backend/src/githubAppAuth.test.ts b/packages/backend/src/githubAppAuth.test.ts
@@ -17,10 +17,15 @@ const mocks = vi.hoisted(() => ({
             this.name = 'GithubAppInstallationNotFoundError';
         }
     },
+    octokitOptions: [] as Array<{ auth?: string, baseUrl?: string }>,
 }));
 
 vi.mock('@octokit/rest', () => ({
     Octokit: class {
+        constructor(options: { auth?: string, baseUrl?: string }) {
+            mocks.octokitOptions.push(options);
+        }
+
         public paginate = {
             iterator: async function* (_request: unknown, options: { org: string }) {
                 yield {
@@ -77,10 +82,9 @@ vi.mock('./ee/githubAppManager.js', () => ({
     },
 }));
 
-import type { Octokit } from '@octokit/rest';
-import { getGitHubReposFromConfig, getOctokitWithGithubApp } from './github.js';
+import { createOctokit, getGitHubReposFromConfig } from './github.js';
 
-describe('getOctokitWithGithubApp', () => {
+describe('createOctokit', () => {
     beforeEach(() => {
         mocks.appsConfigured.mockReset().mockReturnValue(true);
         mocks.ensureInitialized.mockReset().mockResolvedValue(undefined);
@@ -90,45 +94,48 @@ describe('getOctokitWithGithubApp', () => {
         mocks.logger.error.mockReset();
         mocks.logger.info.mockReset();
         mocks.logger.warn.mockReset();
+        mocks.octokitOptions.length = 0;
     });
 
     test('fails safely, then uses the GitHub App when the entitlement appears after startup', async () => {
-        const fallbackOctokit = {} as Octokit;
         mocks.hasEntitlement
             .mockResolvedValueOnce(false)
             .mockResolvedValueOnce(true);
 
-        await expect(getOctokitWithGithubApp(
-            fallbackOctokit,
-            'example',
-            undefined,
-            'org example',
-        )).rejects.toThrow('GitHub App authentication is not currently licensed for org example.');
-
-        const entitledOctokit = await getOctokitWithGithubApp(
-            fallbackOctokit,
-            'example',
-            undefined,
-            'org example',
-        );
+        await expect(createOctokit({
+            token: 'legacy-token',
+            owner: 'example',
+            context: 'org example',
+        })).rejects.toThrow('GitHub App authentication is not currently licensed for org example.');
+
+        const result = await createOctokit({
+            token: 'legacy-token',
+            owner: 'example',
+            context: 'org example',
+        });
 
         expect(mocks.ensureInitialized).toHaveBeenCalledTimes(2);
         expect(mocks.getInstallationToken).toHaveBeenCalledWith('example', 'github.com');
-        expect(entitledOctokit).not.toBe(fallbackOctokit);
+        expect(result.isAuthenticated).toBe(true);
+        expect(mocks.octokitOptions).toEqual([{
+            auth: 'installation-token',
+        }]);
     });
 
-    test('uses legacy authentication when no GitHub App is configured', async () => {
-        const fallbackOctokit = {} as Octokit;
+    test('falls back to token authentication when no GitHub App is configured', async () => {
         mocks.appsConfigured.mockReturnValue(false);
         mocks.hasEntitlement.mockResolvedValue(false);
 
-        await expect(getOctokitWithGithubApp(
-            fallbackOctokit,
-            'example',
-            undefined,
-            'org example',
-        )).resolves.toBe(fallbackOctokit);
+        const result = await createOctokit({
+            token: 'legacy-token',
+            owner: 'example',
+            context: 'org example',
+        });
 
+        expect(result.isAuthenticated).toBe(true);
+        expect(mocks.octokitOptions).toEqual([{
+            auth: 'legacy-token',
+        }]);
         expect(mocks.hasEntitlement).not.toHaveBeenCalled();
     });
 
@@ -137,12 +144,12 @@ describe('getOctokitWithGithubApp', () => {
         mocks.hasEntitlement.mockResolvedValue(true);
         mocks.getInstallationToken.mockRejectedValue(error);
 
-        await expect(getOctokitWithGithubApp(
-            {} as Octokit,
-            'example',
-            undefined,
-            'org example',
-        )).rejects.toBe(error);
+        await expect(createOctokit({
+            token: 'legacy-token',
+            owner: 'example',
+            context: 'org example',
+        })).rejects.toBe(error);
+        expect(mocks.octokitOptions).toEqual([]);
     });
 
     test('warns and continues when the GitHub App is not installed for one organization', async () => {
