Skip to content

Commit ba7c23e

Browse files
committed
fix(scan github): propagate blocking errors during manifest download
A token-wide GitHub block (rate limit / auth / abuse detection) hit while downloading a manifest file was swallowed when an earlier file had already downloaded, so the scan proceeded on a partial manifest set and reported success. Surface the blocking error immediately, mirroring the repo loop. Also suppress the green per-run success lines on the all-repos-failed path so scripts reading log lines cannot infer success from a run that returns ok:false, and drop internal tracker IDs from comments and a test title.
1 parent 30806f1 commit ba7c23e

3 files changed

Lines changed: 28 additions & 15 deletions

File tree

src/commands/scan/create-scan-from-github.mts

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,12 +113,12 @@ export async function createScanFromGithub({
113113
* Drive the per-repo scan loop and decide the overall run result.
114114
*
115115
* The loop stops early on a blocking GitHub error (rate limit / auth / abuse
116-
* detection) because every remaining repo would fail the same way — that is
117-
* the ASK-167 bug: a rate-limited token made every repo fail its API calls,
118-
* the loop swallowed each failure, and the final "N repos / 0 manifests"
119-
* summary misled users and CI into thinking the scan succeeded when nothing
120-
* was uploaded. A run where every attempted repo failed for a non-blocking
121-
* reason is also surfaced as an error rather than a silent ok:true.
116+
* detection) because every remaining repo would fail the same way. Previously
117+
* a rate-limited token made every repo fail its API calls, the loop swallowed
118+
* each failure, and the final "N repos / 0 manifests" summary misled users and
119+
* CI into thinking the scan succeeded when nothing was uploaded. A run where
120+
* every attempted repo failed for a non-blocking reason is also surfaced as an
121+
* error rather than a silent ok:true.
122122
*
123123
* `scanRepoFn` is injected so this decision logic can be tested without the
124124
* GitHub network path.
@@ -161,12 +161,11 @@ export async function runGithubScanLoop(
161161
return blockingError
162162
}
163163

164-
logger.success(reposScanned, 'GitHub repos processed')
165-
logger.success(scansCreated, 'with supported Manifest files')
166-
167164
// If every attempted repo failed (but not for a known blocking reason),
168165
// treat the run as an error so scripts do not infer success from an
169-
// ok:true with zero scans created.
166+
// ok:true with zero scans created. Checked before the success lines below
167+
// so an all-failed run never prints green "processed" output that a script
168+
// reading log lines could mistake for success.
170169
if (
171170
reposScanned > 0 &&
172171
scansCreated === 0 &&
@@ -182,6 +181,9 @@ export async function runGithubScanLoop(
182181
}
183182
}
184183

184+
logger.success(reposScanned, 'GitHub repos processed')
185+
logger.success(scansCreated, 'with supported Manifest files')
186+
185187
return {
186188
ok: true,
187189
data: undefined,
@@ -389,8 +391,20 @@ async function testAndDownloadManifestFiles({
389391
if (result.data.isManifest) {
390392
fileCount += 1
391393
}
392-
} else if (!firstFailureResult) {
393-
firstFailureResult = result
394+
} else {
395+
// A blocking error (rate limit / auth / abuse detection) is token-wide:
396+
// every remaining download would fail the same way, and an earlier
397+
// successful download must not mask it into an ok:true scan of a
398+
// partial manifest set. Surface it immediately, the same way the repo
399+
// loop short-circuits on blocking errors.
400+
if (isGitHubBlockingError(result.message)) {
401+
logger.groupEnd()
402+
logger.fail(result.message)
403+
return result
404+
}
405+
if (!firstFailureResult) {
406+
firstFailureResult = result
407+
}
394408
}
395409
}
396410
logger.groupEnd()

src/commands/scan/create-scan-from-github.test.mts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ const authFailed: ScanResult = {
4545
cause: 'GitHub authentication failed.',
4646
}
4747

48-
describe('runGithubScanLoop (ASK-167)', () => {
48+
describe('runGithubScanLoop GitHub blocking-error handling', () => {
4949
it('stops and returns ok:false on a GitHub rate limit', async () => {
5050
const scanner = fakeScanner({
5151
'repo-a': rateLimited,

src/utils/github-errors.mts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
* to read every response body and JSON-parse it without ever inspecting the
77
* HTTP status, so a rate-limit response (`403` with `x-ratelimit-remaining: 0`,
88
* `429`, or a secondary-limit body) was misread as "repo has no default
9-
* branch / no manifests" and the run reported a silent success. See Linear
10-
* ASK-167.
9+
* branch / no manifests" and the run reported a silent success.
1110
*
1211
* This module centralizes:
1312
* - Classifying a GitHub response as a blocking error (rate limit / abuse

0 commit comments

Comments
 (0)