fix(github-app): fail over to another app on rate limit, attribute retry-path calls correctly

docs/rate-limits-and-tuning.md

@@ -54,6 +54,8 @@ Without a token cache, each runner also costs a `POST /app/installations/{id}/ac
 
 Rate limits are per App installation and cannot be raised. To scale beyond one App's budget, configure extra Apps with `additional_github_apps`. The control-plane lambdas select one App per invocation, making the effective limit N × the per-App limit. Selection prefers the App with the most rate-limit budget remaining, based on the `x-ratelimit-remaining` headers observed by the running Lambda container; Apps that hit a secondary rate limit are skipped for 60 seconds.
 
+If the selected App still gets rate-limited mid-invocation (its budget was exhausted since the last time headers were observed, or a burst raced past that check), the job-status check (`isJobQueued`, in both the scale-up and retry lambdas) fails over immediately to another configured App with headroom instead of sleeping out the `retry-after` window. Failover keeps trying further Apps (bounded) if a second one also turns out exhausted, so it isn't limited to two-App deployments. The registration-token and JIT-config API calls (the ones that create the actual runner) don't yet get this reactive failover — they still rely on the proactive selection above and, if rate-limited, sleep out the window like before.
+
 > [!IMPORTANT]
 > Every additional App must be installed on the same organizations or repositories as the primary App. The module cannot verify this. A missing installation surfaces at runtime as installation lookup 404s on the fraction of invocations that select the misconfigured App, which is hard to trace back to the installation.
 

lambdas/functions/control-plane/src/github/auth.test.ts

@@ -10,6 +10,8 @@ import {
   createGithubAppAuth,
   createOctokitClient,
   getStoredInstallationId,
+  hasAlternativeAppWithHeadroom,
+  isGitHubRateLimitError,
   onRateLimit,
   onSecondaryRateLimit,
   reportAppRateLimit,
@@ -533,4 +535,143 @@ describe('Test rate-limit aware app selection', () => {
     const result = await createGithubAppAuth(undefined, '', 1);
     expect(result.appIndex).toBe(1);
   });
+
+  it('excludes the given app index from selection, e.g. one that just got rate-limited', async () => {
+    reportAppRateLimit(0, 5000);
+    reportAppRateLimit(1, 100);
+
+    const result = await createGithubAppAuth(undefined, '', undefined, undefined, 0);
+    expect(result.appIndex).toBe(1);
+  });
+
+  it('falls back to the excluded app when it is the only one configured', async () => {
+    delete process.env.PARAMETER_GITHUB_APPS_MANIFEST_NAME;
+
+    const result = await createGithubAppAuth(undefined, '', undefined, undefined, 0);
+    expect(result.appIndex).toBe(0);
+  });
+});
+
+describe('Test app selection with 3+ apps (multi-app failover)', () => {
+  const decryptedValue = 'decryptedValue';
+  const b64 = Buffer.from(decryptedValue, 'binary').toString('base64');
+  const app2IdParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_id`;
+  const app2KeyParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_key_base64`;
+  const app3IdParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_1_id`;
+  const app3KeyParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_1_key_base64`;
+
+  beforeEach(() => {
+    const mockedAuth = vi.fn().mockResolvedValue({ token: 'token' });
+    vi.mocked(createAppAuth).mockReturnValue(Object.assign(mockedAuth, { hook: vi.fn() }));
+
+    process.env.PARAMETER_GITHUB_APPS_MANIFEST_NAME = `/actions-runner/${ENVIRONMENT}/additional_github_apps_manifest`;
+    mockedGetParameter.mockResolvedValue(
+      JSON.stringify([
+        { idParamName: app2IdParam, keyParamName: app2KeyParam },
+        { idParamName: app3IdParam, keyParamName: app3KeyParam },
+      ]),
+    );
+    mockedGetParameters.mockResolvedValue(
+      new Map([
+        [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID],
+        [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64],
+        [app2IdParam, '2'],
+        [app2KeyParam, b64],
+        [app3IdParam, '3'],
+        [app3KeyParam, b64],
+      ]),
+    );
+    vi.spyOn(Math, 'random').mockReturnValue(0);
+  });
+
+  it('excludes every already-tried app, not just the most recent one, when given an array', async () => {
+    reportAppRateLimit(0, 5000);
+    reportAppRateLimit(1, 4000);
+    reportAppRateLimit(2, 3000);
+
+    const result = await createGithubAppAuth(undefined, '', undefined, undefined, [0, 1]);
+    expect(result.appIndex).toBe(2);
+  });
+
+  it('hasAlternativeAppWithHeadroom finds the third app once the first two are excluded', async () => {
+    await createGithubAppAuth(undefined); // populate the credentials cache
+    reportAppRateLimit(0, 0);
+    reportAppRateLimit(1, 0);
+    reportAppRateLimit(2, 100);
+
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(true); // apps 1/2 still uninspected in this call
+    expect(hasAlternativeAppWithHeadroom([0, 1])).toBe(true); // app 2 still has budget
+    expect(hasAlternativeAppWithHeadroom([0, 1, 2])).toBe(false); // nothing left to try
+  });
+});
+
+describe('Test hasAlternativeAppWithHeadroom', () => {
+  const decryptedValue = 'decryptedValue';
+  const b64 = Buffer.from(decryptedValue, 'binary').toString('base64');
+  const app2IdParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_id`;
+  const app2KeyParam = `/actions-runner/${ENVIRONMENT}/additional_github_app_0_key_base64`;
+
+  beforeEach(async () => {
+    const mockedAuth = vi.fn().mockResolvedValue({ token: 'token' });
+    vi.mocked(createAppAuth).mockReturnValue(Object.assign(mockedAuth, { hook: vi.fn() }));
+
+    process.env.PARAMETER_GITHUB_APPS_MANIFEST_NAME = `/actions-runner/${ENVIRONMENT}/additional_github_apps_manifest`;
+    mockedGetParameter.mockResolvedValue(JSON.stringify([{ idParamName: app2IdParam, keyParamName: app2KeyParam }]));
+    mockedGetParameters.mockResolvedValue(
+      new Map([
+        [PARAMETER_GITHUB_APP_ID_NAME, GITHUB_APP_ID],
+        [PARAMETER_GITHUB_APP_KEY_BASE64_NAME, b64],
+        [app2IdParam, '2'],
+        [app2KeyParam, b64],
+      ]),
+    );
+    // Populates the credentials cache that the sync check reads.
+    await createGithubAppAuth(undefined);
+  });
+
+  it('returns false before any credentials have been loaded', () => {
+    resetAppCredentialsCache();
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(false);
+  });
+
+  it('returns true when another app has remaining budget', () => {
+    reportAppRateLimit(1, 100);
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(true);
+  });
+
+  it('returns false when the only other app is exhausted', () => {
+    reportAppRateLimit(1, 0);
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(false);
+  });
+
+  it('returns false when the only other app is cooling down from a secondary rate limit', () => {
+    reportAppRateLimit(1, 100);
+    reportAppSecondaryRateLimit(1);
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(false);
+  });
+
+  it('returns false in a single-app deployment', async () => {
+    resetAppCredentialsCache();
+    delete process.env.PARAMETER_GITHUB_APPS_MANIFEST_NAME;
+    await createGithubAppAuth(undefined);
+
+    expect(hasAlternativeAppWithHeadroom(0)).toBe(false);
+  });
+});
+
+describe('Test isGitHubRateLimitError', () => {
+  it.each([
+    [
+      'a 403 with x-ratelimit-remaining: 0',
+      { status: 403, response: { headers: { 'x-ratelimit-remaining': '0' } } },
+      true,
+    ],
+    ['a 429 with a rate limit message', { status: 429, message: 'You have exceeded a secondary rate limit' }, true],
+    ['a 403 that is a plain permission error', { status: 403, response: { headers: {} } }, false],
+    ['a 404', { status: 404 }, false],
+    ['a non-error value', 'not an error', false],
+    ['null', null, false],
+  ])('%s -> %s', (_description, error, expected) => {
+    expect(isGitHubRateLimitError(error)).toBe(expected);
+  });
 });

