Skip to content

fix(scan github): surface GitHub rate limits instead of silent success - #1426

Open
John-David Dalton (jdalton) wants to merge 1 commit into
v1.xfrom
jdalton/ask-167-improve-cli-to-handle-github-api-rate-limiting-issues
Open

fix(scan github): surface GitHub rate limits instead of silent success#1426
John-David Dalton (jdalton) wants to merge 1 commit into
v1.xfrom
jdalton/ask-167-improve-cli-to-handle-github-api-rate-limiting-issues

Conversation

@jdalton

@jdalton John-David Dalton (jdalton) commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

When your GitHub token is rate-limited, socket scan github currently tells you it succeeded. It exits 0 and prints:

✔ N GitHub repos detected
✔ 0 with supported Manifest files

Nothing was uploaded. Every repo failed, and the run reported green. On a laptop that is confusing; in CI it is worse, because a pipeline step that exits 0 is a pipeline step nobody looks at. Teams can go a long time believing their repos are being scanned when no scan has happened at all.

After this change, a rate-limited, unauthorized, or abuse-throttled run stops at the first blocking error, prints what hit and — when GitHub tells us — when it resets, and exits non-zero. Genuinely empty repos with no manifests still succeed, exactly as before.

Refs ASK-167.

Why it looked like success — the status code was never read

The GitHub-API scanning path read every GitHub REST response body and JSON-parsed it without ever checking the HTTP status. When a token is rate-limited, GitHub answers with one of three shapes:

Response What it means
403 with x-ratelimit-remaining: 0 primary rate limit, quota exhausted
429 rate limited
403 with a secondary rate limit body secondary limit / abuse detection

None of those parse into the JSON the code expected, so the old code read them as "repo has no default branch / no manifests". The per-repo error was then swallowed by the scan loop, and the run finished with the success lines above.

The change — classify the response, retry only what is worth retrying, stop the loop on a blocking error

New utils/github-errors.mts holds two pieces:

  • classifyGitHubResponse detects rate-limit, abuse-detection and auth responses and returns a clear, actionable CResult error. It returns nothing for anything else, so the caller keeps its own handling of 404s, empty repos and the rest.
  • githubApiRequest is a bounded-retry wrapper around apiFetch. It waits once and retries when the reset window is short (Retry-After or x-ratelimit-reset within 30 seconds), surfaces long hourly resets immediately rather than blocking a CLI run on them, retries 5xx and network failures with capped exponential backoff, and never retries an auth failure.

Wired in at the call sites:

  • The repo-details, tree, commit and contents calls all go through githubApiRequest. The raw manifest-download host is checked too, so a bulk run that trips the limit mid-download fails loudly instead of counting the file as skipped.
  • The per-repo loop is extracted into runGithubScanLoop, which stops at the first blocking error (rate limit / auth / abuse) and returns ok: false, giving a non-zero exit. A run where every repo failed is also no longer reported as success.

The success line now reads N GitHub repos processed rather than N GitHub repos detected, since "detected" was part of what made the old output read as a completed scan.

Scenario Before After
rate-limited token exit 0, "0 manifests" exit 1, GitHub rate limit exceeded …
every repo failed exit 0 exit 1, with the first repo's failure quoted
empty repo success success (unchanged)
What I checked — 25 tests, no network and no module mocks

src/utils/github-errors.test.mts and src/commands/scan/create-scan-from-github.test.mts. Fake fetch and scanRepoFn are injected as plain functions rather than mocked at the module level, so the tests exercise the real decision logic.

They cover detection of 403 + remaining: 0, 429, secondary-limit and 401; Retry-After and reset parsing; the cheap-retry-then-succeed path; 5xx backoff; the loop stopping on a blocking error and returning a non-zero exit; and an empty repo still succeeding.

Ran:

Test Files  2 passed (2)
     Tests  25 passed (25)

tsgo, eslint, and biome are clean on the changed files.

Branch scope — this is the v1.x port; main already has the equivalent fix

main already carries an equivalent Octokit-based fix, so no companion PR is needed there.

This ports the same behavior to the shipping v1.x line, adapted to its raw-apiFetch architecture.


Note

Medium Risk
Changes CLI exit semantics and GitHub request handling for a CI-facing scan path; behavior is well covered by 25 unit tests with injected fakes.

Overview
Fixes ASK-167: socket scan github no longer exits successfully when GitHub rate limits, auth failures, or abuse detection block the run.

Adds utils/github-errors.mts with classifyGitHubResponse (rate limit / auth / abuse), githubApiRequest (bounded retries for short reset windows, 5xx, and network errors), and canonical blocking error messages. Repo details, tree, commit, and contents calls use githubApiRequest; manifest download responses also run through classifyGitHubResponse.

Extracts runGithubScanLoop so multi-repo scans stop early on blocking GitHub errors, return ok: false (non-zero exit), and fail when every repo errors for non-blocking reasons instead of reporting "N repos / 0 manifests" as success. Empty repos and partial success behavior are unchanged.

Reviewed by Cursor Bugbot for commit 30806f1. Configure here.

cursor[bot]

This comment was marked as resolved.

@jdalton
John-David Dalton (jdalton) force-pushed the jdalton/ask-167-improve-cli-to-handle-github-api-rate-limiting-issues branch from ba7c23e to 349fa46 Compare July 24, 2026 21:50
Classify GitHub API failures (rate limit, auth, abuse detection) as
blocking and short-circuit the repo scan loop and manifest download
instead of swallowing them into an ok:true scan of a partial manifest
set. Surface an error when every attempted repo fails so scripts do not
infer success from ok:true with zero scans created.
@jdalton
John-David Dalton (jdalton) force-pushed the jdalton/ask-167-improve-cli-to-handle-github-api-rate-limiting-issues branch from 349fa46 to 96d89e2 Compare July 30, 2026 15:21
@jdalton
John-David Dalton (jdalton) enabled auto-merge (squash) July 30, 2026 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant