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

Problem

socket scan github (the GitHub-API scanning path) read every GitHub REST response body and JSON-parsed it without ever checking the HTTP status. When the user's PAT is rate-limited, GitHub returns one of:

  • 403 with x-ratelimit-remaining: 0 (primary limit),
  • 429, or
  • 403 with a secondary rate limit body.

The old code misread these as "repo has no default branch / no manifests", the per-repo error was swallowed by the scan loop, and the run exited 0 printing:

✔ N GitHub repos detected
✔ 0 with supported Manifest files

So users and CI believed the scan succeeded when nothing had been uploaded.

Fix

New utils/github-errors.mts:

  • classifyGitHubResponse — detects rate-limit / abuse-detection / auth responses and returns a clear, actionable CResult error.
  • githubApiRequest — bounded-retry wrapper around apiFetch: waits once on short Retry-After / x-ratelimit-reset windows (≤ 30s), surfaces long/hourly resets immediately, retries 5xx / network with capped exponential backoff, never retries auth.

Wired in:

  • repo-details / tree / commit / contents calls go through githubApiRequest; the raw manifest-download host is checked too.
  • runGithubScanLoop stops on a blocking error (rate limit / auth / abuse) and returns ok:falsenon-zero exit, printing which condition hit and, when known, when it resets. A run where every repo failed is no longer reported as success. Genuine empty repos still succeed.

Before / after

scenario before after
rate-limited PAT exit 0, "0 manifests" exit 1, GitHub rate limit exceeded …
empty repo success success (unchanged)

Tests

src/utils/github-errors.test.mts + src/commands/scan/create-scan-from-github.test.mts — 25 tests, no network and no module mocks (injected fake fetch / scanRepoFn). Cover 403+remaining:0 / 429 / secondary-limit / 401 detection, Retry-After & reset parsing, cheap-retry-then-succeed, 5xx backoff, loop stop-on-blocking + non-zero exit, and empty repo still succeeds.

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

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

Note

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.

Refs ASK-167.


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