lambdas/functions/control-plane/src/github/auth.ts

@@ -83,19 +83,29 @@ export function reportAppSecondaryRateLimit(appIndex: number): void {
   logger.warn(`GitHub App index ${appIndex} put in secondary rate limit cooldown`);
 }
 
-// Select the app with the most primary rate limit budget remaining, skipping
-// apps cooling down after a secondary rate limit. Apps with no observed state
-// are assumed full. Iteration starts at a random offset so concurrent
-// cold-started lambdas do not all converge on the same app.
-async function selectAppIndex(credentialsStore?: GitHubAppCredentialsStore): Promise<number> {
+// One or more apps to skip during selection, e.g. apps already tried in a failover sequence.
+export type AppIndexExclusion = number | number[];
+
+function toExcludeSet(exclude?: AppIndexExclusion): Set<number> {
+  if (exclude === undefined) return new Set();
+  return new Set(Array.isArray(exclude) ? exclude : [exclude]);
+}
+
+// Picks the app with the most rate-limit budget left, skipping excluded/cooling-down apps; random offset avoids concurrent cold starts converging on the same app.
+async function selectAppIndex(
+  credentialsStore?: GitHubAppCredentialsStore,
+  excludeAppIndexes?: AppIndexExclusion,
+): Promise<number> {
   const credentials = await getAppCredentials(credentialsStore);
+  const exclude = toExcludeSet(excludeAppIndexes);
   if (credentials.length === 1) return 0;
   const now = Date.now();
   const offset = Math.floor(Math.random() * credentials.length);
   let best = -1;
   let bestRemaining = -1;
   for (let n = 0; n < credentials.length; n++) {
     const i = (offset + n) % credentials.length;
+    if (exclude.has(i)) continue;
     const state = appRateLimitStates.get(i);
     if (state && state.cooldownUntil > now) continue;
     const remaining = state?.remaining ?? Number.MAX_SAFE_INTEGER;
@@ -105,26 +115,45 @@ async function selectAppIndex(credentialsStore?: GitHubAppCredentialsStore): Pro
     }
   }
   if (best === -1) {
-    // Every app is cooling down; pick the one with the most remaining anyway.
+    // Every non-excluded app is cooling down; pick the one with the most remaining anyway.
     for (let i = 0; i < credentials.length; i++) {
+      if (exclude.has(i)) continue;
       const remaining = appRateLimitStates.get(i)?.remaining ?? Number.MAX_SAFE_INTEGER;
       if (remaining > bestRemaining) {
         bestRemaining = remaining;
         best = i;
       }
     }
   }
+  if (best === -1) best = exclude.size > 0 ? [...exclude][0] : 0; // only excluded apps exist; nothing else to pick
   // Info so the app selection distribution is observable at default log level.
   logger.info(`Selected GitHub App index ${best} with ${bestRemaining} rate limit remaining`);
   return best;
 }
 
