fix: track per-repo attempted tokens and match HTTP status by word boundary

Signed-off-by: Joana Maia <jmaia@contractor.linuxfoundation.org>

Author: joanagmaia

services/apps/security_best_practices_worker/src/activities/index.ts

@@ -203,8 +203,10 @@ function classifyTokenError(output: string, source: string): void {
     })
   }
 
-  const has403 = output.includes('403')
-  const has429 = output.includes('429')
+  // Word-boundary match so unrelated numbers (e.g. "processed 4291 records") don't get
+  // misclassified as HTTP 429/403.
+  const has403 = /\b403\b/.test(output)
+  const has429 = /\b429\b/.test(output)
   if (!has403 && !has429) return
 
   // Rate-limit responses carry a distinctive body: "API rate limit exceeded" or

services/apps/security_best_practices_worker/src/workflows/triggerSecurityInsightsCheckForRepos.ts

@@ -70,23 +70,33 @@ export async function triggerSecurityInsightsCheckForRepos(
     // via `!tokenInfo`, so requeue decisions have to be made here too — otherwise a rate-limit
     // wave would falsely fail the repo for the rest of the continueAsNew chain.
     let sawRateLimit = false
+    // Per-repo attempted-tokens set. TokenRepoAccessError releases the token in "clean"
+    // global state, so without this filter `getNextToken` could re-pick the same token
+    // while the others are held by concurrent children — burning attempts without ever
+    // reaching a token that might have access.
+    const attemptedTokens = new Set<string>()
     while (attempts < MAX_TOKEN_ATTEMPTS) {
-      const tokenInfo = getNextToken(tokenInfos, currentTimeMs)
+      const tokenInfo = getNextToken(tokenInfos, currentTimeMs, attemptedTokens)
       if (!tokenInfo) {
-        // Only mark the repo failed if every token is permanently invalid. Rate-limited
-        // tokens recover after 1h and in-use tokens free up as concurrent tasks finish, so
-        // in both cases requeue the repo — the outer loop's hasFreeToken gate + defer path
-        // handles the wait.
+        // No untried, usable token remains for this repo. Distinguish two cases:
+        // - all tokens permanently invalid, OR every non-invalid token has been tried for
+        //   this repo → this repo genuinely can't be scanned, fail it
+        // - some non-invalid, non-attempted tokens exist but are currently in-use or
+        //   rate-limited → requeue so a later iteration can retry when they free up
         const allPermanentlyInvalid = tokenInfos.every((t) => t.isInvalid)
-        if (allPermanentlyInvalid) {
-          console.error(`All tokens permanently invalid for repo ${repo.repoUrl}, skipping`)
+        const anyUnattemptedNonInvalid = tokenInfos.some(
+          (t) => !t.isInvalid && !attemptedTokens.has(t.token),
+        )
+        if (allPermanentlyInvalid || !anyUnattemptedNonInvalid) {
+          console.error(`No untried tokens remain for repo ${repo.repoUrl}, skipping`)
           failedRepoUrls.push(repo.repoUrl)
         } else {
           queue.unshift(repo)
         }
         break
       }
       const token = tokenInfo.token
+      attemptedTokens.add(token)
       tokenInfo.inUse = true
 
       try {
@@ -256,10 +266,17 @@ function refreshExpiredRateLimits(tokenInfos: ITokenInfo[], nowMs: number): void
   }
 }
 
-function getNextToken(tokenInfos: ITokenInfo[], nowMs: number): ITokenInfo | null {
+function getNextToken(
+  tokenInfos: ITokenInfo[],
+  nowMs: number,
+  excludeTokens?: Set<string>,
+): ITokenInfo | null {
   refreshExpiredRateLimits(tokenInfos, nowMs)
 
-  const usableTokenInfos = tokenInfos.filter((t) => !t.inUse && !t.isRateLimited && !t.isInvalid)
+  const usableTokenInfos = tokenInfos.filter(
+    (t) =>
+      !t.inUse && !t.isRateLimited && !t.isInvalid && !(excludeTokens && excludeTokens.has(t.token)),
+  )
 
   // sort usable tokens by last used date from oldest to newest
   const sortedTokenInfos = usableTokenInfos.sort((a, b) => {