+let cachedAppCredentials: GitHubAppCredential[] | null = null;
+
 async function loadAppCredentials(): Promise<GitHubAppCredential[]> {
   const credentials = await createCommonStorage().githubAppCredentials.get();
   logger.info(`Loaded ${credentials.length} GitHub App credential(s)`);
+  cachedAppCredentials = credentials;
   return credentials;
 }
 
+// Sync (for the throttle plugin's callbacks); relies on createGithubAppAuth() having already cached credentials earlier in the same auth flow.
+export function hasAlternativeAppWithHeadroom(excludeAppIndexes: AppIndexExclusion): boolean {
+  if (!cachedAppCredentials || cachedAppCredentials.length <= 1) return false;
+  const exclude = toExcludeSet(excludeAppIndexes);
+  const now = Date.now();
+  for (let i = 0; i < cachedAppCredentials.length; i++) {
+    if (exclude.has(i)) continue;
+    const state = appRateLimitStates.get(i);
+    if (state && state.cooldownUntil > now) continue;
+    if ((state?.remaining ?? Number.MAX_SAFE_INTEGER) > 0) return true;
+  }
+  return false;
+}
+
 function getAppCredentials(credentialsStore?: GitHubAppCredentialsStore): Promise<GitHubAppCredential[]> {
   if (credentialsStore) {
     return credentialsStore.get();
@@ -139,6 +168,7 @@ export async function getAppCount(credentialsStore?: GitHubAppCredentialsStore):
 
 export function resetAppCredentialsCache(): void {
   appCredentialsPromise = null;
+  cachedAppCredentials = null;
   appRateLimitStates.clear();
 }
 
@@ -188,8 +218,14 @@ export async function createOctokitClient(token: string, ghesApiUrl = '', appInd
         retryCount: number,
       ) => {
         if (appIndex !== undefined) {
-          // Primary budget exhausted for this app; steer new flows elsewhere.
-          reportAppRateLimit(appIndex, 0);
+          reportAppRateLimit(appIndex, 0); // primary budget exhausted for this app; steer new flows elsewhere
+          if (hasAlternativeAppWithHeadroom(appIndex)) {
+            logger.warn(
+              `GitHub App index ${appIndex} rate-limited with an alternate app available; ` +
+                `failing over instead of waiting ${retryAfter}s`,
+            );
+            return false;
+          }
         }
         return onRateLimit(retryAfter, options, octokit, retryCount);
       },
@@ -201,6 +237,13 @@ export async function createOctokitClient(token: string, ghesApiUrl = '', appInd
       ) => {
         if (appIndex !== undefined) {
           reportAppSecondaryRateLimit(appIndex);
+          if (hasAlternativeAppWithHeadroom(appIndex)) {
+            logger.warn(
+              `GitHub App index ${appIndex} secondary rate-limited with an alternate app available; ` +
+                `failing over instead of waiting ${retryAfter}s`,
+            );
+            return false;
+          }
         }
         return onSecondaryRateLimit(retryAfter, options, octokit, retryCount);
       },
@@ -213,12 +256,21 @@ export async function createGithubAppAuth(
   ghesApiUrl = '',
   appIndex?: number,
   credentialsStore?: GitHubAppCredentialsStore,
+  excludeAppIndexes?: AppIndexExclusion,
 ): Promise<AppAuthentication & { appIndex: number }> {
-  const idx = appIndex ?? (await selectAppIndex(credentialsStore));
+  const idx = appIndex ?? (await selectAppIndex(credentialsStore, excludeAppIndexes));
   const auth = await createAuth(installationId, ghesApiUrl, idx, credentialsStore);
   return { ...(await auth({ type: 'app' })), appIndex: idx };
 }
 
+export function isGitHubRateLimitError(error: unknown): boolean {
+  if (typeof error !== 'object' || error === null) return false;
+  const err = error as { status?: number; response?: { headers?: Record<string, string> }; message?: string };
+  if (err.status !== 403 && err.status !== 429) return false;
+  if (err.response?.headers?.['x-ratelimit-remaining'] === '0') return true;
+  return typeof err.message === 'string' && /rate limit/i.test(err.message);
+}
+
 export async function createGithubInstallationAuth(
   installationId: number | undefined,
   ghesApiUrl = '',